On Wed, Jul 02, 2025 at 06:50:59PM +0200, Stefano Garzarella wrote:

On Tue, Jul 01, 2025 at 05:45:05PM +0100, Will Deacon wrote:

quoted

-static inline struct sk_buff *virtio_vsock_alloc_skb(unsigned int size, gfp_t mask)
+static inline struct sk_buff *
+__virtio_vsock_alloc_skb_with_frags(unsigned int header_len,
+				    unsigned int data_len,
+				    gfp_t mask)
{
	struct sk_buff *skb;
+	int err;

-	if (size < VIRTIO_VSOCK_SKB_HEADROOM)
-		return NULL;

I would have made this change in a separate patch, but IIUC the only other
caller is virtio_transport_alloc_skb() where this condition is implied,
right?

At this point in the series, virtio_vsock_alloc_skb() only has a single
caller (vhost_vsock_alloc_skb()) which already has a partial bounds check
and so this patch extends it to cover the lower bound.

Later (in "vsock/virtio: Allocate nonlinear SKBs for handling large
transmit buffers"), virtio_transport_alloc_skb() gets converted over
and that's fine because it never allocates less than
VIRTIO_VSOCK_SKB_HEADROOM.

I don't know, maybe we could have one patch where you touch this and
virtio_vsock_skb_rx_put(), and another where you introduce nonlinear
allocation for vhost/vsock.  What do you think? (not a strong opinion, just
worried about doing 2 things in a single patch)

I can spin a separate patch for the bounds check.

Will