linux-kernel - Re: [PATCH net-next v3 1/3] vsock: Add support for SIOCINQ ioctl

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20250626050219.1847316-1-niuxuewei.nxw@antgroup.com>
Date: Thu, 26 Jun 2025 13:02:19 +0800
From: Xuewei Niu <niuxuewei97@...il.com>
To: sgarzare@...hat.com
Cc: davem@...emloft.net,
	decui@...rosoft.com,
	fupan.lfp@...group.com,
	haiyangz@...rosoft.com,
	jasowang@...hat.com,
	kvm@...r.kernel.org,
	kys@...rosoft.com,
	leonardi@...hat.com,
	linux-hyperv@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	mst@...hat.com,
	netdev@...r.kernel.org,
	niuxuewei.nxw@...group.com,
	niuxuewei97@...il.com,
	pabeni@...hat.com,
	stefanha@...hat.com,
	virtualization@...ts.linux.dev,
	wei.liu@...nel.org,
	xuanzhuo@...ux.alibaba.com
Subject: Re: [PATCH net-next v3 1/3] vsock: Add support for SIOCINQ ioctl

> On Wed, Jun 25, 2025 at 08:03:00AM +0000, Dexuan Cui wrote:
> >> From: Stefano Garzarella <sgarzare@...hat.com>
> >> Sent: Tuesday, June 17, 2025 7:39 AM
> >>  ...
> >> Now looks better to me, I just checked transports: vmci and virtio/vhost
> >> returns what we want, but for hyperv we have:
> >>
> >> 	static s64 hvs_stream_has_data(struct vsock_sock *vsk)
> >> 	{
> >> 		struct hvsock *hvs = vsk->trans;
> >> 		s64 ret;
> >>
> >> 		if (hvs->recv_data_len > 0)
> >> 			return 1;
> >>
> >> @Hyper-v maintainers: do you know why we don't return `recv_data_len`?
> >
> >Sorry for the late response!  This is the complete code of the function:
> >
> >static s64 hvs_stream_has_data(struct vsock_sock *vsk)
> >{
> >        struct hvsock *hvs = vsk->trans;
> >        s64 ret;
> >
> >        if (hvs->recv_data_len > 0)
> >                return 1;
> >
> >        switch (hvs_channel_readable_payload(hvs->chan)) {
> >        case 1:
> >                ret = 1;
> >                break;
> >        case 0:
> >                vsk->peer_shutdown |= SEND_SHUTDOWN;
> >                ret = 0;
> >                break;
> >        default: /* -1 */
> >                ret = 0;
> >                break;
> >        }
> >
> >        return ret;
> >}
> >
> >If (hvs->recv_data_len > 0), I think we can return hvs->recv_data_len here.
> >
> >If hvs->recv_data_len is 0, and hvs_channel_readable_payload(hvs->chan)
> >returns 1, we should not return hvs->recv_data_len (which is 0 here), 
> >and it's
> >not very easy to find how many bytes of payload in total is available right now:
> >each host-to-guest "packet" in the VMBus channel ringbuffer has a header
> >(which is not part of the payload data) and a trailing padding field, and we
> >would have to iterate on all the "packets" (or at least the next
> >"packet"?) to find the exact bytes of pending payload. Please see
> >hvs_stream_dequeue() for details.
> >
> >Ideally hvs_stream_has_data() should return the exact length of pending
> >readable payload, but when the hv_sock code was written in 2017,
> >vsock_stream_has_data() -> ... -> hvs_stream_has_data() basically only needs
> >to know whether there is any data or not, i.e. it's kind of a boolean variable, so
> >hvs_stream_has_data() was written to return 1 or 0 for simplicity. :-)
> 
> Yeah, I see, thanks for the details! :-)
> 
> >
> >I can post the patch below (not tested yet) to fix hvs_stream_has_data() by
> >returning the payload length of the next single "packet".  Does it look good
> >to you?
> 
> Yep, LGTM! Can be a best effort IMO.
> 
> Maybe when you have it tested, post it here as proper patch, and Xuewei 
> can include it in the next version of this series (of course with you as 
> author, etc.). In this way will be easy to test/merge, since they are 
> related.
> 
> @Xuewei @Dexuan Is it okay for you?

Yeah, sounds good to me!

Thanks,
Xuewei

> Thanks,
> Stefano
> 
> >
> >--- a/net/vmw_vsock/hyperv_transport.c
> >+++ b/net/vmw_vsock/hyperv_transport.c
> >@@ -694,15 +694,25 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
> > static s64 hvs_stream_has_data(struct vsock_sock *vsk)
> > {
> >        struct hvsock *hvs = vsk->trans;
> >+       bool need_refill = !hvs->recv_desc;
> >        s64 ret;
> >
> >        if (hvs->recv_data_len > 0)
> >-               return 1;
> >+               return hvs->recv_data_len;
> >
> >        switch (hvs_channel_readable_payload(hvs->chan)) {
> >        case 1:
> >-               ret = 1;
> >-               break;
> >+               if (!need_refill)
> >+                       return -EIO;
> >+
> >+               hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
> >+               if (!hvs->recv_desc)
> >+                       return -ENOBUFS;
> >+
> >+               ret = hvs_update_recv_data(hvs);
> >+               if (ret)
> >+                       return ret;
> >+               return hvs->recv_data_len;
> >        case 0:
> >                vsk->peer_shutdown |= SEND_SHUTDOWN;
> >                ret = 0;
> >
> >Thanks,
> >Dexuan