[v1] hv_sock: Return -EIO for malformed/short packets

[PATCH net] hv_sock: Return -EIO for malformed/short packets

Posted by Dexuan Cui 1 month, 4 weeks ago

Commit f63152958994 fixes a regression, however it fails to report an
error for malformed/short packets -- normally we should never see such
packets, but let's report an error for them just in case.

Fixes: f63152958994 ("hv_sock: Report EOF instead of -EIO for FIN")
Cc: stable@vger.kernel.org
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---

Commit f63152958994 is currently only in net.git's master branch.

 net/vmw_vsock/hyperv_transport.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 76e78c83fdbc..8faaa14bccda 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -704,18 +704,27 @@ static s64 hvs_stream_has_data(struct vsock_sock *vsk)
 		if (hvs->recv_desc) {
 			/* Here hvs->recv_data_len is 0, so hvs->recv_desc must
 			 * be NULL unless it points to the 0-byte-payload FIN
-			 * packet: see hvs_update_recv_data().
+			 * packet or a malformed/short packet: see
+			 * hvs_update_recv_data().
 			 *
-			 * Here all the payload has been dequeued, but
-			 * hvs_channel_readable_payload() still returns 1,
-			 * because the VMBus ringbuffer's read_index is not
-			 * updated for the FIN packet: hvs_stream_dequeue() ->
-			 * hv_pkt_iter_next() updates the cached priv_read_index
-			 * but has no opportunity to update the read_index in
-			 * hv_pkt_iter_close() as hvs_stream_has_data() returns
-			 * 0 for the FIN packet, so it won't get dequeued.
+			 * If hvs->recv_desc points to the FIN packet, here all
+			 * the payload has been dequeued and the peer_shutdown
+			 * flag is set, but hvs_channel_readable_payload() still
+			 * returns 1, because the VMBus ringbuffer's read_index
+			 * is not updated for the FIN packet:
+			 * hvs_stream_dequeue() -> hv_pkt_iter_next() updates
+			 * the cached priv_read_index but has no opportunity to
+			 * update the read_index in hv_pkt_iter_close() as
+			 * hvs_stream_has_data() returns 0 for the FIN packet,
+			 * so it won't get dequeued.
+			 *
+			 * In case hvs->recv_desc points to a malformed/short
+			 * packet, return -EIO.
 			 */
-			return 0;
+			if (hvs->vsk->peer_shutdown & SEND_SHUTDOWN)
+				return 0;
+			else
+				return -EIO;
 		}
 
 		hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
-- 
2.49.0

Re: [PATCH net] hv_sock: Return -EIO for malformed/short packets

Posted by Stefano Garzarella 1 month, 4 weeks ago

On Tue, Apr 21, 2026 at 10:49:31AM -0700, Dexuan Cui wrote:
>Commit f63152958994 fixes a regression, however it fails to report an
>error for malformed/short packets -- normally we should never see such
>packets, but let's report an error for them just in case.
>
>Fixes: f63152958994 ("hv_sock: Report EOF instead of -EIO for FIN")
>Cc: stable@vger.kernel.org
>Signed-off-by: Dexuan Cui <decui@microsoft.com>
>---
>
>Commit f63152958994 is currently only in net.git's master branch.
>
> net/vmw_vsock/hyperv_transport.c | 29 +++++++++++++++++++----------
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 76e78c83fdbc..8faaa14bccda 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -704,18 +704,27 @@ static s64 hvs_stream_has_data(struct vsock_sock *vsk)
> 		if (hvs->recv_desc) {
> 			/* Here hvs->recv_data_len is 0, so hvs->recv_desc must
> 			 * be NULL unless it points to the 0-byte-payload FIN
>-			 * packet: see hvs_update_recv_data().
>+			 * packet or a malformed/short packet: see
>+			 * hvs_update_recv_data().
> 			 *
>-			 * Here all the payload has been dequeued, but
>-			 * hvs_channel_readable_payload() still returns 1,
>-			 * because the VMBus ringbuffer's read_index is not
>-			 * updated for the FIN packet: hvs_stream_dequeue() ->
>-			 * hv_pkt_iter_next() updates the cached priv_read_index
>-			 * but has no opportunity to update the read_index in
>-			 * hv_pkt_iter_close() as hvs_stream_has_data() returns
>-			 * 0 for the FIN packet, so it won't get dequeued.
>+			 * If hvs->recv_desc points to the FIN packet, here all
>+			 * the payload has been dequeued and the peer_shutdown
>+			 * flag is set, but hvs_channel_readable_payload() still
>+			 * returns 1, because the VMBus ringbuffer's read_index
>+			 * is not updated for the FIN packet:
>+			 * hvs_stream_dequeue() -> hv_pkt_iter_next() updates
>+			 * the cached priv_read_index but has no opportunity to
>+			 * update the read_index in hv_pkt_iter_close() as
>+			 * hvs_stream_has_data() returns 0 for the FIN packet,
>+			 * so it won't get dequeued.
>+			 *
>+			 * In case hvs->recv_desc points to a malformed/short
>+			 * packet, return -EIO.
> 			 */
>-			return 0;
>+			if (hvs->vsk->peer_shutdown & SEND_SHUTDOWN)

We can access `vsk` directly, I mean `vsk->peer_shutdown`.

>+				return 0;
>+			else

nit: we usually avoid the `else` if the other branch returns early, and 
maybe have the error returned first, so it's more clear when reading the 
comment on top.  I mean something like this:

			if (!(vsk->peer_shutdown & SEND_SHUTDOWN))
				return -EIO;

			return 0;

BTW, not a strong opinion on that.

The rest, LGTM!

Thanks,
Stefano