[PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures

[PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures

[Michael S. Tsirkin]

virtio_net_hdr_to_skb() refuses packets whose checksum metadata
it cannot make sense of, but the vector raw transport ignored
its return value: rejected packets went on into the stack with
their metadata unvalidated.

Propagate the failure.

Assisted-by: LLM
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/um/drivers/vector_transports.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
index e1fb2a76fdff..8b7548458663 100644
--- a/arch/um/drivers/vector_transports.c
+++ b/arch/um/drivers/vector_transports.c
@@ -210,7 +210,9 @@ static int raw_verify_header(
 		return 1;
 
 	skb_set_network_header(skb, ETH_HLEN);
-	virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
+	if (virtio_net_hdr_to_skb(skb, vheader,
+			virtio_legacy_is_little_endian()) < 0)
+		return -1;
 	return 0;
 }
 
-- 
MST

[PATCH net-next v2 3/7] um: vector: trim skbs before verifying the header

[Michael S. Tsirkin]

The vector receive path over-allocates the skb and only trims it
to the received length after the transport header is verified.

A follow up patch will complete the packet checksum in software within
virtio_net_hdr_to_skb so we need the packet trimmed (otherwise the
checksum would be computed incorrectly over the unreceived tail of the
buffer).

Trim first, so the import sees the packet as it was received.

Assisted-by: LLM
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/um/drivers/vector_kern.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/um/drivers/vector_kern.c b/arch/um/drivers/vector_kern.c
index 8a70b3a62553..1afbef78df72 100644
--- a/arch/um/drivers/vector_kern.c
+++ b/arch/um/drivers/vector_kern.c
@@ -865,6 +865,7 @@ static int vector_legacy_rx(struct vector_private *vp)
 
 	if (skb != NULL) {
 		if (pkt_len > vp->header_size) {
+			pskb_trim(skb, pkt_len - vp->rx_header_size);
 			if (vp->header_size > 0) {
 				header_check = vp->verify_header(
 					vp->header_rxbuffer, skb, vp);
@@ -879,7 +880,6 @@ static int vector_legacy_rx(struct vector_private *vp)
 					skb->ip_summed = CHECKSUM_UNNECESSARY;
 				}
 			}
-			pskb_trim(skb, pkt_len - vp->rx_header_size);
 			skb->protocol = eth_type_trans(skb, skb->dev);
 			vp->dev->stats.rx_bytes += skb->len;
 			vp->dev->stats.rx_packets++;
@@ -981,6 +981,8 @@ static int vector_mmsg_rx(struct vector_private *vp, int budget)
 	for (i = 0; i < packet_count; i++) {
 		skb = (*skbuff_vector);
 		if (mmsg_vector->msg_len > vp->header_size) {
+			pskb_trim(skb,
+				mmsg_vector->msg_len - vp->rx_header_size);
 			if (vp->header_size > 0) {
 				header_check = vp->verify_header(
 					mmsg_vector->msg_hdr.msg_iov->iov_base,
@@ -1005,8 +1007,6 @@ static int vector_mmsg_rx(struct vector_private *vp, int budget)
 					skb->ip_summed = CHECKSUM_UNNECESSARY;
 				}
 			}
-			pskb_trim(skb,
-				mmsg_vector->msg_len - vp->rx_header_size);
 			skb->protocol = eth_type_trans(skb, skb->dev);
 			/*
 			 * We do not need to lock on updating stats here
-- 
MST

Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures

[Johannes Berg]

On Thu, 2026-10-15 at 08:41 -0400, Michael S. Tsirkin wrote:
> virtio_net_hdr_to_skb() refuses packets whose checksum metadata
> it cannot make sense of, but the vector raw transport ignored
> its return value: rejected packets went on into the stack with
> their metadata unvalidated.
> 
> Propagate the failure.
> 
> Assisted-by: LLM
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  arch/um/drivers/vector_transports.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
> index e1fb2a76fdff..8b7548458663 100644
> --- a/arch/um/drivers/vector_transports.c
> +++ b/arch/um/drivers/vector_transports.c
> @@ -210,7 +210,9 @@ static int raw_verify_header(
>  		return 1;
>  
>  	skb_set_network_header(skb, ETH_HLEN);
> -	virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
> +	if (virtio_net_hdr_to_skb(skb, vheader,
> +			virtio_legacy_is_little_endian()) < 0)
> +		return -1;
>  	return 0;
> 

Seems it'd be simpler to just

	return virtio_net_hdr_to_skb(...);

As far as I can tell, the function only returns -EINVAL or 0 (never a
positive value), and the caller only cares about negative (error) or
positive (no checksum needed), so that'd be equivalent?

johannes

Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures

[Michael S. Tsirkin]

On Thu, Oct 15, 2026 at 02:47:01PM +0200, Johannes Berg wrote:
> On Thu, 2026-10-15 at 08:41 -0400, Michael S. Tsirkin wrote:
> > virtio_net_hdr_to_skb() refuses packets whose checksum metadata
> > it cannot make sense of, but the vector raw transport ignored
> > its return value: rejected packets went on into the stack with
> > their metadata unvalidated.
> > 
> > Propagate the failure.
> > 
> > Assisted-by: LLM
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  arch/um/drivers/vector_transports.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
> > index e1fb2a76fdff..8b7548458663 100644
> > --- a/arch/um/drivers/vector_transports.c
> > +++ b/arch/um/drivers/vector_transports.c
> > @@ -210,7 +210,9 @@ static int raw_verify_header(
> >  		return 1;
> >  
> >  	skb_set_network_header(skb, ETH_HLEN);
> > -	virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
> > +	if (virtio_net_hdr_to_skb(skb, vheader,
> > +			virtio_legacy_is_little_endian()) < 0)
> > +		return -1;
> >  	return 0;
> > 
> 
> Seems it'd be simpler to just
> 
> 	return virtio_net_hdr_to_skb(...);
> 
> As far as I can tell, the function only returns -EINVAL or 0 (never a
> positive value), and the caller only cares about negative (error) or
> positive (no checksum needed), so that'd be equivalent?
> 
> johannes

if raw_verify_header can safely return any errno - i did not want to
audit it.

-- 
MST

Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures

[Johannes Berg]

On Thu, 2026-10-15 at 08:52 -0400, Michael S. Tsirkin wrote:
> On Thu, Oct 15, 2026 at 02:47:01PM +0200, Johannes Berg wrote:
> > On Thu, 2026-10-15 at 08:41 -0400, Michael S. Tsirkin wrote:
> > > virtio_net_hdr_to_skb() refuses packets whose checksum metadata
> > > it cannot make sense of, but the vector raw transport ignored
> > > its return value: rejected packets went on into the stack with
> > > their metadata unvalidated.
> > > 
> > > Propagate the failure.
> > > 
> > > Assisted-by: LLM
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  arch/um/drivers/vector_transports.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
> > > index e1fb2a76fdff..8b7548458663 100644
> > > --- a/arch/um/drivers/vector_transports.c
> > > +++ b/arch/um/drivers/vector_transports.c
> > > @@ -210,7 +210,9 @@ static int raw_verify_header(
> > >  		return 1;
> > >  
> > >  	skb_set_network_header(skb, ETH_HLEN);
> > > -	virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
> > > +	if (virtio_net_hdr_to_skb(skb, vheader,
> > > +			virtio_legacy_is_little_endian()) < 0)
> > > +		return -1;
> > >  	return 0;
> > > 
> > 
> > Seems it'd be simpler to just
> > 
> > 	return virtio_net_hdr_to_skb(...);
> > 
> > As far as I can tell, the function only returns -EINVAL or 0 (never a
> > positive value), and the caller only cares about negative (error) or
> > positive (no checksum needed), so that'd be equivalent?
> > 
> > johannes
> 
> if raw_verify_header can safely return any errno - i did not want to
> audit it.

You already used an LLM and still didn't want to? ;-)

I checked briefly (by hand) and it seems fine, it's assigned to the
function pointer, which only has two callers on the _kern.c side.

johannes