On Mon, Oct 05, 2026 at 08:51:01AM +0900, Daehyeon Ko wrote:
> vhost_vsock_alloc_skb() sizes the skb from iov_length(), the total guest
> descriptor length.  virtio_transport_inc_rx_pkt() instead adds hdr->len to
> rx_bytes and buf_used, and budgets each queued skb as SKB_TRUESIZE(0).  The
> guest controls the descriptor length and hdr->len independently.
> 
> A guest can therefore provide a 64 KiB descriptor while declaring a zero
> payload.  The descriptor-sized skb remains queued, rx_bytes and buf_used
> remain unchanged, and only the fixed queue-entry budget limits retention.
> This is not a lifetime leak: the skb is freed when dequeued or at socket
> teardown.  Repeating this across connections can nevertheless exhaust host
> memory.  On Linux v7.2, 455 such descriptors retained 30,255,680 bytes for
> a 262,144-byte receive buffer; a full-payload control retained 265,984
> bytes.
> 
> Vhost previously read hdr->len and allocated pkt->buf from that value.  The
> conversion from virtio_vsock_pkt to sk_buff instead allocated receive skbs
> from the descriptor length, introducing the allocation-accounting mismatch.
> 
> Copy the header into a stack object and validate hdr->len before allocating
> the skb.  Size the allocation as sizeof(hdr) + payload_len so descriptor
> capacity no longer determines queued memory.  Check payload_len against
> len - sizeof(hdr) to avoid addition overflow on 32-bit hosts.
> 
> Fixes: 71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with sk_buff")
> Cc: [email protected]
> Assisted-by: LLM
> Signed-off-by: Daehyeon Ko <[email protected]>


Weird that this wasn't applied in net yet.

Acked-by: Michael S. Tsirkin <[email protected]>

let me know if you want me to apply this.

> ---
> Changes in v3:
> - clarify that queued skb memory is under-accounted rather than leaked, and
>   explain why a zero payload is the strongest case;
> - remove v1-to-v2 implementation rationale from the permanent changelog;
> - correct the Fixes tag from ab9aa2f3afc2 to 71dc9ec9ac7d; and
> - make no code changes.
> 
> v2: 
> https://lore.kernel.org/netdev/[email protected]/
> 
> The issue was identified during an LLM-assisted source audit and confirmed
> with a focused in-kernel probe on Linux v7.2.  The code diff is
> byte-identical to v2, so its existing build and runtime results remain
> applicable.
> 
>  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


Reply via email to