linux-kernel - Re: [PATCH net v3 2/3] vsock: Fix transport

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e69847a1-7a54-417d-95df-4c0a98a6120f@rbox.co>
Date: Thu, 3 Jul 2025 10:36:22 +0200
From: Michal Luczaj <mhal@...x.co>
To: Stefano Garzarella <sgarzare@...hat.com>
Cc: "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
 Stefan Hajnoczi <stefanha@...hat.com>, virtualization@...ts.linux.dev,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v3 2/3] vsock: Fix transport_* TOCTOU

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@...hat.com>
>> Signed-off-by: Michal Luczaj <mhal@...x.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