* [PATCH net v3 0/3] vsock: Fix transport_{h2g,g2h,dgram,local} TOCTOU issues
@ 2025-07-02 13:38 Michal Luczaj
2025-07-02 13:38 ` [PATCH net v3 1/3] vsock: Fix transport_{g2h,h2g} TOCTOU Michal Luczaj
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Michal Luczaj @ 2025-07-02 13:38 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi
Cc: virtualization, netdev, linux-kernel, Michal Luczaj
transport_{h2g,g2h,dgram,local} may become NULL on vsock_core_unregister().
Make sure a poorly timed `rmmod transport` won't lead to a NULL/stale
pointer dereference.
Note that these oopses are pretty unlikely to happen in the wild. Splats
were collected after sprinkling kernel with mdelay()s.
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
Changes in v3:
- Static transport_* CID getter rename and comment [Stefano]
- Link to v2: https://lore.kernel.org/r/20250620-vsock-transports-toctou-v2-0-02ebd20b1d03@rbox.co
Changes in v2:
- Introduce a helper function to get local CIDs safely [Stefano]
- Rename goto label to indicate an error path, explain why releasing
vsock_register_mutex after try_module_get() is safe [Stefano]
- Link to v1: https://lore.kernel.org/r/20250618-vsock-transports-toctou-v1-0-dd2d2ede9052@rbox.co
---
Michal Luczaj (3):
vsock: Fix transport_{g2h,h2g} TOCTOU
vsock: Fix transport_* TOCTOU
vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `transport_local`
net/vmw_vsock/af_vsock.c | 57 ++++++++++++++++++++++++++++++++++++++----------
1 file changed, 46 insertions(+), 11 deletions(-)
---
base-commit: 561aa0e22b70a5e7246b73d62a824b3aef3fc375
change-id: 20250523-vsock-transports-toctou-4b75d9c2a805
Best regards,
--
Michal Luczaj <mhal@rbox.co>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net v3 1/3] vsock: Fix transport_{g2h,h2g} TOCTOU
2025-07-02 13:38 [PATCH net v3 0/3] vsock: Fix transport_{h2g,g2h,dgram,local} TOCTOU issues Michal Luczaj
@ 2025-07-02 13:38 ` Michal Luczaj
2025-07-03 8:18 ` Stefano Garzarella
2025-07-02 13:38 ` [PATCH net v3 2/3] vsock: Fix transport_* TOCTOU Michal Luczaj
2025-07-02 13:38 ` [PATCH net v3 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `transport_local` Michal Luczaj
2 siblings, 1 reply; 8+ messages in thread
From: Michal Luczaj @ 2025-07-02 13:38 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi
Cc: virtualization, netdev, linux-kernel, Michal Luczaj
vsock_find_cid() and vsock_dev_do_ioctl() may race with module unload.
transport_{g2h,h2g} may become NULL after the NULL check.
Introduce vsock_transport_local_cid() to protect from a potential
null-ptr-deref.
KASAN: null-ptr-deref in range [0x0000000000000118-0x000000000000011f]
RIP: 0010:vsock_find_cid+0x47/0x90
Call Trace:
__vsock_bind+0x4b2/0x720
vsock_bind+0x90/0xe0
__sys_bind+0x14d/0x1e0
__x64_sys_bind+0x6e/0xc0
do_syscall_64+0x92/0x1c0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
KASAN: null-ptr-deref in range [0x0000000000000118-0x000000000000011f]
RIP: 0010:vsock_dev_do_ioctl.isra.0+0x58/0xf0
Call Trace:
__x64_sys_ioctl+0x12d/0x190
do_syscall_64+0x92/0x1c0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
net/vmw_vsock/af_vsock.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 2e7a3034e965db30b6ee295370d866e6d8b1c341..39473b9e0829f240045262aef00cbae82a425dcc 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -531,9 +531,25 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
}
EXPORT_SYMBOL_GPL(vsock_assign_transport);
+/*
+ * Provide safe access to static transport_{h2g,g2h,dgram,local} callbacks.
+ * Otherwise we may race with module removal. Do not use on `vsk->transport`.
+ */
+static u32 vsock_registered_transport_cid(const struct vsock_transport **transport)
+{
+ u32 cid = VMADDR_CID_ANY;
+
+ mutex_lock(&vsock_register_mutex);
+ if (*transport)
+ cid = (*transport)->get_local_cid();
+ mutex_unlock(&vsock_register_mutex);
+
+ return cid;
+}
+
bool vsock_find_cid(unsigned int cid)
{
- if (transport_g2h && cid == transport_g2h->get_local_cid())
+ if (cid == vsock_registered_transport_cid(&transport_g2h))
return true;
if (transport_h2g && cid == VMADDR_CID_HOST)
@@ -2536,18 +2552,17 @@ static long vsock_dev_do_ioctl(struct file *filp,
unsigned int cmd, void __user *ptr)
{
u32 __user *p = ptr;
- u32 cid = VMADDR_CID_ANY;
int retval = 0;
+ u32 cid;
switch (cmd) {
case IOCTL_VM_SOCKETS_GET_LOCAL_CID:
/* To be compatible with the VMCI behavior, we prioritize the
* guest CID instead of well-know host CID (VMADDR_CID_HOST).
*/
- if (transport_g2h)
- cid = transport_g2h->get_local_cid();
- else if (transport_h2g)
- cid = transport_h2g->get_local_cid();
+ cid = vsock_registered_transport_cid(&transport_g2h);
+ if (cid == VMADDR_CID_ANY)
+ cid = vsock_registered_transport_cid(&transport_h2g);
if (put_user(cid, p) != 0)
retval = -EFAULT;
--
2.49.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net v3 2/3] vsock: Fix transport_* TOCTOU
2025-07-02 13:38 [PATCH net v3 0/3] vsock: Fix transport_{h2g,g2h,dgram,local} TOCTOU issues Michal Luczaj
2025-07-02 13:38 ` [PATCH net v3 1/3] vsock: Fix transport_{g2h,h2g} TOCTOU Michal Luczaj
@ 2025-07-02 13:38 ` Michal Luczaj
2025-07-03 8:20 ` Stefano Garzarella
2025-07-02 13:38 ` [PATCH net v3 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `transport_local` Michal Luczaj
2 siblings, 1 reply; 8+ messages in thread
From: Michal Luczaj @ 2025-07-02 13:38 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi
Cc: virtualization, netdev, linux-kernel, Michal Luczaj
Transport assignment may race with module unload. Protect new_transport
from becoming a stale pointer.
This also takes care of an insecure call in vsock_use_local_transport();
add a lockdep assert.
BUG: unable to handle page fault for address: fffffbfff8056000
Oops: Oops: 0000 [#1] SMP KASAN
RIP: 0010:vsock_assign_transport+0x366/0x600
Call Trace:
vsock_connect+0x59c/0xc40
__sys_connect+0xe8/0x100
__x64_sys_connect+0x6e/0xc0
do_syscall_64+0x92/0x1c0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
net/vmw_vsock/af_vsock.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 39473b9e0829f240045262aef00cbae82a425dcc..9b2af5c63f7c2ae575c160415bd77208a3980835 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -407,6 +407,8 @@ EXPORT_SYMBOL_GPL(vsock_enqueue_accept);
static bool vsock_use_local_transport(unsigned int remote_cid)
{
+ lockdep_assert_held(&vsock_register_mutex);
+
if (!transport_local)
return false;
@@ -464,6 +466,8 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
remote_flags = vsk->remote_addr.svm_flags;
+ mutex_lock(&vsock_register_mutex);
+
switch (sk->sk_type) {
case SOCK_DGRAM:
new_transport = transport_dgram;
@@ -479,12 +483,15 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
new_transport = transport_h2g;
break;
default:
- return -ESOCKTNOSUPPORT;
+ ret = -ESOCKTNOSUPPORT;
+ goto err;
}
if (vsk->transport) {
- if (vsk->transport == new_transport)
- return 0;
+ if (vsk->transport == new_transport) {
+ ret = 0;
+ goto err;
+ }
/* transport->release() must be called with sock lock acquired.
* This path can only be taken during vsock_connect(), where we
@@ -508,8 +515,16 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
/* We increase the module refcnt to prevent the transport unloading
* while there are open sockets assigned to it.
*/
- if (!new_transport || !try_module_get(new_transport->module))
- return -ENODEV;
+ if (!new_transport || !try_module_get(new_transport->module)) {
+ ret = -ENODEV;
+ goto err;
+ }
+
+ /* It's safe to release the mutex after a successful try_module_get().
+ * Whichever transport `new_transport` points at, it won't go await
+ * until the last module_put() below or in vsock_deassign_transport().
+ */
+ mutex_unlock(&vsock_register_mutex);
if (sk->sk_type == SOCK_SEQPACKET) {
if (!new_transport->seqpacket_allow ||
@@ -528,6 +543,9 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
vsk->transport = new_transport;
return 0;
+err:
+ mutex_unlock(&vsock_register_mutex);
+ return ret;
}
EXPORT_SYMBOL_GPL(vsock_assign_transport);
--
2.49.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net v3 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `transport_local`
2025-07-02 13:38 [PATCH net v3 0/3] vsock: Fix transport_{h2g,g2h,dgram,local} TOCTOU issues Michal Luczaj
2025-07-02 13:38 ` [PATCH net v3 1/3] vsock: Fix transport_{g2h,h2g} TOCTOU Michal Luczaj
2025-07-02 13:38 ` [PATCH net v3 2/3] vsock: Fix transport_* TOCTOU Michal Luczaj
@ 2025-07-02 13:38 ` Michal Luczaj
2025-07-03 8:23 ` Stefano Garzarella
2 siblings, 1 reply; 8+ messages in thread
From: Michal Luczaj @ 2025-07-02 13:38 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi
Cc: virtualization, netdev, linux-kernel, Michal Luczaj
Support returning VMADDR_CID_LOCAL in case no other vsock transport is
available.
Fixes: 0e12190578d0 ("vsock: add local transport support in the vsock core")
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
net/vmw_vsock/af_vsock.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 9b2af5c63f7c2ae575c160415bd77208a3980835..c8398f9cec5296e07395df8e7ad0f52b8ceb65d5 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -2581,6 +2581,8 @@ static long vsock_dev_do_ioctl(struct file *filp,
cid = vsock_registered_transport_cid(&transport_g2h);
if (cid == VMADDR_CID_ANY)
cid = vsock_registered_transport_cid(&transport_h2g);
+ if (cid == VMADDR_CID_ANY)
+ cid = vsock_registered_transport_cid(&transport_local);
if (put_user(cid, p) != 0)
retval = -EFAULT;
--
2.49.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 1/3] vsock: Fix transport_{g2h,h2g} TOCTOU
2025-07-02 13:38 ` [PATCH net v3 1/3] vsock: Fix transport_{g2h,h2g} TOCTOU Michal Luczaj
@ 2025-07-03 8:18 ` Stefano Garzarella
0 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2025-07-03 8:18 UTC (permalink / raw)
To: Michal Luczaj
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, virtualization, netdev,
linux-kernel
On Wed, Jul 02, 2025 at 03:38:43PM +0200, Michal Luczaj wrote:
>vsock_find_cid() and vsock_dev_do_ioctl() may race with module unload.
>transport_{g2h,h2g} may become NULL after the NULL check.
>
>Introduce vsock_transport_local_cid() to protect from a potential
>null-ptr-deref.
>
>KASAN: null-ptr-deref in range [0x0000000000000118-0x000000000000011f]
>RIP: 0010:vsock_find_cid+0x47/0x90
>Call Trace:
> __vsock_bind+0x4b2/0x720
> vsock_bind+0x90/0xe0
> __sys_bind+0x14d/0x1e0
> __x64_sys_bind+0x6e/0xc0
> do_syscall_64+0x92/0x1c0
> entry_SYSCALL_64_after_hwframe+0x4b/0x53
>
>KASAN: null-ptr-deref in range [0x0000000000000118-0x000000000000011f]
>RIP: 0010:vsock_dev_do_ioctl.isra.0+0x58/0xf0
>Call Trace:
> __x64_sys_ioctl+0x12d/0x190
> do_syscall_64+0x92/0x1c0
> entry_SYSCALL_64_after_hwframe+0x4b/0x53
>
>Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> net/vmw_vsock/af_vsock.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Thanks!
Stefano
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 2e7a3034e965db30b6ee295370d866e6d8b1c341..39473b9e0829f240045262aef00cbae82a425dcc 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -531,9 +531,25 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> }
> EXPORT_SYMBOL_GPL(vsock_assign_transport);
>
>+/*
>+ * Provide safe access to static transport_{h2g,g2h,dgram,local} callbacks.
>+ * Otherwise we may race with module removal. Do not use on `vsk->transport`.
>+ */
>+static u32 vsock_registered_transport_cid(const struct vsock_transport **transport)
>+{
>+ u32 cid = VMADDR_CID_ANY;
>+
>+ mutex_lock(&vsock_register_mutex);
>+ if (*transport)
>+ cid = (*transport)->get_local_cid();
>+ mutex_unlock(&vsock_register_mutex);
>+
>+ return cid;
>+}
>+
> bool vsock_find_cid(unsigned int cid)
> {
>- if (transport_g2h && cid == transport_g2h->get_local_cid())
>+ if (cid == vsock_registered_transport_cid(&transport_g2h))
> return true;
>
> if (transport_h2g && cid == VMADDR_CID_HOST)
>@@ -2536,18 +2552,17 @@ static long vsock_dev_do_ioctl(struct file *filp,
> unsigned int cmd, void __user *ptr)
> {
> u32 __user *p = ptr;
>- u32 cid = VMADDR_CID_ANY;
> int retval = 0;
>+ u32 cid;
>
> switch (cmd) {
> case IOCTL_VM_SOCKETS_GET_LOCAL_CID:
> /* To be compatible with the VMCI behavior, we prioritize the
> * guest CID instead of well-know host CID (VMADDR_CID_HOST).
> */
>- if (transport_g2h)
>- cid = transport_g2h->get_local_cid();
>- else if (transport_h2g)
>- cid = transport_h2g->get_local_cid();
>+ cid = vsock_registered_transport_cid(&transport_g2h);
>+ if (cid == VMADDR_CID_ANY)
>+ cid = vsock_registered_transport_cid(&transport_h2g);
>
> if (put_user(cid, p) != 0)
> retval = -EFAULT;
>
>--
>2.49.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 2/3] vsock: Fix transport_* TOCTOU
2025-07-02 13:38 ` [PATCH net v3 2/3] vsock: Fix transport_* TOCTOU Michal Luczaj
@ 2025-07-03 8:20 ` Stefano Garzarella
2025-07-03 8:36 ` Michal Luczaj
0 siblings, 1 reply; 8+ messages in thread
From: Stefano Garzarella @ 2025-07-03 8:20 UTC (permalink / raw)
To: Michal Luczaj
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, virtualization, netdev,
linux-kernel
On Wed, Jul 02, 2025 at 03:38:44PM +0200, Michal Luczaj wrote:
>Transport assignment may race with module unload. Protect new_transport
>from becoming a stale pointer.
>
>This also takes care of an insecure call in vsock_use_local_transport();
>add a lockdep assert.
>
>BUG: unable to handle page fault for address: fffffbfff8056000
>Oops: Oops: 0000 [#1] SMP KASAN
>RIP: 0010:vsock_assign_transport+0x366/0x600
>Call Trace:
> vsock_connect+0x59c/0xc40
> __sys_connect+0xe8/0x100
> __x64_sys_connect+0x6e/0xc0
> do_syscall_64+0x92/0x1c0
> entry_SYSCALL_64_after_hwframe+0x4b/0x53
>
>Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
>Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> net/vmw_vsock/af_vsock.c | 28 +++++++++++++++++++++++-----
> 1 file changed, 23 insertions(+), 5 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 39473b9e0829f240045262aef00cbae82a425dcc..9b2af5c63f7c2ae575c160415bd77208a3980835 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -407,6 +407,8 @@ EXPORT_SYMBOL_GPL(vsock_enqueue_accept);
>
> static bool vsock_use_local_transport(unsigned int remote_cid)
> {
>+ lockdep_assert_held(&vsock_register_mutex);
>+
> if (!transport_local)
> return false;
>
>@@ -464,6 +466,8 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
>
> remote_flags = vsk->remote_addr.svm_flags;
>
>+ mutex_lock(&vsock_register_mutex);
>+
> switch (sk->sk_type) {
> case SOCK_DGRAM:
> new_transport = transport_dgram;
>@@ -479,12 +483,15 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> new_transport = transport_h2g;
> break;
> default:
>- return -ESOCKTNOSUPPORT;
>+ ret = -ESOCKTNOSUPPORT;
>+ goto err;
> }
>
> if (vsk->transport) {
>- if (vsk->transport == new_transport)
>- return 0;
>+ if (vsk->transport == new_transport) {
>+ ret = 0;
>+ goto err;
>+ }
>
> /* transport->release() must be called with sock lock acquired.
> * This path can only be taken during vsock_connect(), where we
>@@ -508,8 +515,16 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> /* We increase the module refcnt to prevent the transport unloading
> * while there are open sockets assigned to it.
> */
>- if (!new_transport || !try_module_get(new_transport->module))
>- return -ENODEV;
>+ if (!new_transport || !try_module_get(new_transport->module)) {
>+ ret = -ENODEV;
>+ goto err;
>+ }
>+
>+ /* It's safe to release the mutex after a successful try_module_get().
>+ * Whichever transport `new_transport` points at, it won't go await
Little typo, s/await/away
Up to you to resend or not. My R-b stay for both cases.
Thanks,
Stefano
>+ * until the last module_put() below or in vsock_deassign_transport().
>+ */
>+ mutex_unlock(&vsock_register_mutex);
>
> if (sk->sk_type == SOCK_SEQPACKET) {
> if (!new_transport->seqpacket_allow ||
>@@ -528,6 +543,9 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> vsk->transport = new_transport;
>
> return 0;
>+err:
>+ mutex_unlock(&vsock_register_mutex);
>+ return ret;
> }
> EXPORT_SYMBOL_GPL(vsock_assign_transport);
>
>
>--
>2.49.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `transport_local`
2025-07-02 13:38 ` [PATCH net v3 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `transport_local` Michal Luczaj
@ 2025-07-03 8:23 ` Stefano Garzarella
0 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2025-07-03 8:23 UTC (permalink / raw)
To: Michal Luczaj
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, virtualization, netdev,
linux-kernel
On Wed, Jul 02, 2025 at 03:38:45PM +0200, Michal Luczaj wrote:
>Support returning VMADDR_CID_LOCAL in case no other vsock transport is
>available.
>
>Fixes: 0e12190578d0 ("vsock: add local transport support in the vsock core")
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> net/vmw_vsock/af_vsock.c | 2 ++
> 1 file changed, 2 insertions(+)
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 9b2af5c63f7c2ae575c160415bd77208a3980835..c8398f9cec5296e07395df8e7ad0f52b8ceb65d5 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -2581,6 +2581,8 @@ static long vsock_dev_do_ioctl(struct file *filp,
> cid = vsock_registered_transport_cid(&transport_g2h);
> if (cid == VMADDR_CID_ANY)
> cid = vsock_registered_transport_cid(&transport_h2g);
>+ if (cid == VMADDR_CID_ANY)
>+ cid = vsock_registered_transport_cid(&transport_local);
>
> if (put_user(cid, p) != 0)
> retval = -EFAULT;
>
>--
>2.49.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 2/3] vsock: Fix transport_* TOCTOU
2025-07-03 8:20 ` Stefano Garzarella
@ 2025-07-03 8:36 ` Michal Luczaj
0 siblings, 0 replies; 8+ messages in thread
From: Michal Luczaj @ 2025-07-03 8:36 UTC (permalink / raw)
To: Stefano Garzarella
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, virtualization, netdev,
linux-kernel
On 7/3/25 10:20, Stefano Garzarella wrote:
> On Wed, Jul 02, 2025 at 03:38:44PM +0200, Michal Luczaj wrote:
>> Transport assignment may race with module unload. Protect new_transport
>>from becoming a stale pointer.
>>
>> This also takes care of an insecure call in vsock_use_local_transport();
>> add a lockdep assert.
>>
>> BUG: unable to handle page fault for address: fffffbfff8056000
>> Oops: Oops: 0000 [#1] SMP KASAN
>> RIP: 0010:vsock_assign_transport+0x366/0x600
>> Call Trace:
>> vsock_connect+0x59c/0xc40
>> __sys_connect+0xe8/0x100
>> __x64_sys_connect+0x6e/0xc0
>> do_syscall_64+0x92/0x1c0
>> entry_SYSCALL_64_after_hwframe+0x4b/0x53
>>
>> Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>> ---
>> net/vmw_vsock/af_vsock.c | 28 +++++++++++++++++++++++-----
>> 1 file changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index 39473b9e0829f240045262aef00cbae82a425dcc..9b2af5c63f7c2ae575c160415bd77208a3980835 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -407,6 +407,8 @@ EXPORT_SYMBOL_GPL(vsock_enqueue_accept);
>>
>> static bool vsock_use_local_transport(unsigned int remote_cid)
>> {
>> + lockdep_assert_held(&vsock_register_mutex);
>> +
>> if (!transport_local)
>> return false;
>>
>> @@ -464,6 +466,8 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
>>
>> remote_flags = vsk->remote_addr.svm_flags;
>>
>> + mutex_lock(&vsock_register_mutex);
>> +
>> switch (sk->sk_type) {
>> case SOCK_DGRAM:
>> new_transport = transport_dgram;
>> @@ -479,12 +483,15 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
>> new_transport = transport_h2g;
>> break;
>> default:
>> - return -ESOCKTNOSUPPORT;
>> + ret = -ESOCKTNOSUPPORT;
>> + goto err;
>> }
>>
>> if (vsk->transport) {
>> - if (vsk->transport == new_transport)
>> - return 0;
>> + if (vsk->transport == new_transport) {
>> + ret = 0;
>> + goto err;
>> + }
>>
>> /* transport->release() must be called with sock lock acquired.
>> * This path can only be taken during vsock_connect(), where we
>> @@ -508,8 +515,16 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
>> /* We increase the module refcnt to prevent the transport unloading
>> * while there are open sockets assigned to it.
>> */
>> - if (!new_transport || !try_module_get(new_transport->module))
>> - return -ENODEV;
>> + if (!new_transport || !try_module_get(new_transport->module)) {
>> + ret = -ENODEV;
>> + goto err;
>> + }
>> +
>> + /* It's safe to release the mutex after a successful try_module_get().
>> + * Whichever transport `new_transport` points at, it won't go await
>
> Little typo, s/await/away
>
> Up to you to resend or not. My R-b stay for both cases.
Arrgh, thanks. I'll fix it.
pw-bot: changes-requested
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-07-03 8:36 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-02 13:38 [PATCH net v3 0/3] vsock: Fix transport_{h2g,g2h,dgram,local} TOCTOU issues Michal Luczaj
2025-07-02 13:38 ` [PATCH net v3 1/3] vsock: Fix transport_{g2h,h2g} TOCTOU Michal Luczaj
2025-07-03 8:18 ` Stefano Garzarella
2025-07-02 13:38 ` [PATCH net v3 2/3] vsock: Fix transport_* TOCTOU Michal Luczaj
2025-07-03 8:20 ` Stefano Garzarella
2025-07-03 8:36 ` Michal Luczaj
2025-07-02 13:38 ` [PATCH net v3 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `transport_local` Michal Luczaj
2025-07-03 8:23 ` Stefano Garzarella
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).