[PATCH net] vhost/vsock: trim nonlinear SKBs to declared payload length

[PATCH net] vhost/vsock: trim nonlinear SKBs to declared payload length

[Daehyeon Ko]

vhost_vsock_alloc_skb() sizes its skb from the total guest descriptor
length, while virtio_vsock_hdr.len independently declares the payload
length.  Since commit ab9aa2f3afc2 ("vhost/vsock: Allocate nonlinear SKBs
for handling large receive buffers"), large descriptors use skb fragments.

virtio_vsock_skb_put() currently changes only skb->len for a nonlinear
skb, leaving skb->data_len and all fragments attached.  The zero-payload
fast path skips the helper entirely.  A guest can therefore retain the
full descriptor allocation while receive credit accounts no payload.

On Linux v7.2, 455 zero-payload skbs retained 30,255,680 bytes on a
256 KiB receive buffer while rx_bytes and buf_used remained zero.  A
full-payload control retained 265,984 bytes in four skbs.  The existing
SKB_TRUESIZE(0) queue budget caps skb count but does not account for these
descriptor-sized fragments.

Set skb->len to the fragment length before calling pskb_trim(), then trim
to the declared payload length.  This releases unused fragments and lets
skb_condense() reduce truesize.  Move the zero-payload return after the
helper so zero-length packets are trimmed too.

These skbs are newly allocated, unique, and have no frag_list, so the trim
does not enter an allocation-bearing path.  Keep a warning for a violated
caller contract.

The fixed v7.2 image left a one-byte skb with no fragments and reduced
the zero-payload queue to one 960-byte skb.  The build had no compiler
warnings, and the run had no sanitizer, WARN, oops, or panic findings.

Fixes: ab9aa2f3afc2 ("vhost/vsock: Allocate nonlinear SKBs for handling large receive buffers")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
Required configuration is CONFIG_VSOCKETS, CONFIG_VIRTIO_VSOCKETS_COMMON,
and CONFIG_VHOST_VSOCK.

The source reproducer is available privately to maintainers and is
omitted from this public AI-assisted report. It exercises the real
allocation helper and VSOCK receive queue from an in-kernel module, not
a live guest virtqueue. Guest control is source-confirmed, but live guest
end-to-end validation and deliberate host OOM were not performed. No KASAN
splat is expected or claimed; the oracle is retained truesize and receive
credit state above.

 drivers/vhost/vsock.c        | 6 ++----
 include/linux/virtio_vsock.h | 9 ++++++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index abed1fbcf66c..fa59456abda9 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -400,10 +400,6 @@ vhost_vsock_alloc_skb(struct vhost_virtqueue *vq,
 
 	payload_len = le32_to_cpu(hdr->len);
 
-	/* No payload */
-	if (!payload_len)
-		return skb;
-
 	/* The pkt is too big or the length in the header is invalid */
 	if (payload_len + sizeof(*hdr) > len) {
 		kfree_skb(skb);
@@ -411,6 +407,8 @@ vhost_vsock_alloc_skb(struct vhost_virtqueue *vq,
 	}
 
 	virtio_vsock_skb_put(skb, payload_len);
+	if (!payload_len)
+		return skb;
 
 	if (skb_copy_datagram_from_iter(skb, 0, &iov_iter, payload_len)) {
 		vq_err(vq, "Failed to copy %zu byte payload\n", payload_len);
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index f91704731057..31358683e23e 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -51,10 +51,13 @@ static inline void virtio_vsock_skb_put(struct sk_buff *skb, u32 len)
 {
 	DEBUG_NET_WARN_ON_ONCE(skb->len);
 
-	if (skb_is_nonlinear(skb))
-		skb->len = len;
-	else
+	if (skb_is_nonlinear(skb)) {
+		skb->len = skb->data_len;
+		if (WARN_ON_ONCE(pskb_trim(skb, len)))
+			return;
+	} else {
 		skb_put(skb, len);
+	}
 }
 
 static inline struct sk_buff *

base-commit: 54518e0e827f4ca9229ae657022c60bf60f5c1bf
-- 
2.55.0

Re: [PATCH net] vhost/vsock: trim nonlinear SKBs to declared payload length

[Stefano Garzarella]

+Cc Will that touched this code recently

On Wed, Sep 30, 2026 at 01:41:45PM +0900, Daehyeon Ko wrote:
>vhost_vsock_alloc_skb() sizes its skb from the total guest descriptor
>length, while virtio_vsock_hdr.len independently declares the payload
>length.  Since commit ab9aa2f3afc2 ("vhost/vsock: Allocate nonlinear SKBs
>for handling large receive buffers"), large descriptors use skb fragments.
>
>virtio_vsock_skb_put() currently changes only skb->len for a nonlinear
>skb, leaving skb->data_len and all fragments attached.  The zero-payload
>fast path skips the helper entirely.  A guest can therefore retain the
>full descriptor allocation while receive credit accounts no payload.
>
>On Linux v7.2, 455 zero-payload skbs retained 30,255,680 bytes on a
>256 KiB receive buffer while rx_bytes and buf_used remained zero.  A
>full-payload control retained 265,984 bytes in four skbs.  The existing
>SKB_TRUESIZE(0) queue budget caps skb count but does not account for these
>descriptor-sized fragments.
>
>Set skb->len to the fragment length before calling pskb_trim(), then trim
>to the declared payload length.  This releases unused fragments and lets
>skb_condense() reduce truesize.  Move the zero-payload return after the
>helper so zero-length packets are trimmed too.
>
>These skbs are newly allocated, unique, and have no frag_list, so the trim
>does not enter an allocation-bearing path.  Keep a warning for a violated
>caller contract.
>
>The fixed v7.2 image left a one-byte skb with no fragments and reduced
>the zero-payload queue to one 960-byte skb.  The build had no compiler
>warnings, and the run had no sanitizer, WARN, oops, or panic findings.

I think this is a requirement for every patch sent, no?
Why putting in the commit message?

>
>Fixes: ab9aa2f3afc2 ("vhost/vsock: Allocate nonlinear SKBs for handling large receive buffers")
>Cc: stable@vger.kernel.org
>Assisted-by: LLM
>Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
>---
>Required configuration is CONFIG_VSOCKETS, CONFIG_VIRTIO_VSOCKETS_COMMON,
>and CONFIG_VHOST_VSOCK.
>
>The source reproducer is available privately to maintainers and is
>omitted from this public AI-assisted report. It exercises the real
>allocation helper and VSOCK receive queue from an in-kernel module, not
>a live guest virtqueue. Guest control is source-confirmed, but live guest
>end-to-end validation and deliberate host OOM were not performed. No KASAN
>splat is expected or claimed; the oracle is retained truesize and receive
>credit state above.
>
> drivers/vhost/vsock.c        | 6 ++----
> include/linux/virtio_vsock.h | 9 ++++++---
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index abed1fbcf66c..fa59456abda9 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -400,10 +400,6 @@ vhost_vsock_alloc_skb(struct vhost_virtqueue *vq,
>
> 	payload_len = le32_to_cpu(hdr->len);
>
>-	/* No payload */
>-	if (!payload_len)
>-		return skb;
>-
> 	/* The pkt is too big or the length in the header is invalid */
> 	if (payload_len + sizeof(*hdr) > len) {
> 		kfree_skb(skb);
>@@ -411,6 +407,8 @@ vhost_vsock_alloc_skb(struct vhost_virtqueue *vq,
> 	}
>
> 	virtio_vsock_skb_put(skb, payload_len);
>+	if (!payload_len)
>+		return skb;
>
> 	if (skb_copy_datagram_from_iter(skb, 0, &iov_iter, payload_len)) {
> 		vq_err(vq, "Failed to copy %zu byte payload\n", payload_len);
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index f91704731057..31358683e23e 100644
>--- a/include/linux/virtio_vsock.h
>+++ b/include/linux/virtio_vsock.h
>@@ -51,10 +51,13 @@ static inline void virtio_vsock_skb_put(struct sk_buff *skb, u32 len)
> {
> 	DEBUG_NET_WARN_ON_ONCE(skb->len);
>
>-	if (skb_is_nonlinear(skb))
>-		skb->len = len;
>-	else
>+	if (skb_is_nonlinear(skb)) {

Would be nice to have a comment here to explain better the reason we are 
doing this.

Thanks,
Stefano

>+		skb->len = skb->data_len;
>+		if (WARN_ON_ONCE(pskb_trim(skb, len)))
>+			return;
>+	} else {
> 		skb_put(skb, len);
>+	}
> }
>
> static inline struct sk_buff *
>
>base-commit: 54518e0e827f4ca9229ae657022c60bf60f5c1bf
>-- 
>2.55.0
>

Re: [PATCH net] vhost/vsock: trim nonlinear SKBs to declared payload length

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete.
- [Medium] On 32-bit hosts, the check `payload_len + sizeof(*hdr) > len`
  in vhost_vsock_alloc_skb() (drivers/vhost/vsock.c:404) can wrap, and…

--

vhost/vsock: trim nonlinear SKBs to declared payload length

This commit changes virtio_vsock_skb_put() so that a nonlinear skb is
trimmed with pskb_trim() to the payload length declared in hdr->len.
Previously only skb->len was adjusted. The zero-payload early return in
vhost_vsock_alloc_skb() now comes after the helper call, so zero-length
packets are trimmed as well.

> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index abed1fbcf66cc..fa59456abda97 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -400,10 +400,6 @@ vhost_vsock_alloc_skb(struct vhost_virtqueue *vq,
>  
>  	payload_len = le32_to_cpu(hdr->len);
>  
> -	/* No payload */
> -	if (!payload_len)
> -		return skb;
> -
>  	/* The pkt is too big or the length in the header is invalid */
>  	if (payload_len + sizeof(*hdr) > len) {

[Severity: Medium]
Can this check wrap on 32-bit hosts? payload_len is a size_t that comes
from the guest-controlled hdr->len. A value from 0xFFFFFFD4 to 0xFFFFFFFF
makes payload_len + sizeof(*hdr) wrap to 0..43. Since len is at least
VIRTIO_VSOCK_SKB_HEADROOM, the check always passes.

Before this patch, the nonlinear branch set skb->len to that huge value.
skb_copy_datagram_from_iter() then skipped every fragment and returned
-EFAULT without a warning.

With the new virtio_vsock_skb_put(), skb->len is set to skb->data_len.
pskb_trim(skb, huge) then does nothing, because len < skb->len is false.
The skb reaches this call with headlen 0 and a negative int length (for
example -32):

vhost_vsock_alloc_skb()
    skb_copy_datagram_from_iter(skb, 0, &iov_iter, payload_len)

In the frag loop of skb_copy_datagram_from_iter(), this evaluates
0 > -32 and fires:

    WARN_ON(start > offset + len);

copy_page_from_iter() is then called with bytes = (size_t)-32, which
trips the check in page_copy_sane():

    if (WARN_ON(n > v || v > page_size(head)))

Neither warning is a once-only warning. A guest that posts TX descriptors
larger than about 32.5 KiB with such hdr->len values would get two host
WARN splats per packet. With panic_on_warn set, it would panic the host.

The guest-side virtio_transport_rx_work() already uses an overflow-safe
form of this check. Would it make sense to use it here as well?

    if (unlikely(payload_len > len - sizeof(*hdr))) {

[ ... ]

> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index f91704731057e..31358683e23e3 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -51,10 +51,13 @@ static inline void virtio_vsock_skb_put(struct sk_buff *skb, u32 len)
>  {
>  	DEBUG_NET_WARN_ON_ONCE(skb->len);
>  
> -	if (skb_is_nonlinear(skb))
> -		skb->len = len;
> -	else
> +	if (skb_is_nonlinear(skb)) {
> +		skb->len = skb->data_len;
> +		if (WARN_ON_ONCE(pskb_trim(skb, len)))
> +			return;
> +	} else {
>  		skb_put(skb, len);
> +	}
>  }

[Severity: High]
Does this fully fix the retention described in the commit message?
vhost_vsock_alloc_skb() still sizes the skb from iov_length(), not from
hdr->len:

    skb = virtio_vsock_alloc_skb(len, GFP_KERNEL);

The trim here only frees memory when the whole payload fits in the head
tailroom. Two cases seem to remain open.

The first case is the linear path. virtio_vsock_alloc_skb() gives any
descriptor up to about 32448 bytes a linear head of the full descriptor
size:

    if (size <= SKB_WITH_OVERHEAD(PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER))
            return virtio_vsock_alloc_linear_skb(size, mask);

The else branch above still just calls skb_put(skb, len). A zero or small
payload therefore keeps the whole ~32 KiB head.

The second case is the nonlinear path with a payload larger than the head
tailroom. That is a few hundred bytes; the 960-byte truesize in the commit
message suggests about 340. ___pskb_trim() shortens frag[0] and unrefs
only the frags after it:

    skb_frag_size_set(&skb_shinfo(skb)->frags[i++], len - offset);

    drop_pages:
    skb_shinfo(skb)->nr_frags = i;

    for (; i < nfrags; i++)
            skb_frag_unref(skb, i);

The order-3 page behind frag[0] stays pinned. skb_condense() then returns
early, so truesize is not recomputed:

    if (skb->data_len > skb->end - skb->tail ||
        skb_cloned(skb) || !skb_frags_readable(skb))
            return;

virtio_transport_inc_rx_pkt() charges only hdr->len plus this per-skb
overhead:

    u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);

For a 256 KiB buf_alloc, that admits about 455 skbs.

The merge and collapse paths don't seem to reclaim these skbs either:

- virtio_transport_can_collapse() refuses any skb with
  VIRTIO_VSOCK_SEQ_EOM set, and the guest controls hdr->flags.
- The GOOD_COPY_LEN merge in virtio_transport_recv_enqueue() skips
  nonlinear skbs, payloads over 128 bytes, and any skb after an EOM skb.
- Collapse only runs once queue_len reaches queue_max.

So one socket could still pin roughly 455 x ~33 KiB, about 15 MB.
Meanwhile buf_used stays at zero (linear, zero payload, EOM) or about
155 KB (nonlinear, 341-byte payloads). A guest can repeat this across
connections.

The commit message says:

  "Set skb->len to the fragment length before calling pskb_trim(), then
  trim to the declared payload length.  This releases unused fragments
  and lets skb_condense() reduce truesize."

Is that accurate for nonlinear skbs whose payload is larger than the head
tailroom?

Would it work better to read and validate hdr->len first and then allocate
payload_len + VIRTIO_VSOCK_SKB_HEADROOM? Copying into a right-sized skb,
or charging the memory actually retained, are other options.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930044147.3818241-1-4ncienth%40gmail.com

Re: [PATCH net] vhost/vsock: trim nonlinear SKBs to declared payload length

[Daehyeon Ko]

Both findings are valid.  V2 validates hdr->len before allocation and sizes
the skb from the declared payload, which also avoids the 32-bit addition
overflow.  I am posting it as a separate thread per netdev policy.  Thank you
for the review.

pw-bot: cr