* [PATCH net-next] net: gso: validate transport header before computing segment length
@ 2026-10-18 12:46 Michael S. Tsirkin
2026-10-18 14:16 ` Willem de Bruijn
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2026-10-18 12:46 UTC (permalink / raw)
To: linux-kernel
Cc: Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, netdev
If a TCP GSO packet is processed in the stack before a transport
header was ever established, the TCP data offset is read through the
unset header offset, far outside the packet.
This was demonstrated by syzkaller, back when the input path did not
dissect such packets: a virtio net header supplying a malformed
network header was enough. Qdisc length estimation has carried a
guard against exactly this since a0dce8752193 ("net: Skip GSO
length estimation if transport header is not set"); the GSO length
validators never got one.
We used to, but we no longer allow such packets in: since 44378d02c5aa
("net: always dissect GSO packets in __virtio_net_hdr_to_skb()") the
virtio net input path dissects all GSO packets.
But as a defence in depth measure, let's also check before we read: if
the TCP header is unset or outside the linear data, drop it.
Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
Cc: Willem de Bruijn <willemb@google.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Can be net or net-next, but given we have Eric's patch in net already,
net-next seems more appropriate, and it's safer.
include/linux/skbuff.h | 5 +++++
net/core/gso.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a3ff380d43f0..aa5bc608de06 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3041,6 +3041,11 @@ static inline bool skb_inner_network_header_was_set(const struct sk_buff *skb)
return skb->inner_network_header > 0;
}
+static inline bool skb_inner_transport_header_was_set(const struct sk_buff *skb)
+{
+ return skb->inner_transport_header > 0;
+}
+
static inline unsigned char *skb_inner_mac_header(const struct sk_buff *skb)
{
return skb->head + skb->inner_mac_header;
diff --git a/net/core/gso.c b/net/core/gso.c
index e96ef6350064..d98acfd68595 100644
--- a/net/core/gso.c
+++ b/net/core/gso.c
@@ -131,6 +131,36 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
}
EXPORT_SYMBOL(__skb_gso_segment);
+/*
+ * GSO processing (but not UDP/SCTP GSO, which uses fixed header
+ * sizes) reads the TCP header as specified by
+ * skb->transport_header/inner_transport_header. Make sure it is
+ * there to read.
+ */
+static bool skb_gso_transport_header_ok(const struct sk_buff *skb)
+{
+ const struct skb_shared_info *shinfo = skb_shinfo(skb);
+ unsigned int thoff;
+
+ if (likely(!(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))))
+ return true;
+
+ if (skb->encapsulation) {
+ if (!skb_inner_transport_header_was_set(skb))
+ return false;
+
+ thoff = skb_inner_transport_offset(skb);
+ } else {
+ if (!skb_transport_header_was_set(skb))
+ return false;
+
+ thoff = skb_transport_offset(skb);
+ }
+
+ /* A negative offset converts to a huge value and fails. */
+ return thoff + sizeof(struct tcphdr) <= skb_headlen(skb);
+}
+
/**
* skb_gso_transport_seglen - Return length of individual segments of a gso packet
*
@@ -253,6 +283,9 @@ static inline bool skb_gso_size_check(const struct sk_buff *skb,
*/
bool skb_gso_validate_network_len(const struct sk_buff *skb, unsigned int mtu)
{
+ if (unlikely(!skb_gso_transport_header_ok(skb)))
+ return false;
+
return skb_gso_size_check(skb, skb_gso_network_seglen(skb), mtu);
}
EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
@@ -268,6 +301,9 @@ EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
*/
bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len)
{
+ if (unlikely(!skb_gso_transport_header_ok(skb)))
+ return false;
+
return skb_gso_size_check(skb, skb_gso_mac_seglen(skb), len);
}
EXPORT_SYMBOL_GPL(skb_gso_validate_mac_len);
--
MST
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net-next] net: gso: validate transport header before computing segment length
2026-10-18 12:46 [PATCH net-next] net: gso: validate transport header before computing segment length Michael S. Tsirkin
@ 2026-10-18 14:16 ` Willem de Bruijn
2026-10-18 16:21 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Willem de Bruijn @ 2026-10-18 14:16 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, netdev
Michael S. Tsirkin wrote:
> If a TCP GSO packet is processed in the stack before a transport
TSO
> header was ever established, the TCP data offset is read through the
> unset header offset, far outside the packet.
>
> This was demonstrated by syzkaller, back when the input path did not
> dissect such packets: a virtio net header supplying a malformed
> network header was enough. Qdisc length estimation has carried a
> guard against exactly this since a0dce8752193 ("net: Skip GSO
> length estimation if transport header is not set"); the GSO length
> validators never got one.
>
> We used to, but we no longer allow such packets in: since 44378d02c5aa
> ("net: always dissect GSO packets in __virtio_net_hdr_to_skb()") the
> virtio net input path dissects all GSO packets.
>
> But as a defence in depth measure, let's also check before we read: if
> the TCP header is unset or outside the linear data, drop it.
>
> Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
> Cc: Willem de Bruijn <willemb@google.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Can be net or net-next, but given we have Eric's patch in net already,
> net-next seems more appropriate, and it's safer.
>
> include/linux/skbuff.h | 5 +++++
> net/core/gso.c | 36 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 41 insertions(+)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index a3ff380d43f0..aa5bc608de06 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3041,6 +3041,11 @@ static inline bool skb_inner_network_header_was_set(const struct sk_buff *skb)
> return skb->inner_network_header > 0;
> }
>
> +static inline bool skb_inner_transport_header_was_set(const struct sk_buff *skb)
> +{
> + return skb->inner_transport_header > 0;
skb_transport_header_was_set has
return skb->transport_header != (typeof(skb->transport_header))~0U;
is that initialization different for inner_transport_header?
If so, slightly confusing. But also understandable that zero is the
default initialization, I guess.
> +}
> +
> static inline unsigned char *skb_inner_mac_header(const struct sk_buff *skb)
> {
> return skb->head + skb->inner_mac_header;
> diff --git a/net/core/gso.c b/net/core/gso.c
> index e96ef6350064..d98acfd68595 100644
> --- a/net/core/gso.c
> +++ b/net/core/gso.c
> @@ -131,6 +131,36 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
> }
> EXPORT_SYMBOL(__skb_gso_segment);
>
> +/*
> + * GSO processing (but not UDP/SCTP GSO, which uses fixed header
TSO
> + * sizes) reads the TCP header as specified by
> + * skb->transport_header/inner_transport_header. Make sure it is
> + * there to read.
> + */
> +static bool skb_gso_transport_header_ok(const struct sk_buff *skb)
> +{
> + const struct skb_shared_info *shinfo = skb_shinfo(skb);
> + unsigned int thoff;
> +
> + if (likely(!(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))))
> + return true;
The likely case for GSO is TSO.
> +
> + if (skb->encapsulation) {
> + if (!skb_inner_transport_header_was_set(skb))
> + return false;
> +
> + thoff = skb_inner_transport_offset(skb);
> + } else {
> + if (!skb_transport_header_was_set(skb))
> + return false;
> +
> + thoff = skb_transport_offset(skb);
> + }
This is not GSO specific. Could be a helper in skbuff.h?
> +
> + /* A negative offset converts to a huge value and fails. */
> + return thoff + sizeof(struct tcphdr) <= skb_headlen(skb);
> +}
> +
> /**
> * skb_gso_transport_seglen - Return length of individual segments of a gso packet
> *
> @@ -253,6 +283,9 @@ static inline bool skb_gso_size_check(const struct sk_buff *skb,
> */
> bool skb_gso_validate_network_len(const struct sk_buff *skb, unsigned int mtu)
> {
> + if (unlikely(!skb_gso_transport_header_ok(skb)))
> + return false;
> +
seems weird to test a transport header feature in a network
header test?
Should this instead where it is needed?
> return skb_gso_size_check(skb, skb_gso_network_seglen(skb), mtu);
> }
> EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
> @@ -268,6 +301,9 @@ EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
> */
> bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len)
> {
> + if (unlikely(!skb_gso_transport_header_ok(skb)))
> + return false;
> +
even more so in a link layer header test
> return skb_gso_size_check(skb, skb_gso_mac_seglen(skb), len);
> }
> EXPORT_SYMBOL_GPL(skb_gso_validate_mac_len);
> --
> MST
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net-next] net: gso: validate transport header before computing segment length
2026-10-18 14:16 ` Willem de Bruijn
@ 2026-10-18 16:21 ` Michael S. Tsirkin
0 siblings, 0 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2026-10-18 16:21 UTC (permalink / raw)
To: Willem de Bruijn
Cc: linux-kernel, Willem de Bruijn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, netdev
Thanks for the quick review! I'll wait a bit for more comments
and post v2.
On Sun, Oct 18, 2026 at 10:16:23AM -0400, Willem de Bruijn wrote:
> Michael S. Tsirkin wrote:
> > If a TCP GSO packet is processed in the stack before a transport
>
> TSO
right.
> > header was ever established, the TCP data offset is read through the
> > unset header offset, far outside the packet.
> >
> > This was demonstrated by syzkaller, back when the input path did not
> > dissect such packets: a virtio net header supplying a malformed
> > network header was enough. Qdisc length estimation has carried a
> > guard against exactly this since a0dce8752193 ("net: Skip GSO
> > length estimation if transport header is not set"); the GSO length
> > validators never got one.
> >
> > We used to, but we no longer allow such packets in: since 44378d02c5aa
> > ("net: always dissect GSO packets in __virtio_net_hdr_to_skb()") the
> > virtio net input path dissects all GSO packets.
> >
> > But as a defence in depth measure, let's also check before we read: if
> > the TCP header is unset or outside the linear data, drop it.
> >
> > Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
> > Cc: Willem de Bruijn <willemb@google.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > Can be net or net-next, but given we have Eric's patch in net already,
> > net-next seems more appropriate, and it's safer.
> >
> > include/linux/skbuff.h | 5 +++++
> > net/core/gso.c | 36 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 41 insertions(+)
> >
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index a3ff380d43f0..aa5bc608de06 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -3041,6 +3041,11 @@ static inline bool skb_inner_network_header_was_set(const struct sk_buff *skb)
> > return skb->inner_network_header > 0;
> > }
> >
> > +static inline bool skb_inner_transport_header_was_set(const struct sk_buff *skb)
> > +{
> > + return skb->inner_transport_header > 0;
>
> skb_transport_header_was_set has
>
> return skb->transport_header != (typeof(skb->transport_header))~0U;
>
> is that initialization different for inner_transport_header?
> If so, slightly confusing. But also understandable that zero is the
> default initialization, I guess.
Indeed.
> > +}
> > +
> > static inline unsigned char *skb_inner_mac_header(const struct sk_buff *skb)
> > {
> > return skb->head + skb->inner_mac_header;
> > diff --git a/net/core/gso.c b/net/core/gso.c
> > index e96ef6350064..d98acfd68595 100644
> > --- a/net/core/gso.c
> > +++ b/net/core/gso.c
> > @@ -131,6 +131,36 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
> > }
> > EXPORT_SYMBOL(__skb_gso_segment);
> >
> > +/*
> > + * GSO processing (but not UDP/SCTP GSO, which uses fixed header
>
> TSO
right.
> > + * sizes) reads the TCP header as specified by
> > + * skb->transport_header/inner_transport_header. Make sure it is
> > + * there to read.
> > + */
> > +static bool skb_gso_transport_header_ok(const struct sk_buff *skb)
> > +{
> > + const struct skb_shared_info *shinfo = skb_shinfo(skb);
> > + unsigned int thoff;
> > +
> > + if (likely(!(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))))
> > + return true;
>
> The likely case for GSO is TSO.
indeed.
> > +
> > + if (skb->encapsulation) {
> > + if (!skb_inner_transport_header_was_set(skb))
> > + return false;
> > +
> > + thoff = skb_inner_transport_offset(skb);
> > + } else {
> > + if (!skb_transport_header_was_set(skb))
> > + return false;
> > +
> > + thoff = skb_transport_offset(skb);
> > + }
>
> This is not GSO specific. Could be a helper in skbuff.h?
will do.
> > +
> > + /* A negative offset converts to a huge value and fails. */
> > + return thoff + sizeof(struct tcphdr) <= skb_headlen(skb);
> > +}
> > +
> > /**
> > * skb_gso_transport_seglen - Return length of individual segments of a gso packet
> > *
> > @@ -253,6 +283,9 @@ static inline bool skb_gso_size_check(const struct sk_buff *skb,
> > */
> > bool skb_gso_validate_network_len(const struct sk_buff *skb, unsigned int mtu)
> > {
> > + if (unlikely(!skb_gso_transport_header_ok(skb)))
> > + return false;
> > +
>
> seems weird to test a transport header feature in a network
> header test?
>
> Should this instead where it is needed?
> > return skb_gso_size_check(skb, skb_gso_network_seglen(skb), mtu);
> > }
> > EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
> > @@ -268,6 +301,9 @@ EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
> > */
> > bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len)
> > {
> > + if (unlikely(!skb_gso_transport_header_ok(skb)))
> > + return false;
> > +
>
> even more so in a link layer header test
>
skb_gso_transport_seglen maybe? I'll have to make it return int
and make callers check that then.
> > return skb_gso_size_check(skb, skb_gso_mac_seglen(skb), len);
> > }
> > EXPORT_SYMBOL_GPL(skb_gso_validate_mac_len);
> > --
> > MST
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-18 16:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-18 12:46 [PATCH net-next] net: gso: validate transport header before computing segment length Michael S. Tsirkin
2026-10-18 14:16 ` Willem de Bruijn
2026-10-18 16:21 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox