linux-kernel - Re: [RESEND PATCH net-next v4 2/4] hv_sock: Return the readable bytes in hvs_stream_has

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: <mofyjvpvlrh75sfu7c7pi4ea6p5nkatkqqtnwpwne7uuhhl5ms@gaqcs3m6i6kx>
Date: Wed, 2 Jul 2025 11:58:55 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: Xuewei Niu <niuxuewei97@...il.com>
Cc: mst@...hat.com, pabeni@...hat.com, jasowang@...hat.com, 
	xuanzhuo@...ux.alibaba.com, davem@...emloft.net, netdev@...r.kernel.org, stefanha@...hat.com, 
	leonardi@...hat.com, decui@...rosoft.com, virtualization@...ts.linux.dev, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org, fupan.lfp@...group.com, 
	Xuewei Niu <niuxuewei.nxw@...group.com>
Subject: Re: [RESEND PATCH net-next v4 2/4] hv_sock: Return the readable
 bytes in hvs_stream_has_data()

On Mon, Jun 30, 2025 at 03:57:25PM +0800, Xuewei Niu wrote:

IMO here you should not reset the author to you, but you should keep
Dexuan as authour of this patch.

>When hv_sock was originally added, __vsock_stream_recvmsg() and
>vsock_stream_has_data() actually only needed to know whether there
>is any readable data or not, so hvs_stream_has_data() was written to
>return 1 or 0 for simplicity.
>
>However, now hvs_stream_has_data() should return the readable bytes
>because vsock_data_ready() -> vsock_stream_has_data() needs to know the
>actual bytes rather than a boolean value of 1 or 0.
>
>The SIOCINQ ioctl support also needs hvs_stream_has_data() to return
>the readable bytes.
>
>Let hvs_stream_has_data() return the readable bytes of the payload in
>the next host-to-guest VMBus hv_sock packet.
>
>Note: there may be multpile incoming hv_sock packets pending in the
>VMBus channel's ringbuffer, but so far there is not a VMBus API that
>allows us to know all the readable bytes in total without reading and
>caching the payload of the multiple packets, so let's just return the
>readable bytes of the next single packet. In the future, we'll either
>add a VMBus API that allows us to know the total readable bytes without
>touching the data in the ringbuffer, or the hv_sock driver needs to
>understand the VMBus packet format and parse the packets directly.
>
>Signed-off-by: Dexuan Cui <decui@...rosoft.com>
>Signed-off-by: Xuewei Niu <niuxuewei.nxw@...group.com>
>---
> net/vmw_vsock/hyperv_transport.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 31342ab502b4..64f1290a9ae7 100644
>--- 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;

For v5 remember to fix this as Paolo suggested. Dexuan proposed a fix on 
his thread.

Stefano

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