* [PATCH net v2] net: gso: validate transport header before computing segment length
@ 2026-10-19 11:58 Michael S. Tsirkin
2026-10-20 19:10 ` Willem de Bruijn
0 siblings, 1 reply; 2+ messages in thread
From: Michael S. Tsirkin @ 2026-10-19 11:58 UTC (permalink / raw)
To: linux-kernel
Cc: Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Max Krasnyansky, netdev
If a TSO 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>
---
changes from v1:
- address Willem's comments, in particular, teach skb_gso_network_seglen
to return a negative error, and propagate through callers.
include/linux/skbuff.h | 5 +++
net/core/gso.c | 82 ++++++++++++++++++++++++++++++++++++------
2 files changed, 77 insertions(+), 10 deletions(-)
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..874e4c8b1f1b 100644
--- a/net/core/gso.c
+++ b/net/core/gso.c
@@ -131,6 +131,35 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
}
EXPORT_SYMBOL(__skb_gso_segment);
+/*
+ * TSO 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);
+ int thoff;
+
+ if (unlikely(!(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);
+ }
+
+ return thoff >= 0 && thoff + sizeof(struct tcphdr) <= skb_headlen(skb);
+}
+
/**
* skb_gso_transport_seglen - Return length of individual segments of a gso packet
*
@@ -140,12 +169,18 @@ EXPORT_SYMBOL(__skb_gso_segment);
* individual segments, including Layer4 headers (TCP/UDP).
*
* The MAC/L2 or network (IP, IPv6) headers are not accounted for.
+ *
+ * Return: the length, or a negative errno if the TCP header is not
+ * there to read.
*/
-static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
+static int skb_gso_transport_seglen(const struct sk_buff *skb)
{
const struct skb_shared_info *shinfo = skb_shinfo(skb);
unsigned int thlen = 0;
+ if (unlikely(!skb_gso_transport_header_ok(skb)))
+ return -EINVAL;
+
if (skb->encapsulation) {
thlen = skb_inner_transport_header(skb) -
skb_transport_header(skb);
@@ -175,13 +210,21 @@ static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
* individual segments, including Layer3 (IP, IPv6) and L4 headers (TCP/UDP).
*
* The MAC/L2 header is not accounted for.
+ *
+ * Return: the length, or a negative errno if the TCP header is not
+ * there to read.
*/
-static unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
+static int skb_gso_network_seglen(const struct sk_buff *skb)
{
- unsigned int hdr_len = skb_transport_header(skb) -
- skb_network_header(skb);
+ unsigned int hdr_len;
+ int tlen = skb_gso_transport_seglen(skb);
- return hdr_len + skb_gso_transport_seglen(skb);
+ if (tlen < 0)
+ return tlen;
+
+ hdr_len = skb_transport_header(skb) - skb_network_header(skb);
+
+ return hdr_len + tlen;
}
/**
@@ -192,12 +235,21 @@ static unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
* skb_gso_mac_seglen is used to determine the real size of the
* individual segments, including MAC/L2, Layer3 (IP, IPv6) and L4
* headers (TCP/UDP).
+ *
+ * Return: the length, or a negative errno if the TCP header is not
+ * there to read.
*/
-static unsigned int skb_gso_mac_seglen(const struct sk_buff *skb)
+static int skb_gso_mac_seglen(const struct sk_buff *skb)
{
- unsigned int hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
+ unsigned int hdr_len;
+ int tlen = skb_gso_transport_seglen(skb);
- return hdr_len + skb_gso_transport_seglen(skb);
+ if (tlen < 0)
+ return tlen;
+
+ hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
+
+ return hdr_len + tlen;
}
/**
@@ -253,7 +305,12 @@ 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)
{
- return skb_gso_size_check(skb, skb_gso_network_seglen(skb), mtu);
+ int seg_len = skb_gso_network_seglen(skb);
+
+ if (seg_len < 0)
+ return false;
+
+ return skb_gso_size_check(skb, seg_len, mtu);
}
EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
@@ -268,7 +325,12 @@ EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
*/
bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len)
{
- return skb_gso_size_check(skb, skb_gso_mac_seglen(skb), len);
+ int seg_len = skb_gso_mac_seglen(skb);
+
+ if (seg_len < 0)
+ return false;
+
+ return skb_gso_size_check(skb, seg_len, len);
}
EXPORT_SYMBOL_GPL(skb_gso_validate_mac_len);
--
MST
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net v2] net: gso: validate transport header before computing segment length
2026-10-19 11:58 [PATCH net v2] net: gso: validate transport header before computing segment length Michael S. Tsirkin
@ 2026-10-20 19:10 ` Willem de Bruijn
0 siblings, 0 replies; 2+ messages in thread
From: Willem de Bruijn @ 2026-10-20 19:10 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Max Krasnyansky, netdev
Michael S. Tsirkin wrote:
> If a TSO 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
please insert commit here and before 44378d02c5aa. I'm surprised
checkpatch did not complain.
> 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.
is the packet indeed dropped?
>
> 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>
> ---
> changes from v1:
> - address Willem's comments, in particular, teach skb_gso_network_seglen
> to return a negative error, and propagate through callers.
>
> include/linux/skbuff.h | 5 +++
> net/core/gso.c | 82 ++++++++++++++++++++++++++++++++++++------
> 2 files changed, 77 insertions(+), 10 deletions(-)
>
> 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;
> +}
It may be possible that skb_reset_inner_headers gets called when
skb->transport_header is ~0U. For safety, also check ~0U here?
> 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..874e4c8b1f1b 100644
> --- a/net/core/gso.c
> +++ b/net/core/gso.c
> @@ -131,6 +131,35 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
> }
> EXPORT_SYMBOL(__skb_gso_segment);
>
> +/*
> + * TSO 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);
> + int thoff;
> +
> + if (unlikely(!(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))))
> + return true;
maybe skb_tso_transport_header_ok given this limitation
> +
> + if (skb->encapsulation) {
> + if (!skb_inner_transport_header_was_set(skb))
> + return false;
> +
> + thoff = skb_inner_transport_offset(skb);
skb_gso_transport_seglen reads both transport_offset and
inner_transport_offset, so if skb->encapsulation we probably should
not check only inner, but both?
> + } else {
> + if (!skb_transport_header_was_set(skb))
> + return false;
> +
> + thoff = skb_transport_offset(skb);
> + }
> +
> + return thoff >= 0 && thoff + sizeof(struct tcphdr) <= skb_headlen(skb);
> +}
> +
> /**
> * skb_gso_transport_seglen - Return length of individual segments of a gso packet
> *
> @@ -140,12 +169,18 @@ EXPORT_SYMBOL(__skb_gso_segment);
> * individual segments, including Layer4 headers (TCP/UDP).
> *
> * The MAC/L2 or network (IP, IPv6) headers are not accounted for.
> + *
> + * Return: the length, or a negative errno if the TCP header is not
> + * there to read.
> */
> -static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
> +static int skb_gso_transport_seglen(const struct sk_buff *skb)
> {
> const struct skb_shared_info *shinfo = skb_shinfo(skb);
> unsigned int thlen = 0;
>
> + if (unlikely(!skb_gso_transport_header_ok(skb)))
> + return -EINVAL;
> +
> if (skb->encapsulation) {
> thlen = skb_inner_transport_header(skb) -
> skb_transport_header(skb);
> @@ -175,13 +210,21 @@ static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
> * individual segments, including Layer3 (IP, IPv6) and L4 headers (TCP/UDP).
> *
> * The MAC/L2 header is not accounted for.
> + *
> + * Return: the length, or a negative errno if the TCP header is not
> + * there to read.
> */
> -static unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
> +static int skb_gso_network_seglen(const struct sk_buff *skb)
> {
> - unsigned int hdr_len = skb_transport_header(skb) -
> - skb_network_header(skb);
> + unsigned int hdr_len;
> + int tlen = skb_gso_transport_seglen(skb);
nit: reverse xmas tree
>
> - return hdr_len + skb_gso_transport_seglen(skb);
> + if (tlen < 0)
> + return tlen;
> +
> + hdr_len = skb_transport_header(skb) - skb_network_header(skb);
> +
> + return hdr_len + tlen;
> }
>
> /**
> @@ -192,12 +235,21 @@ static unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
> * skb_gso_mac_seglen is used to determine the real size of the
> * individual segments, including MAC/L2, Layer3 (IP, IPv6) and L4
> * headers (TCP/UDP).
> + *
> + * Return: the length, or a negative errno if the TCP header is not
> + * there to read.
> */
> -static unsigned int skb_gso_mac_seglen(const struct sk_buff *skb)
> +static int skb_gso_mac_seglen(const struct sk_buff *skb)
> {
> - unsigned int hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
> + unsigned int hdr_len;
> + int tlen = skb_gso_transport_seglen(skb);
>
> - return hdr_len + skb_gso_transport_seglen(skb);
> + if (tlen < 0)
> + return tlen;
> +
> + hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
> +
> + return hdr_len + tlen;
> }
>
> /**
> @@ -253,7 +305,12 @@ 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)
> {
> - return skb_gso_size_check(skb, skb_gso_network_seglen(skb), mtu);
> + int seg_len = skb_gso_network_seglen(skb);
> +
> + if (seg_len < 0)
> + return false;
> +
> + return skb_gso_size_check(skb, seg_len, mtu);
> }
> EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
>
> @@ -268,7 +325,12 @@ EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
> */
> bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len)
> {
> - return skb_gso_size_check(skb, skb_gso_mac_seglen(skb), len);
> + int seg_len = skb_gso_mac_seglen(skb);
> +
> + if (seg_len < 0)
> + return false;
> +
> + return skb_gso_size_check(skb, seg_len, len);
> }
> EXPORT_SYMBOL_GPL(skb_gso_validate_mac_len);
>
> --
> MST
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-20 19:10 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-19 11:58 [PATCH net v2] net: gso: validate transport header before computing segment length Michael S. Tsirkin
2026-10-20 19:10 ` Willem de Bruijn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox