Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH net-next] virtio_net: validate csum_start before installing partial state
@ 2026-10-16 21:25 Megane Hebo
  2026-10-16 23:57 ` Michael S. Tsirkin
  0 siblings, 1 reply; 3+ messages in thread
From: Megane Hebo @ 2026-10-16 21:25 UTC (permalink / raw)
  To: netdev
  Cc: virtualization, mst, jasowangio, eperezma, xuanzhuo,
	willemdebruijn.kernel, habte.yibelo

__virtio_net_hdr_to_skb() currently installs untrusted NEEDS_CSUM
metadata into the skb with skb_partial_csum_set() and only then checks
that the resulting transport offset is beyond the minimum network
header length.

Validate the raw csum_start first. Successful packets are unchanged,
but malformed metadata is rejected before it can transition the skb to
CHECKSUM_PARTIAL state. This deliberately does not replace the broader
ingress validation being discussed for virtio checksum metadata; it only
narrows the state transition at the existing check.

Link: https://lore.kernel.org/netdev/20260922030310.8684-2-habte.yibelo@gmail.com/
Signed-off-by: tonegawa <kiyotaka.tonegawa@gmail.com>
---
 include/linux/virtio_net.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index d6466f96cdd00c..c8180f1222f492 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -102,9 +102,9 @@ static inline int __virtio_net_hdr_to_skb(struct
sk_buff *skb,
 		if (!pskb_may_pull(skb, needed))
 			return -EINVAL;

-		if (!skb_partial_csum_set(skb, start, off))
+		if (start < nh_min_len)
 			return -EINVAL;
-		if (skb_transport_offset(skb) < nh_min_len)
+		if (!skb_partial_csum_set(skb, start, off))
 			return -EINVAL;

 		nh_min_len = skb_transport_offset(skb);
--
2.51.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] virtio_net: validate csum_start before installing partial state
  2026-10-16 21:25 [PATCH net-next] virtio_net: validate csum_start before installing partial state Megane Hebo
@ 2026-10-16 23:57 ` Michael S. Tsirkin
  2026-10-17  5:53   ` Megane Hebo
  0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2026-10-16 23:57 UTC (permalink / raw)
  To: Megane Hebo
  Cc: netdev, virtualization, jasowangio, eperezma, xuanzhuo,
	willemdebruijn.kernel, habte.yibelo

On Fri, Oct 16, 2026 at 02:25:31PM -0700, Megane Hebo wrote:
> __virtio_net_hdr_to_skb() currently installs untrusted NEEDS_CSUM
> metadata into the skb with skb_partial_csum_set() and only then checks
> that the resulting transport offset is beyond the minimum network
> header length.

and? why does it matter, the packet is dropped.

> Validate the raw csum_start first. Successful packets are unchanged,
> but malformed metadata is rejected before it can transition the skb to
> CHECKSUM_PARTIAL state. This deliberately does not replace the broader
> ingress validation being discussed for virtio checksum metadata; it only
> narrows the state transition at the existing check.
> 
> Link: https://lore.kernel.org/netdev/20260922030310.8684-2-habte.yibelo@gmail.com/
> Signed-off-by: tonegawa <kiyotaka.tonegawa@gmail.com>

what is the point?

> ---
>  include/linux/virtio_net.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index d6466f96cdd00c..c8180f1222f492 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -102,9 +102,9 @@ static inline int __virtio_net_hdr_to_skb(struct
> sk_buff *skb,
>  		if (!pskb_may_pull(skb, needed))
>  			return -EINVAL;
> 
> -		if (!skb_partial_csum_set(skb, start, off))
> +		if (start < nh_min_len)
>  			return -EINVAL;
> -		if (skb_transport_offset(skb) < nh_min_len)
> +		if (!skb_partial_csum_set(skb, start, off))
>  			return -EINVAL;
> 
>  		nh_min_len = skb_transport_offset(skb);
> --
> 2.51.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] virtio_net: validate csum_start before installing partial state
  2026-10-16 23:57 ` Michael S. Tsirkin
@ 2026-10-17  5:53   ` Megane Hebo
  0 siblings, 0 replies; 3+ messages in thread
From: Megane Hebo @ 2026-10-17  5:53 UTC (permalink / raw)
  To: mst
  Cc: netdev, virtualization, jasowangio, eperezma, xuanzhuo,
	willemdebruijn.kernel, habte.yibelo

Your objection to my previous patch was right: since the packet was
dropped, the reorder had no observable benefit. I dropped that
approach.

Reading your v2 series, I think the stronger invariant is simpler:
make packet semantics stable at ingress before later protocol
processing can observe two meanings. I wrote a small kselftest
follow-up for the three outcomes and will send it separately.

My broader side idea is just to make repeated reuse of an observed
attack capability progressively more expensive. One speculative
mechanism is a minimal ancestry "scar" that transformations cannot
silently erase. I assume that is too stateful and out of scope here,
so I am not proposing it for your series.

I have also shared that broader defensive idea with a few relevant
organizations because it may be useful beyond this bug; I do not claim
to know the final architecture.Your objection to my previous patch was
right: since the packet was dropped, the reorder had no observable
benefit. I dropped that approach.

Reading your v2 series, I think the stronger invariant is simpler:
make packet semantics stable at ingress before later protocol
processing can observe two meanings. I wrote a small kselftest
follow-up for the three outcomes and will send it separately.

My broader side idea is just to make repeated reuse of an observed
attack capability progressively more expensive. One speculative
mechanism is a minimal ancestry “scar” that transformations cannot
silently erase. I assume that is too stateful and out of scope here,
so I am not proposing it for your series.

I have also shared that broader defensive idea with a few relevant
organizations because it may be useful beyond this bug; I do not claim
to know the final architecture.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-17  5:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16 21:25 [PATCH net-next] virtio_net: validate csum_start before installing partial state Megane Hebo
2026-10-16 23:57 ` Michael S. Tsirkin
2026-10-17  5:53   ` Megane Hebo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox