[PATCH net] vsock: account only unread bytes in read_skb()

[PATCH net] vsock: account only unread bytes in read_skb()

[Daehyeon Ko]

After a partial stream receive, rx_bytes tracks the unread suffix while
buf_used and peer credit still cover the whole packet.
virtio_transport_read_skb() dequeues that packet but passes pkt_len for
both counters, underflowing rx_bytes.

On a connected CID_LOCAL stream with an SK_SKB verdict, a 40-byte read
from a 100-byte packet made SIOCINQ report -40, an empty recv return
ELOOP, and poll report the empty socket readable.  After verdict detach,
receiving 40 bytes wrapped the counter to zero and hid those queued bytes
until one more byte arrived.  The fixed run kept the counter balanced
and exposed all queued data immediately.

Subtract only skb->len minus the VSOCK offset from rx_bytes.  Retain the
full packet length for buf_used and peer credit.

Fixes: 45ca7e9f0730 ("vsock/virtio: fix `rx_bytes` accounting for stream sockets")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
 net/vmw_vsock/virtio_transport_common.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index f225f53ed4bab..1e762a480df49 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1927,6 +1927,7 @@ int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t recv_acto
 	struct sock *sk = sk_vsock(vsk);
 	struct virtio_vsock_hdr *hdr;
 	struct sk_buff *skb;
+	u32 bytes_read;
 	u32 pkt_len;
 	int off = 0;
 	int err;
@@ -1946,7 +1947,8 @@ int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t recv_acto
 		vvs->msg_count--;
 
 	pkt_len = le32_to_cpu(hdr->len);
-	virtio_transport_dec_rx_pkt(vvs, pkt_len, pkt_len);
+	bytes_read = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
+	virtio_transport_dec_rx_pkt(vvs, bytes_read, pkt_len);
 	spin_unlock_bh(&vvs->rx_lock);
 
 	virtio_transport_send_credit_update(vsk);

base-commit: e23a64eb244356ee47c0620f0722d51bd88db522
-- 
2.55.0

Re: [PATCH net] vsock: account only unread bytes in read_skb()

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] vsock: account only unread bytes in read_skb()

[Daehyeon Ko]

Thanks. I found this during an LLM-assisted manual source audit of VSOCK
receive accounting while re-evaluating virtio_transport_read_skb() after
CVE-2024-50169. I then confirmed it on vulnerable and fixed kernels using a
connected CID_LOCAL stream and a real SK_SKB/SOCKMAP verdict path.

Thanks,
Daehyeon

Re: [PATCH net] vsock: account only unread bytes in read_skb()

[Luigi Leonardi]

Hi,

what about using `vsock/virtio` instead of `vsock`? We are changing
just the virtio parts of vsock.

On Thu, Oct 01, 2026 at 09:15:41PM +0900, Daehyeon Ko wrote:
>After a partial stream receive, rx_bytes tracks the unread suffix while
>buf_used and peer credit still cover the whole packet.
>virtio_transport_read_skb() dequeues that packet but passes pkt_len for
>both counters, underflowing rx_bytes.
>
>On a connected CID_LOCAL stream with an SK_SKB verdict, a 40-byte read
>from a 100-byte packet made SIOCINQ report -40, an empty recv return
>ELOOP, and poll report the empty socket readable.  After verdict detach,
>receiving 40 bytes wrapped the counter to zero and hid those queued bytes
>until one more byte arrived.  The fixed run kept the counter balanced
>and exposed all queued data immediately.
>
>Subtract only skb->len minus the VSOCK offset from rx_bytes.  Retain the
>full packet length for buf_used and peer credit.
>
>Fixes: 45ca7e9f0730 ("vsock/virtio: fix `rx_bytes` accounting for stream sockets")
>Cc: stable@vger.kernel.org
>Assisted-by: LLM
>Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index f225f53ed4bab..1e762a480df49 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1927,6 +1927,7 @@ int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t recv_acto
> 	struct sock *sk = sk_vsock(vsk);
> 	struct virtio_vsock_hdr *hdr;
> 	struct sk_buff *skb;
>+	u32 bytes_read;
> 	u32 pkt_len;
> 	int off = 0;
> 	int err;
>@@ -1946,7 +1947,8 @@ int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t recv_acto
> 		vvs->msg_count--;
>
> 	pkt_len = le32_to_cpu(hdr->len);
>-	virtio_transport_dec_rx_pkt(vvs, pkt_len, pkt_len);

maybe it's worth adding a comment? not a strong opinion.

>+	bytes_read = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
>+	virtio_transport_dec_rx_pkt(vvs, bytes_read, pkt_len);
> 	spin_unlock_bh(&vvs->rx_lock);
>
> 	virtio_transport_send_credit_update(vsk);
>
>base-commit: e23a64eb244356ee47c0620f0722d51bd88db522
>-- 
>2.55.0
>

code LGTM!

Reviewed-by: Luigi Leonardi <leonardi@redhat.com>

Thanks,
Luigi