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:
> > -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
