linux-kernel - Re: [PATCH RFC net v2 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `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: <0adde2db-3a75-4ade-a1c8-8c3bc2a0b1fd@rbox.co>
Date: Wed, 25 Jun 2025 23:23:54 +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 RFC net v2 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID
 to check also `transport_local`

On 6/25/25 10:54, Stefano Garzarella wrote:
> On Fri, Jun 20, 2025 at 09:52: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@...hat.com>
>> Signed-off-by: Michal Luczaj <mhal@...x.co>
>> ---
>> man vsock(7) mentions IOCTL_VM_SOCKETS_GET_LOCAL_CID vs. VMADDR_CID_LOCAL:
>>
>>   Ioctls
>>       ...
>>       IOCTL_VM_SOCKETS_GET_LOCAL_CID
>>              ...
>>              Consider using VMADDR_CID_ANY when binding instead of
>>              getting the local CID with IOCTL_VM_SOCKETS_GET_LOCAL_CID.
>>
>>   Local communication
>>       ....
>>       The local CID obtained with IOCTL_VM_SOCKETS_GET_LOCAL_CID can be
>>       used for the same purpose, but it is preferable to use
>>       VMADDR_CID_LOCAL.
>>
>> I was wondering it that would need some rewriting, since we're adding
>> VMADDR_CID_LOCAL as a possible ioctl's return value.
> 
> IIRC the reason was, that if we have for example a G2H module loaded, 
> the ioctl returns the CID of that module (e.g. 42). So, we can use both 
> 42 and VMADDR_CID_LOCAL to do the loopback communication, but we 
> encourage to always use VMADDR_CID_LOCAL.  With this change we basically 
> don't change that, but we change the fact that if there is only the 
> loopback module loaded, before the ioctl returned VMADDR_CID_ANY, while 
> now it returns LOCAL rightly.
> 
> So, IMO we are fine.

All right, got it.

>> ---
>> 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 a1b1073a2c89f865fcdb58b38d8e7feffcf1544f..4bdb4016bd14d790f3d217d5063be64a1553b194 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -2577,6 +2577,8 @@ static long vsock_dev_do_ioctl(struct file *filp,
>> 		cid = vsock_transport_local_cid(&transport_g2h);
>> 		if (cid == VMADDR_CID_ANY)
>> 			cid = vsock_transport_local_cid(&transport_h2g);
>> +		if (cid == VMADDR_CID_ANY && transport_local)
>> +			cid = VMADDR_CID_LOCAL;
> 
> why not `cid = vsock_transport_local_cid(&transport_local)` like for 
> H2G?

Sure, can do. I've assumed transport_local would always have a local CID of
VMADDR_CID_LOCAL. So taking mutex and going through a callback function to
get VMADDR_CID_LOCAL seemed superfluous. But I get it, if you want to have
it symmetrical with the other vsock_transport_local_cid()s.

Thanks,
Michal