netdev - 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: <b3fx4pma6l2zgfw7azvzliqfwu3ldjdlvbnegmzcuef5ryldux@2mbc5qsgkf45>
Date: Fri, 27 Jun 2025 10:10:23 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: Michal Luczaj <mhal@...x.co>
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 Wed, Jun 25, 2025 at 11:23:54PM +0200, Michal Luczaj wrote:
>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.

Yeah, BTW for transport_h2g is the same, they always should return 
VMADDR_CID_HOST, so I think we should be symmetrical.

Thanks,
Stefano