Re: [PATCH v4] vhost: fix crash caused by accessing a freed vsocket

[David Marchand <david.marchand@redhat.com>]

Hello,

On Mon, Jul 8, 2024 at 6:41 AM Gongming Chen
<chengongming1900@outlook.com> wrote:
>
> From: Gongming Chen <chengm11@chinatelecom.cn>
>
> When a vhost user message handling error in the event dispatch thread,
> vsocket reconn is added to the reconnection list of the reconnection
> thread.
> Since the reconnection, event dispatching and app configuration thread
> do not have common thread protection restrictions, the app config
> thread freed vsocket in the rte_vhost_driver_unregister process,
> but vsocket reconn can still exist in the reconn_list through this
> mechanism.
> Then in the reconnection thread, the vsocket is connected again and
> conn is added to the dispatch thread.
> Finally, the vsocket that has been freed by rte_vhost_driver_unregister
> is accessed again in the event dispatch thread, resulting in a
> use-after-free error.
>
> This patch adds a vhost threads read-write lock to restrict
> reconnection, event dispatching and app configuration threads.
> When the vhost driver unregisters, it exclusively holds the lock to
> safely free the vsocket.
>
> #0  0x0000000000000025 in ?? ()
> #1  0x0000000003ed7ca0 in vhost_user_read_cb at lib/vhost/socket.c:323
> #2  0x0000000003ed625f in fdset_event_dispatch at lib/vhost/fd_man.c:365
>
> Fixes: e623e0c6d8a5 ("vhost: add vhost-user client mode")
> Cc: stable@dpdk.org
>
> Signed-off-by: Gongming Chen <chengm11@chinatelecom.cn>

Maxime is off for the coming weeks.
Adding one lock is risky at this point of the release, especially as
it is mixed with other locks.

I prefer not to take this fix without an in depth review, and ideally
a ack from Maxime.
I marked this patch as deferred to the next release.


-- 
David Marchand