All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] vsock: fix use-after free and null-ptr-deref
@ 2025-02-14 17:53 Luigi Leonardi
  2025-02-14 17:53 ` [PATCH 1/2] vsock: Keep the binding until socket destruction Luigi Leonardi
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Luigi Leonardi @ 2025-02-14 17:53 UTC (permalink / raw)
  To: Stefano Garzarella, Michal Luczaj, stable
  Cc: Jakub Kicinski, syzbot+9d55b199192a4be7d02c, Luigi Leonardi

Hi all,

This series contains two patches that are already available upstream:

- The first commit fixes a use-after-free[1], but introduced a 
null-ptr-deref[2].
- The second commit fixes it. [3]

I suggested waiting for both of them to be merged upstream and then 
applying them togheter to stable[4].

It should be applied to:
- 6.13.y
- 6.12.y
- 6.6.y

I will send another series for
- 6.1.y
- 5.15.y
- 5.10.y

because of conflicts.

[1]https://lore.kernel.org/all/20250128-vsock-transport-vs-autobind-v3-0-1cf57065b770@rbox.co/
[2]https://lore.kernel.org/all/67a09300.050a0220.d7c5a.008b.GAE@google.com/
[3]https://lore.kernel.org/all/20250210-vsock-linger-nullderef-v3-0-ef6244d02b54@rbox.co/
[4]https://lore.kernel.org/all/2025020644-unwitting-scary-3c0d@gregkh/

Thanks,
Luigi

---
Michal Luczaj (2):
      vsock: Keep the binding until socket destruction
      vsock: Orphan socket after transport release

 net/vmw_vsock/af_vsock.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
---
base-commit: a1856aaa2ca74c88751f7d255dfa0c8c50fcc1ca
change-id: 20250214-linux-rolling-stable-d73f0bed815d

Best regards,
-- 
Luigi Leonardi <leonardi@redhat.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] vsock: Keep the binding until socket destruction
  2025-02-14 17:53 [PATCH 0/2] vsock: fix use-after free and null-ptr-deref Luigi Leonardi
@ 2025-02-14 17:53 ` Luigi Leonardi
  2025-02-14 17:53 ` [PATCH 2/2] vsock: Orphan socket after transport release Luigi Leonardi
  2025-02-17 11:18 ` [PATCH 0/2] vsock: fix use-after free and null-ptr-deref Luigi Leonardi
  2 siblings, 0 replies; 8+ messages in thread
From: Luigi Leonardi @ 2025-02-14 17:53 UTC (permalink / raw)
  To: Stefano Garzarella, Michal Luczaj, stable; +Cc: Jakub Kicinski

From: Michal Luczaj <mhal@rbox.co>

commit fcdd2242c0231032fc84e1404315c245ae56322a upstream.

Preserve sockets bindings; this includes both resulting from an explicit
bind() and those implicitly bound through autobind during connect().

Prevents socket unbinding during a transport reassignment, which fixes a
use-after-free:

    1. vsock_create() (refcnt=1) calls vsock_insert_unbound() (refcnt=2)
    2. transport->release() calls vsock_remove_bound() without checking if
       sk was bound and moved to bound list (refcnt=1)
    3. vsock_bind() assumes sk is in unbound list and before
       __vsock_insert_bound(vsock_bound_sockets()) calls
       __vsock_remove_bound() which does:
           list_del_init(&vsk->bound_table); // nop
           sock_put(&vsk->sk);               // refcnt=0

BUG: KASAN: slab-use-after-free in __vsock_bind+0x62e/0x730
Read of size 4 at addr ffff88816b46a74c by task a.out/2057
 dump_stack_lvl+0x68/0x90
 print_report+0x174/0x4f6
 kasan_report+0xb9/0x190
 __vsock_bind+0x62e/0x730
 vsock_bind+0x97/0xe0
 __sys_bind+0x154/0x1f0
 __x64_sys_bind+0x6e/0xb0
 do_syscall_64+0x93/0x1b0
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

Allocated by task 2057:
 kasan_save_stack+0x1e/0x40
 kasan_save_track+0x10/0x30
 __kasan_slab_alloc+0x85/0x90
 kmem_cache_alloc_noprof+0x131/0x450
 sk_prot_alloc+0x5b/0x220
 sk_alloc+0x2c/0x870
 __vsock_create.constprop.0+0x2e/0xb60
 vsock_create+0xe4/0x420
 __sock_create+0x241/0x650
 __sys_socket+0xf2/0x1a0
 __x64_sys_socket+0x6e/0xb0
 do_syscall_64+0x93/0x1b0
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

Freed by task 2057:
 kasan_save_stack+0x1e/0x40
 kasan_save_track+0x10/0x30
 kasan_save_free_info+0x37/0x60
 __kasan_slab_free+0x4b/0x70
 kmem_cache_free+0x1a1/0x590
 __sk_destruct+0x388/0x5a0
 __vsock_bind+0x5e1/0x730
 vsock_bind+0x97/0xe0
 __sys_bind+0x154/0x1f0
 __x64_sys_bind+0x6e/0xb0
 do_syscall_64+0x93/0x1b0
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

refcount_t: addition on 0; use-after-free.
WARNING: CPU: 7 PID: 2057 at lib/refcount.c:25 refcount_warn_saturate+0xce/0x150
RIP: 0010:refcount_warn_saturate+0xce/0x150
 __vsock_bind+0x66d/0x730
 vsock_bind+0x97/0xe0
 __sys_bind+0x154/0x1f0
 __x64_sys_bind+0x6e/0xb0
 do_syscall_64+0x93/0x1b0
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

refcount_t: underflow; use-after-free.
WARNING: CPU: 7 PID: 2057 at lib/refcount.c:28 refcount_warn_saturate+0xee/0x150
RIP: 0010:refcount_warn_saturate+0xee/0x150
 vsock_remove_bound+0x187/0x1e0
 __vsock_release+0x383/0x4a0
 vsock_release+0x90/0x120
 __sock_release+0xa3/0x250
 sock_close+0x14/0x20
 __fput+0x359/0xa80
 task_work_run+0x107/0x1d0
 do_exit+0x847/0x2560
 do_group_exit+0xb8/0x250
 __x64_sys_exit_group+0x3a/0x50
 x64_sys_call+0xfec/0x14f0
 do_syscall_64+0x93/0x1b0
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
Link: https://patch.msgid.link/20250128-vsock-transport-vs-autobind-v3-1-1cf57065b770@rbox.co
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/vmw_vsock/af_vsock.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index f5d116a1bdea1a0b1a80488a27ce71ee636a65fa..ec4c1fbbcec7418d2e715bad30845cd95a9b270f 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -337,7 +337,10 @@ EXPORT_SYMBOL_GPL(vsock_find_connected_socket);
 
 void vsock_remove_sock(struct vsock_sock *vsk)
 {
-	vsock_remove_bound(vsk);
+	/* Transport reassignment must not remove the binding. */
+	if (sock_flag(sk_vsock(vsk), SOCK_DEAD))
+		vsock_remove_bound(vsk);
+
 	vsock_remove_connected(vsk);
 }
 EXPORT_SYMBOL_GPL(vsock_remove_sock);
@@ -821,12 +824,13 @@ static void __vsock_release(struct sock *sk, int level)
 	 */
 	lock_sock_nested(sk, level);
 
+	sock_orphan(sk);
+
 	if (vsk->transport)
 		vsk->transport->release(vsk);
 	else if (sock_type_connectible(sk->sk_type))
 		vsock_remove_sock(vsk);
 
-	sock_orphan(sk);
 	sk->sk_shutdown = SHUTDOWN_MASK;
 
 	skb_queue_purge(&sk->sk_receive_queue);

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] vsock: Orphan socket after transport release
  2025-02-14 17:53 [PATCH 0/2] vsock: fix use-after free and null-ptr-deref Luigi Leonardi
  2025-02-14 17:53 ` [PATCH 1/2] vsock: Keep the binding until socket destruction Luigi Leonardi
@ 2025-02-14 17:53 ` Luigi Leonardi
  2025-02-17 11:18 ` [PATCH 0/2] vsock: fix use-after free and null-ptr-deref Luigi Leonardi
  2 siblings, 0 replies; 8+ messages in thread
From: Luigi Leonardi @ 2025-02-14 17:53 UTC (permalink / raw)
  To: Stefano Garzarella, Michal Luczaj, stable
  Cc: syzbot+9d55b199192a4be7d02c, Luigi Leonardi, Jakub Kicinski

From: Michal Luczaj <mhal@rbox.co>

commit 78dafe1cf3afa02ed71084b350713b07e72a18fb upstream.

During socket release, sock_orphan() is called without considering that it
sets sk->sk_wq to NULL. Later, if SO_LINGER is enabled, this leads to a
null pointer dereferenced in virtio_transport_wait_close().

Orphan the socket only after transport release.

Partially reverts the 'Fixes:' commit.

KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f]
 lock_acquire+0x19e/0x500
 _raw_spin_lock_irqsave+0x47/0x70
 add_wait_queue+0x46/0x230
 virtio_transport_release+0x4e7/0x7f0
 __vsock_release+0xfd/0x490
 vsock_release+0x90/0x120
 __sock_release+0xa3/0x250
 sock_close+0x14/0x20
 __fput+0x35e/0xa90
 __x64_sys_close+0x78/0xd0
 do_syscall_64+0x93/0x1b0
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

Reported-by: syzbot+9d55b199192a4be7d02c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9d55b199192a4be7d02c
Fixes: fcdd2242c023 ("vsock: Keep the binding until socket destruction")
Tested-by: Luigi Leonardi <leonardi@redhat.com>
Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
Link: https://patch.msgid.link/20250210-vsock-linger-nullderef-v3-1-ef6244d02b54@rbox.co
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/vmw_vsock/af_vsock.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index ec4c1fbbcec7418d2e715bad30845cd95a9b270f..37299a7ca1876e58ff516b5112d44b171cb896b0 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -824,13 +824,19 @@ static void __vsock_release(struct sock *sk, int level)
 	 */
 	lock_sock_nested(sk, level);
 
-	sock_orphan(sk);
+	/* Indicate to vsock_remove_sock() that the socket is being released and
+	 * can be removed from the bound_table. Unlike transport reassignment
+	 * case, where the socket must remain bound despite vsock_remove_sock()
+	 * being called from the transport release() callback.
+	 */
+	sock_set_flag(sk, SOCK_DEAD);
 
 	if (vsk->transport)
 		vsk->transport->release(vsk);
 	else if (sock_type_connectible(sk->sk_type))
 		vsock_remove_sock(vsk);
 
+	sock_orphan(sk);
 	sk->sk_shutdown = SHUTDOWN_MASK;
 
 	skb_queue_purge(&sk->sk_receive_queue);

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] vsock: fix use-after free and null-ptr-deref
  2025-02-14 17:53 [PATCH 0/2] vsock: fix use-after free and null-ptr-deref Luigi Leonardi
  2025-02-14 17:53 ` [PATCH 1/2] vsock: Keep the binding until socket destruction Luigi Leonardi
  2025-02-14 17:53 ` [PATCH 2/2] vsock: Orphan socket after transport release Luigi Leonardi
@ 2025-02-17 11:18 ` Luigi Leonardi
  2025-02-17 19:45   ` Michal Luczaj
  2 siblings, 1 reply; 8+ messages in thread
From: Luigi Leonardi @ 2025-02-17 11:18 UTC (permalink / raw)
  To: Stefano Garzarella, Michal Luczaj, stable
  Cc: Jakub Kicinski, syzbot+9d55b199192a4be7d02c

On Fri, Feb 14, 2025 at 06:53:54PM +0100, Luigi Leonardi wrote:
>Hi all,
>
>This series contains two patches that are already available upstream:
>
>- The first commit fixes a use-after-free[1], but introduced a
>null-ptr-deref[2].
>- The second commit fixes it. [3]
>
>I suggested waiting for both of them to be merged upstream and then
>applying them togheter to stable[4].
>
>It should be applied to:
>- 6.13.y
>- 6.12.y
>- 6.6.y
>
>I will send another series for
>- 6.1.y
>- 5.15.y
>- 5.10.y
>
>because of conflicts.
>
>[1]https://lore.kernel.org/all/20250128-vsock-transport-vs-autobind-v3-0-1cf57065b770@rbox.co/
>[2]https://lore.kernel.org/all/67a09300.050a0220.d7c5a.008b.GAE@google.com/
>[3]https://lore.kernel.org/all/20250210-vsock-linger-nullderef-v3-0-ef6244d02b54@rbox.co/
>[4]https://lore.kernel.org/all/2025020644-unwitting-scary-3c0d@gregkh/
>
>Thanks,
>Luigi
>
>---
>Michal Luczaj (2):
>      vsock: Keep the binding until socket destruction
>      vsock: Orphan socket after transport release
>
> net/vmw_vsock/af_vsock.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>---
>base-commit: a1856aaa2ca74c88751f7d255dfa0c8c50fcc1ca
>change-id: 20250214-linux-rolling-stable-d73f0bed815d
>
>Best regards,
>-- Luigi Leonardi <leonardi@redhat.com>
>

Looks like I forgot to add my SoB to the commits, my bad.

For all the other stable trees (6.1, 5.15 and 5.10), there are some 
conflicts due to some indentation changes introduced by 135ffc7 ("bpf, 
vsock: Invoke proto::close on close()"). Should I backport this commit 
too?  There is no real dependency on the commit in the Fixes tag 
("vsock: support sockmap"). IMHO, this would help future backports, 
because of indentation conficts! Otherwise I can simply fix the patches.  
WDYT?

Cheers,
Luigi


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] vsock: fix use-after free and null-ptr-deref
  2025-02-17 11:18 ` [PATCH 0/2] vsock: fix use-after free and null-ptr-deref Luigi Leonardi
@ 2025-02-17 19:45   ` Michal Luczaj
  2025-02-18  8:35     ` Stefano Garzarella
  0 siblings, 1 reply; 8+ messages in thread
From: Michal Luczaj @ 2025-02-17 19:45 UTC (permalink / raw)
  To: Luigi Leonardi, Stefano Garzarella, stable
  Cc: Jakub Kicinski, syzbot+9d55b199192a4be7d02c

On 2/17/25 12:18, Luigi Leonardi wrote:
> On Fri, Feb 14, 2025 at 06:53:54PM +0100, Luigi Leonardi wrote:
>> Hi all,
>>
>> This series contains two patches that are already available upstream:
>>
>> - The first commit fixes a use-after-free[1], but introduced a
>> null-ptr-deref[2].
>> - The second commit fixes it. [3]
>>
>> I suggested waiting for both of them to be merged upstream and then
>> applying them togheter to stable[4].
>>
>> It should be applied to:
>> - 6.13.y
>> - 6.12.y
>> - 6.6.y
>>
>> I will send another series for
>> - 6.1.y
>> - 5.15.y
>> - 5.10.y
>>
>> because of conflicts.
>>
>> [1]https://lore.kernel.org/all/20250128-vsock-transport-vs-autobind-v3-0-1cf57065b770@rbox.co/
>> [2]https://lore.kernel.org/all/67a09300.050a0220.d7c5a.008b.GAE@google.com/
>> [3]https://lore.kernel.org/all/20250210-vsock-linger-nullderef-v3-0-ef6244d02b54@rbox.co/
>> [4]https://lore.kernel.org/all/2025020644-unwitting-scary-3c0d@gregkh/
>>
>> Thanks,
>> Luigi
>>
>> ---
>> Michal Luczaj (2):
>>      vsock: Keep the binding until socket destruction
>>      vsock: Orphan socket after transport release
>>
>> net/vmw_vsock/af_vsock.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>> ---
>> base-commit: a1856aaa2ca74c88751f7d255dfa0c8c50fcc1ca
>> change-id: 20250214-linux-rolling-stable-d73f0bed815d
>>
>> Best regards,
>> -- Luigi Leonardi <leonardi@redhat.com>
>>
> 
> Looks like I forgot to add my SoB to the commits, my bad.
> 
> For all the other stable trees (6.1, 5.15 and 5.10), there are some 
> conflicts due to some indentation changes introduced by 135ffc7 ("bpf, 
> vsock: Invoke proto::close on close()"). Should I backport this commit 
> too?  There is no real dependency on the commit in the Fixes tag 
> ("vsock: support sockmap"). IMHO, this would help future backports, 
> because of indentation conficts! Otherwise I can simply fix the patches.  
> WDYT?

Just a note: since sockmap does not support AF_VSOCK in those kernels <=
6.1, backporting 135ffc7 would introduce a (no-op) callback function
vsock_close(), which would then be (unnecessarily) called on every
vsock_release().


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] vsock: fix use-after free and null-ptr-deref
  2025-02-17 19:45   ` Michal Luczaj
@ 2025-02-18  8:35     ` Stefano Garzarella
  2025-02-18 14:05       ` Michal Luczaj
  0 siblings, 1 reply; 8+ messages in thread
From: Stefano Garzarella @ 2025-02-18  8:35 UTC (permalink / raw)
  To: Michal Luczaj
  Cc: Luigi Leonardi, stable, Jakub Kicinski,
	syzbot+9d55b199192a4be7d02c

On Mon, Feb 17, 2025 at 08:45:57PM +0100, Michal Luczaj wrote:
>On 2/17/25 12:18, Luigi Leonardi wrote:
>> On Fri, Feb 14, 2025 at 06:53:54PM +0100, Luigi Leonardi wrote:
>>> Hi all,
>>>
>>> This series contains two patches that are already available upstream:
>>>
>>> - The first commit fixes a use-after-free[1], but introduced a
>>> null-ptr-deref[2].
>>> - The second commit fixes it. [3]
>>>
>>> I suggested waiting for both of them to be merged upstream and then
>>> applying them togheter to stable[4].
>>>
>>> It should be applied to:
>>> - 6.13.y
>>> - 6.12.y
>>> - 6.6.y
>>>
>>> I will send another series for
>>> - 6.1.y
>>> - 5.15.y
>>> - 5.10.y
>>>
>>> because of conflicts.
>>>
>>> [1]https://lore.kernel.org/all/20250128-vsock-transport-vs-autobind-v3-0-1cf57065b770@rbox.co/
>>> [2]https://lore.kernel.org/all/67a09300.050a0220.d7c5a.008b.GAE@google.com/
>>> [3]https://lore.kernel.org/all/20250210-vsock-linger-nullderef-v3-0-ef6244d02b54@rbox.co/
>>> [4]https://lore.kernel.org/all/2025020644-unwitting-scary-3c0d@gregkh/
>>>
>>> Thanks,
>>> Luigi
>>>
>>> ---
>>> Michal Luczaj (2):
>>>      vsock: Keep the binding until socket destruction
>>>      vsock: Orphan socket after transport release
>>>
>>> net/vmw_vsock/af_vsock.c | 12 +++++++++++-
>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>> ---
>>> base-commit: a1856aaa2ca74c88751f7d255dfa0c8c50fcc1ca
>>> change-id: 20250214-linux-rolling-stable-d73f0bed815d
>>>
>>> Best regards,
>>> -- Luigi Leonardi <leonardi@redhat.com>
>>>
>>
>> Looks like I forgot to add my SoB to the commits, my bad.
>>
>> For all the other stable trees (6.1, 5.15 and 5.10), there are some
>> conflicts due to some indentation changes introduced by 135ffc7 ("bpf,
>> vsock: Invoke proto::close on close()"). Should I backport this commit
>> too?  There is no real dependency on the commit in the Fixes tag
>> ("vsock: support sockmap"). IMHO, this would help future backports,
>> because of indentation conficts! Otherwise I can simply fix the patches.
>> WDYT?
>
>Just a note: since sockmap does not support AF_VSOCK in those kernels <=
>6.1, backporting 135ffc7 would introduce a (no-op) callback function
>vsock_close(), which would then be (unnecessarily) called on every
>vsock_release().
>

But this is the same behavior we have now upstream (without considering 
sockmap), right?

Do you see any potential problems?

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] vsock: fix use-after free and null-ptr-deref
  2025-02-18  8:35     ` Stefano Garzarella
@ 2025-02-18 14:05       ` Michal Luczaj
  2025-02-18 16:01         ` Stefano Garzarella
  0 siblings, 1 reply; 8+ messages in thread
From: Michal Luczaj @ 2025-02-18 14:05 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Luigi Leonardi, stable, Jakub Kicinski,
	syzbot+9d55b199192a4be7d02c

On 2/18/25 09:35, Stefano Garzarella wrote:
> On Mon, Feb 17, 2025 at 08:45:57PM +0100, Michal Luczaj wrote:
>> On 2/17/25 12:18, Luigi Leonardi wrote:
>>> On Fri, Feb 14, 2025 at 06:53:54PM +0100, Luigi Leonardi wrote:
>>>> Hi all,
>>>>
>>>> This series contains two patches that are already available upstream:
>>>>
>>>> - The first commit fixes a use-after-free[1], but introduced a
>>>> null-ptr-deref[2].
>>>> - The second commit fixes it. [3]
>>>>
>>>> I suggested waiting for both of them to be merged upstream and then
>>>> applying them togheter to stable[4].
>>>>
>>>> It should be applied to:
>>>> - 6.13.y
>>>> - 6.12.y
>>>> - 6.6.y
>>>>
>>>> I will send another series for
>>>> - 6.1.y
>>>> - 5.15.y
>>>> - 5.10.y
>>>>
>>>> because of conflicts.
>>>>
>>>> [1]https://lore.kernel.org/all/20250128-vsock-transport-vs-autobind-v3-0-1cf57065b770@rbox.co/
>>>> [2]https://lore.kernel.org/all/67a09300.050a0220.d7c5a.008b.GAE@google.com/
>>>> [3]https://lore.kernel.org/all/20250210-vsock-linger-nullderef-v3-0-ef6244d02b54@rbox.co/
>>>> [4]https://lore.kernel.org/all/2025020644-unwitting-scary-3c0d@gregkh/
>>>>
>>>> Thanks,
>>>> Luigi
>>>>
>>>> ---
>>>> Michal Luczaj (2):
>>>>      vsock: Keep the binding until socket destruction
>>>>      vsock: Orphan socket after transport release
>>>>
>>>> net/vmw_vsock/af_vsock.c | 12 +++++++++++-
>>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>>> ---
>>>> base-commit: a1856aaa2ca74c88751f7d255dfa0c8c50fcc1ca
>>>> change-id: 20250214-linux-rolling-stable-d73f0bed815d
>>>>
>>>> Best regards,
>>>> -- Luigi Leonardi <leonardi@redhat.com>
>>>>
>>>
>>> Looks like I forgot to add my SoB to the commits, my bad.
>>>
>>> For all the other stable trees (6.1, 5.15 and 5.10), there are some
>>> conflicts due to some indentation changes introduced by 135ffc7 ("bpf,
>>> vsock: Invoke proto::close on close()"). Should I backport this commit
>>> too?  There is no real dependency on the commit in the Fixes tag
>>> ("vsock: support sockmap"). IMHO, this would help future backports,
>>> because of indentation conficts! Otherwise I can simply fix the patches.
>>> WDYT?
>>
>> Just a note: since sockmap does not support AF_VSOCK in those kernels <=
>> 6.1, backporting 135ffc7 would introduce a (no-op) callback function
>> vsock_close(), which would then be (unnecessarily) called on every
>> vsock_release().
>>
> 
> But this is the same behavior we have now upstream (without considering 
> sockmap), right?

Oh, right, that's true.

> Do you see any potential problems?

No, nothing I can think of.

Note however that the comment above vsock_close() ("Dummy callback required
by sockmap. See unconditional call of saved_close() in sock_map_close().")
becomes somewhat misleading :)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] vsock: fix use-after free and null-ptr-deref
  2025-02-18 14:05       ` Michal Luczaj
@ 2025-02-18 16:01         ` Stefano Garzarella
  0 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2025-02-18 16:01 UTC (permalink / raw)
  To: Michal Luczaj
  Cc: Luigi Leonardi, stable, Jakub Kicinski,
	syzbot+9d55b199192a4be7d02c

On Tue, Feb 18, 2025 at 03:05:15PM +0100, Michal Luczaj wrote:
>On 2/18/25 09:35, Stefano Garzarella wrote:
>> On Mon, Feb 17, 2025 at 08:45:57PM +0100, Michal Luczaj wrote:
>>> On 2/17/25 12:18, Luigi Leonardi wrote:
>>>> On Fri, Feb 14, 2025 at 06:53:54PM +0100, Luigi Leonardi wrote:
>>>>> Hi all,
>>>>>
>>>>> This series contains two patches that are already available upstream:
>>>>>
>>>>> - The first commit fixes a use-after-free[1], but introduced a
>>>>> null-ptr-deref[2].
>>>>> - The second commit fixes it. [3]
>>>>>
>>>>> I suggested waiting for both of them to be merged upstream and then
>>>>> applying them togheter to stable[4].
>>>>>
>>>>> It should be applied to:
>>>>> - 6.13.y
>>>>> - 6.12.y
>>>>> - 6.6.y
>>>>>
>>>>> I will send another series for
>>>>> - 6.1.y
>>>>> - 5.15.y
>>>>> - 5.10.y
>>>>>
>>>>> because of conflicts.
>>>>>
>>>>> [1]https://lore.kernel.org/all/20250128-vsock-transport-vs-autobind-v3-0-1cf57065b770@rbox.co/
>>>>> [2]https://lore.kernel.org/all/67a09300.050a0220.d7c5a.008b.GAE@google.com/
>>>>> [3]https://lore.kernel.org/all/20250210-vsock-linger-nullderef-v3-0-ef6244d02b54@rbox.co/
>>>>> [4]https://lore.kernel.org/all/2025020644-unwitting-scary-3c0d@gregkh/
>>>>>
>>>>> Thanks,
>>>>> Luigi
>>>>>
>>>>> ---
>>>>> Michal Luczaj (2):
>>>>>      vsock: Keep the binding until socket destruction
>>>>>      vsock: Orphan socket after transport release
>>>>>
>>>>> net/vmw_vsock/af_vsock.c | 12 +++++++++++-
>>>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>>>> ---
>>>>> base-commit: a1856aaa2ca74c88751f7d255dfa0c8c50fcc1ca
>>>>> change-id: 20250214-linux-rolling-stable-d73f0bed815d
>>>>>
>>>>> Best regards,
>>>>> -- Luigi Leonardi <leonardi@redhat.com>
>>>>>
>>>>
>>>> Looks like I forgot to add my SoB to the commits, my bad.
>>>>
>>>> For all the other stable trees (6.1, 5.15 and 5.10), there are some
>>>> conflicts due to some indentation changes introduced by 135ffc7 ("bpf,
>>>> vsock: Invoke proto::close on close()"). Should I backport this commit
>>>> too?  There is no real dependency on the commit in the Fixes tag
>>>> ("vsock: support sockmap"). IMHO, this would help future backports,
>>>> because of indentation conficts! Otherwise I can simply fix the patches.
>>>> WDYT?
>>>
>>> Just a note: since sockmap does not support AF_VSOCK in those kernels <=
>>> 6.1, backporting 135ffc7 would introduce a (no-op) callback function
>>> vsock_close(), which would then be (unnecessarily) called on every
>>> vsock_release().
>>>
>>
>> But this is the same behavior we have now upstream (without considering
>> sockmap), right?
>
>Oh, right, that's true.
>
>> Do you see any potential problems?
>
>No, nothing I can think of.
>
>Note however that the comment above vsock_close() ("Dummy callback required
>by sockmap. See unconditional call of saved_close() in sock_map_close().")
>becomes somewhat misleading :)
>

Yeah, we can mention in the commit description of the backport that we 
backport it just to reduce conflicts but sockmap features are not 
backported. I'd touch as less as possibile in the patch, otherwise IMHO 
is better to just fix the conflicts in the 2 patches.

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-02-18 16:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-14 17:53 [PATCH 0/2] vsock: fix use-after free and null-ptr-deref Luigi Leonardi
2025-02-14 17:53 ` [PATCH 1/2] vsock: Keep the binding until socket destruction Luigi Leonardi
2025-02-14 17:53 ` [PATCH 2/2] vsock: Orphan socket after transport release Luigi Leonardi
2025-02-17 11:18 ` [PATCH 0/2] vsock: fix use-after free and null-ptr-deref Luigi Leonardi
2025-02-17 19:45   ` Michal Luczaj
2025-02-18  8:35     ` Stefano Garzarella
2025-02-18 14:05       ` Michal Luczaj
2025-02-18 16:01         ` Stefano Garzarella

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.