[PATCH net v2] vhost/vsock: size receive SKBs from declared payload

[PATCH net v2] vhost/vsock: size receive SKBs from declared payload

[Daehyeon Ko]

vhost_vsock_alloc_skb() allocates an skb from the total guest descriptor
length before reading hdr->len.  Descriptor capacity and declared payload
length are independent, so a guest can supply a large descriptor with a
zero or short payload.

On Linux v7.2, 455 zero-payload packets with 64 KiB descriptors retained
30,255,680 bytes on a 256 KiB receive buffer while rx_bytes and buf_used
stayed zero.  A full-payload control retained 265,984 bytes.  The existing
SKB_TRUESIZE(0) budget caps skb count but does not account for the
descriptor-sized allocation.

Repeating this across connections can exhaust host kernel memory.

Trimming after allocation is insufficient.  Linear skbs retain their full
head, while a nonlinear payload exceeding head tailroom can retain its
first page fragment.

Copy the header into a stack object, validate hdr->len before allocating,
and size the skb from the declared payload plus the header.  This keeps the
allocation proportional to receive accounting for both linear and
nonlinear skbs.

Use payload_len > len - sizeof(hdr) for validation.  This avoids addition
overflow on 32-bit hosts and ensures payload_len fits the subsequent
int-length copy path.

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>
---
Changes in v2:
- validate the header before allocation with an overflow-safe bounds check;
- size the skb from declared payload and remove the global trim-helper
  change;
- remove the generic clean-validation statement from the changelog; and
- add Will Deacon to Cc.

v1: https://lore.kernel.org/netdev/20260930044147.3818241-1-4ncienth@gmail.com/

Built and booted on v7.2 with KASAN, UBSAN and LOCKDEP.  A focused
allocation/queue probe retained 436,800 and 582,400 bytes for 455 zero and
341-byte packets, versus 30,255,680 bytes for the vulnerable zero case.  No
KASAN report, WARN, oops or panic occurred.  The probe injects skbs
in-kernel rather than through a live guest virtqueue.
 drivers/vhost/vsock.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index abed1fbcf66cc5..35f75e23c7497e 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -364,7 +364,7 @@ static struct sk_buff *
 vhost_vsock_alloc_skb(struct vhost_virtqueue *vq,
 		      unsigned int out, unsigned int in)
 {
-	struct virtio_vsock_hdr *hdr;
+	struct virtio_vsock_hdr hdr;
 	struct iov_iter iov_iter;
 	struct sk_buff *skb;
 	size_t payload_len;
@@ -382,34 +382,32 @@ vhost_vsock_alloc_skb(struct vhost_virtqueue *vq,
 	    len > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE + VIRTIO_VSOCK_SKB_HEADROOM)
 		return NULL;
 
-	/* len contains both payload and hdr */
-	skb = virtio_vsock_alloc_skb(len, GFP_KERNEL);
-	if (!skb)
-		return NULL;
-
 	iov_iter_init(&iov_iter, ITER_SOURCE, vq->iov, out, len);
 
-	hdr = virtio_vsock_hdr(skb);
-	nbytes = copy_from_iter(hdr, sizeof(*hdr), &iov_iter);
-	if (nbytes != sizeof(*hdr)) {
+	nbytes = copy_from_iter(&hdr, sizeof(hdr), &iov_iter);
+	if (nbytes != sizeof(hdr)) {
 		vq_err(vq, "Expected %zu bytes for pkt->hdr, got %zu bytes\n",
-		       sizeof(*hdr), nbytes);
-		kfree_skb(skb);
+		       sizeof(hdr), nbytes);
 		return NULL;
 	}
 
-	payload_len = le32_to_cpu(hdr->len);
+	payload_len = le32_to_cpu(hdr.len);
+
+	/* The pkt is too big or the length in the header is invalid */
+	if (payload_len > len - sizeof(hdr))
+		return NULL;
+
+	/* Allocate only for the payload declared in the header. */
+	skb = virtio_vsock_alloc_skb(payload_len + sizeof(hdr), GFP_KERNEL);
+	if (!skb)
+		return NULL;
+
+	memcpy(virtio_vsock_hdr(skb), &hdr, sizeof(hdr));
 
 	/* 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);
-		return NULL;
-	}
-
 	virtio_vsock_skb_put(skb, payload_len);
 
 	if (skb_copy_datagram_from_iter(skb, 0, &iov_iter, payload_len)) {

base-commit: 6dc989ea46b96ce170840174b4a38c4a387fb005
-- 
2.55.0

Re: [PATCH net v2] vhost/vsock: size receive SKBs from declared payload

[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 v2] vhost/vsock: size receive SKBs from declared payload

[Will Deacon]

On Sun, Oct 04, 2026 at 04:34:19PM +0900, Daehyeon Ko wrote:
> vhost_vsock_alloc_skb() allocates an skb from the total guest descriptor
> length before reading hdr->len.  Descriptor capacity and declared payload
> length are independent, so a guest can supply a large descriptor with a
> zero or short payload.
> 
> On Linux v7.2, 455 zero-payload packets with 64 KiB descriptors retained
> 30,255,680 bytes on a 256 KiB receive buffer while rx_bytes and buf_used
> stayed zero.  A full-payload control retained 265,984 bytes.  The existing
> SKB_TRUESIZE(0) budget caps skb count but does not account for the
> descriptor-sized allocation.

Are you saying that we leak the descriptor memory or that we don't account
for it properly or something else? I'm trying to understand why the
zero-payload part is important.

> Repeating this across connections can exhaust host kernel memory.
> 
> Trimming after allocation is insufficient.  Linear skbs retain their full
> head, while a nonlinear payload exceeding head tailroom can retain its
> first page fragment.

Seems like a weird paragraph to include here... that's commentary on why
you did a v2, no?

> Copy the header into a stack object, validate hdr->len before allocating,
> and size the skb from the declared payload plus the header.  This keeps the
> allocation proportional to receive accounting for both linear and
> nonlinear skbs.
> 
> Use payload_len > len - sizeof(hdr) for validation.  This avoids addition
> overflow on 32-bit hosts and ensures payload_len fits the subsequent
> int-length copy path.
> 
> Fixes: ab9aa2f3afc2 ("vhost/vsock: Allocate nonlinear SKBs for handling large receive buffers")

Are you sure about this Fixes tag? The linear allocation before that
patch doesn't look much different when considering the report above (which
is a little hard to follow given that it's the usual LLM-style of prose).

Will

Re: [PATCH net v2] vhost/vsock: size receive SKBs from declared payload

[netdev-bot+sashiko]

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

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

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

- [Medium] The Fixes: tag names the wrong commit.
- [Low] The commit message presents the new `payload_len > len -
  sizeof(hdr)` check in vhost_vsock_alloc_skb() (drivers/vhost/vsock.c)…

--

vhost/vsock: size receive SKBs from declared payload

This patch changes vhost_vsock_alloc_skb() to copy the virtio_vsock_hdr
into a stack object and validate hdr.len before allocating. The skb is
then sized from the declared payload plus the header, not from the total
guest descriptor length. A guest can then no longer pin descriptor-sized
allocations by sending zero or short payloads.

> Use payload_len > len - sizeof(hdr) for validation.  This avoids addition
> overflow on 32-bit hosts and ensures payload_len fits the subsequent
> int-length copy path.

[Severity: Low]
Should this paragraph say that the old check already overflowed on
32-bit hosts? This patch removes the following check from
vhost_vsock_alloc_skb():

	if (payload_len + sizeof(*hdr) > len) {

That check came from 10a886aaed29 ("vhost/vsock: Avoid allocating
arbitrarily-sized SKBs"), which is Cc: stable. That commit dropped the
earlier payload_len > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE short-circuit and
kept only the addition.

VHOST_VSOCK has no 64BIT dependency, so size_t can be 32 bits. If the
guest sets hdr.len anywhere in [0xFFFFFFD4, 0xFFFFFFFF], then
payload_len + 44 wraps to a value in [0, 43]. That is below len, so the
bad length passes the check.

In mainline this seems harmless. virtio_vsock_skb_put() sets skb->len to
about 4 GiB. skb_copy_datagram_from_iter() then gets a negative int
length, copies nothing and returns -EFAULT, and the skb is freed.

Trees that have 10a886aaed29 but not ab9aa2f3afc2 still copy the payload
with:

	copy_from_iter(skb->data, payload_len, &iov_iter)

Wouldn't check_copy_size() hit WARN_ON_ONCE(bytes > INT_MAX) there? On
32-bit hosts that would be a WARN the guest can trigger, and a host
panic with panic_on_warn.

This patch fixes that as a side effect. Would it be worth saying so in
the changelog and adding a Fixes: tag for 10a886aaed29?

> Fixes: ab9aa2f3afc2 ("vhost/vsock: Allocate nonlinear SKBs for handling large receive buffers")

[Severity: Medium]
Is this the right commit for the Fixes: tag? vhost_vsock_alloc_skb()
sizes the skb from the descriptor length:

	len = iov_length(vq->iov, out);
	...
	/* len contains both payload and hdr */
	skb = virtio_vsock_alloc_skb(len, GFP_KERNEL);

This sizing seems to date from 71dc9ec9ac7d ("virtio/vsock: replace
virtio_vsock_pkt with sk_buff"). Before that commit,
vhost_vsock_alloc_pkt() sized the buffer from the header:

	pkt->buf = kvmalloc(pkt->len, GFP_KERNEL);

Here pkt->len came from hdr.len, which is the behaviour this patch
restores.

ab9aa2f3afc2 only switched the call from virtio_vsock_alloc_linear_skb(len)
to virtio_vsock_alloc_skb(len) so that large sizes could be nonlinear. It
did not change how the size is chosen.

ab9aa2f3afc2 isn't tagged for stable. Trees with 71dc9ec9ac7d and
10a886aaed29 but not ab9aa2f3afc2 would still have the problem in its
linear form. That probably includes 6.6.y and 6.12.y.

In those trees, a 64 KiB descriptor with a short payload allocates and
keeps a kmalloc head of about 128 KiB. Payloads above GOOD_COPY_LEN (128
bytes) skip the coalescing path in virtio_transport_recv_enqueue(). The
whole head stays queued, but only the payload bytes count against
buf_alloc. The changelog itself says "Linear skbs retain their full head",
which covers this case.

10a886aaed29 already uses Fixes: 71dc9ec9ac7d for the descriptor-sized
allocation. Should this patch use the same tag, so the stable trees pick
it up?

Fixes: 71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with sk_buff")

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