On Wed, Nov 26, 2025 at 02:38:26PM +0100, Stefano Garzarella wrote: > From: Stefano Garzarella <[email protected]> > > vhost_vsock_get() uses hash_for_each_possible_rcu() to find the > `vhost_vsock` associated with the `guest_cid`. hash_for_each_possible_rcu() > should only be called within an RCU read section, as mentioned in the > following comment in include/linux/rculist.h: > > /** > * hlist_for_each_entry_rcu - iterate over rcu list of given type > * @pos: the type * to use as a loop cursor. > * @head: the head for your list. > * @member: the name of the hlist_node within the struct. > * @cond: optional lockdep expression if called from non-RCU protection. > * > * This list-traversal primitive may safely run concurrently with > * the _rcu list-mutation primitives such as hlist_add_head_rcu() > * as long as the traversal is guarded by rcu_read_lock(). > */ > > Currently, all calls to vhost_vsock_get() are between rcu_read_lock() > and rcu_read_unlock() except for calls in vhost_vsock_set_cid() and > vhost_vsock_reset_orphans(). In both cases, the current code is safe, > but we can make improvements to make it more robust. > > About vhost_vsock_set_cid(), when building the kernel with > CONFIG_PROVE_RCU_LIST enabled, we get the following RCU warning when the > user space issues `ioctl(dev, VHOST_VSOCK_SET_GUEST_CID, ...)` : > > WARNING: suspicious RCU usage > 6.18.0-rc7 #62 Not tainted > ----------------------------- > drivers/vhost/vsock.c:74 RCU-list traversed in non-reader section!! > > other info that might help us debug this: > > rcu_scheduler_active = 2, debug_locks = 1 > 1 lock held by rpc-libvirtd/3443: > #0: ffffffffc05032a8 (vhost_vsock_mutex){+.+.}-{4:4}, at: > vhost_vsock_dev_ioctl+0x2ff/0x530 [vhost_vsock] > > stack backtrace: > CPU: 2 UID: 0 PID: 3443 Comm: rpc-libvirtd Not tainted 6.18.0-rc7 #62 > PREEMPT(none) > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-7.fc42 > 06/10/2025 > Call Trace: > <TASK> > dump_stack_lvl+0x75/0xb0 > dump_stack+0x14/0x1a > lockdep_rcu_suspicious.cold+0x4e/0x97 > vhost_vsock_get+0x8f/0xa0 [vhost_vsock] > vhost_vsock_dev_ioctl+0x307/0x530 [vhost_vsock] > __x64_sys_ioctl+0x4f2/0xa00 > x64_sys_call+0xed0/0x1da0 > do_syscall_64+0x73/0xfa0 > entry_SYSCALL_64_after_hwframe+0x76/0x7e > ... > </TASK> > > This is not a real problem, because the vhost_vsock_get() caller, i.e. > vhost_vsock_set_cid(), holds the `vhost_vsock_mutex` used by the hash > table writers. Anyway, to prevent that warning, add lockdep_is_held() > condition to hash_for_each_possible_rcu() to verify that either the > caller is in an RCU read section or `vhost_vsock_mutex` is held when > CONFIG_PROVE_RCU_LIST is enabled; and also clarify the comment for > vhost_vsock_get() to better describe the locking requirements and the > scope of the returned pointer validity. > > About vhost_vsock_reset_orphans(), currently this function is only > called via vsock_for_each_connected_socket(), which holds the > `vsock_table_lock` spinlock (which is also an RCU read-side critical > section). However, add an explicit RCU read lock there to make the code > more robust and explicit about the RCU requirements, and to prevent > issues if the calling context changes in the future or if > vhost_vsock_reset_orphans() is called from other contexts. > > Fixes: 834e772c8db0 ("vhost/vsock: fix use-after-free in network stack > callers") > Cc: [email protected] > Signed-off-by: Stefano Garzarella <[email protected]> > --- > drivers/vhost/vsock.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-)
Reviewed-by: Stefan Hajnoczi <[email protected]>
signature.asc
Description: PGP signature
