linux-kernel - RE: [EXTERNAL] 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID:
 <BL1PR21MB31158AE6980AF18E769A4E65BF7BA@BL1PR21MB3115.namprd21.prod.outlook.com>
Date: Wed, 25 Jun 2025 08:03:00 +0000
From: Dexuan Cui <decui@...rosoft.com>
To: Stefano Garzarella <sgarzare@...hat.com>, Xuewei Niu
	<niuxuewei97@...il.com>, KY Srinivasan <kys@...rosoft.com>, Haiyang Zhang
	<haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>
CC: "mst@...hat.com" <mst@...hat.com>, "pabeni@...hat.com"
	<pabeni@...hat.com>, "jasowang@...hat.com" <jasowang@...hat.com>,
	"xuanzhuo@...ux.alibaba.com" <xuanzhuo@...ux.alibaba.com>,
	"davem@...emloft.net" <davem@...emloft.net>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "stefanha@...hat.com" <stefanha@...hat.com>,
	"leonardi@...hat.com" <leonardi@...hat.com>, "virtualization@...ts.linux.dev"
	<virtualization@...ts.linux.dev>, "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "fupan.lfp@...group.com"
	<fupan.lfp@...group.com>, Xuewei Niu <niuxuewei.nxw@...group.com>
Subject: RE: [EXTERNAL] Re: [PATCH net-next v3 1/3] vsock: Add support for
 SIOCINQ ioctl

> 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. :-)

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?

--- 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