netdev - Re: [PATCH v2 3/8] vsock/virtio: Move length check to callers of virtio_vsock_skb_rx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aHQkcBiO_1Xg33Bo@willie-the-truck>
Date: Sun, 13 Jul 2025 22:26:08 +0100
From: Will Deacon <will@...nel.org>
To: Stefano Garzarella <sgarzare@...hat.com>
Cc: linux-kernel@...r.kernel.org, Keir Fraser <keirf@...gle.com>,
	Steven Moreland <smoreland@...gle.com>,
	Frederick Mayle <fmayle@...gle.com>,
	Stefan Hajnoczi <stefanha@...hat.com>,
	"Michael S. Tsirkin" <mst@...hat.com>,
	Jason Wang <jasowang@...hat.com>,
	Eugenio Pérez <eperezma@...hat.com>,
	netdev@...r.kernel.org, virtualization@...ts.linux.dev
Subject: Re: [PATCH v2 3/8] vsock/virtio: Move length check to callers of
 virtio_vsock_skb_rx_put()

On Wed, Jul 02, 2025 at 06:28:37PM +0200, Stefano Garzarella wrote:
> On Tue, Jul 01, 2025 at 05:45:02PM +0100, Will Deacon wrote:
> > virtio_vsock_skb_rx_put() only calls skb_put() if the length in the
> > packet header is not zero even though skb_put() handles this case
> > gracefully.
> > 
> > Remove the functionally redundant check from virtio_vsock_skb_rx_put()
> > and, on the assumption that this is a worthwhile optimisation for
> > handling credit messages, augment the existing length checks in
> > virtio_transport_rx_work() to elide the call for zero-length payloads.
> > Note that the vhost code already has similar logic in
> > vhost_vsock_alloc_skb().
> > 
> > Signed-off-by: Will Deacon <will@...nel.org>
> > ---
> > include/linux/virtio_vsock.h     | 4 +---
> > net/vmw_vsock/virtio_transport.c | 4 +++-
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> > index 36fb3edfa403..eb6980aa19fd 100644
> > --- a/include/linux/virtio_vsock.h
> > +++ b/include/linux/virtio_vsock.h
> > @@ -52,9 +52,7 @@ static inline void virtio_vsock_skb_rx_put(struct sk_buff *skb)
> > 	u32 len;
> > 
> > 	len = le32_to_cpu(virtio_vsock_hdr(skb)->len);
> > -
> > -	if (len > 0)
> > -		skb_put(skb, len);
> > +	skb_put(skb, len);
> 
> Since the caller is supposed to check the len, can we just pass it as
> parameter?
> 
> So we can avoid the `le32_to_cpu(virtio_vsock_hdr(skb)->len)` here.

Sure, I'll do that. It means that virtio_vsock_skb_rx_put() will briefly
be a simple wrapper around skb_put() but once the non-linear handling
comes in then it becomes useful again.

Will