netdev - Re: [PATCH net 4/5] vsock/test: Add test for UAF due to socket unbinding

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: <c50c2bcb-2416-4d72-b80f-3882b811b273@rbox.co>
Date: Tue, 21 Jan 2025 15:47:44 +0100
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>,
 George Zhang <georgezhang@...are.com>, Dmitry Torokhov <dtor@...are.com>,
 Andy King <acking@...are.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net 4/5] vsock/test: Add test for UAF due to socket
 unbinding

On 1/20/25 11:20, Stefano Garzarella wrote:
> On Fri, Jan 17, 2025 at 10:59:44PM +0100, Michal Luczaj wrote:
>> +#define MAX_PORT_RETRIES	24	/* net/vmw_vsock/af_vsock.c */
>> +#define VMADDR_CID_NONEXISTING	42
> 
> I would suggest a slightly bigger and weirder CID, this might actually
> exist, (e.g. 4242424242)

Ok. I'll try to avoid the whole VMADDR_CID_NONEXISTING. See below.

>> +/* Test attempts to trigger a transport release for an unbound socket. This can
>> + * lead to a reference count mishandling.
>> + */
>> +static void test_seqpacket_transport_uaf_client(const struct test_opts *opts)
>> +{
>> +	int sockets[MAX_PORT_RETRIES];
>> +	struct sockaddr_vm addr;
>> +	int s, i, alen;
>> +
>> +	s = vsock_bind(VMADDR_CID_LOCAL, VMADDR_PORT_ANY, SOCK_SEQPACKET);
> 
> In my suite this test failed because I have instances where I run it
> without vsock_loopback loaded.
> 
> 26 - connectible transport release use-after-free...bind: Cannot assign requested address
>
> Is it important to use VMADDR_CID_LOCAL or can we use VMADDR_CID_ANY?

To force the transport release on a live socket (via transport
reassignment) we need to switch between two transports (but it's okay if
the other transport turns out to be NULL). So, considering your setup(s),
is it reasonable to expect VMADDR_CID_ANY and VMADDR_CID_HOST bringing in
different transports (as in: different pointers)? Any other combination?

I've tried that in v2:
https://lore.kernel.org/netdev/20250121-vsock-transport-vs-autobind-v2-0-aad6069a4e8c@rbox.co

>> +	/* Vulnerable system may crash now. */
>> +	bind(s, (struct sockaddr *)&addr, alen);
> 
> Should we check the return of this function or do we not care whether
> it fails or not?

We don't really care, but I guess it just looks bad. I'll fix that.

Thanks,
Michal