[PATCH net-next 0/4] ipv6: fix stale transport header of GSO packets with extension headers

[PATCH net-next 0/4] ipv6: fix stale transport header of GSO packets with extension headers

[Matt Vollrath]

IPv6 input uses transport_header as a cursor over the extension headers:
ip6_rcv_core() sets it just past the fixed header, and each handler
advances it. Forwarding stops after hop-by-hop options, so a forwarded
GSO packet with a destination options or routing header keeps
transport_header at that extension header, in place of the L4 offset
GRO had computed. seg6 inline, rpl and ioam6 inline likewise point it at
the header they insert on output.

Code that runs later reads the extension header as TCP:

- Since commit fe6cc55f3a9a ("net: ip, ipv6: handle gso skbs in
  forwarding path"), ip6_forward()'s MTU check can answer with Packet
  Too Big at the current MTU, and the flow stalls.
- Since commit 7fb4c1967011 ("net: pull headers in
  qdisc_pkt_len_segs_init()"), in v7.1 and 6.12.114,
  __dev_queue_xmit() drops the packet as SKB_BAD_GSO when the bogus
  doff is below 5. Zero padding and SRH segment addresses make that
  common.
- Qdisc length accounting and ndo_features_check() see the wrong
  header lengths.

Rather than make each reader find the L4 header itself, set
transport_header where it goes stale. Patch 1 does it in ip6_forward()
before the MTU check, which every forwarding path reaches, including a
router that processes a routing header and forwards the packet again.
Patches 2-4 restore it after inline insertion on output. The input
paths keep it at the inserted header, where local input parses from.

For these packets, drivers now see the real L4 header in
ndo_features_check(), and in ndo_start_xmit() too for those that are not
CHECKSUM_PARTIAL, such as fraglist GRO. CHECKSUM_PARTIAL packets already
reached ndo_start_xmit() with this value.

Not covered: bridged IPv6 with br_netfilter, where
br_nf_pre_routing_ipv6() does what ip6_rcv_core() does, and GSO
packets that never pass through IPv6 input, such as LRO aggregates
redirected by tc.

Tested on net-next in QEMU with 3 s TCP flows between network
namespaces over veth, forwarded by the host unless noted:

                                        before     after
  dstopts, byte 12 = 0x00 (drop)        1.1 MB     13.9 GB
  dstopts, byte 12 = 0xf0 (PTB loop)    6.2 MB     13.9 GB
  SRH set by an unprivileged sender     5.0 MB     14.4 GB
  SRH with the router as a segment      4.7 MB     11.4 GB
  fraglist GRO, dstopts 0x00            3.8 MB      3.0 GB
  sender's seg6 / rpl / ioam6 inline     65 KB   9.3-11.0 GB

Forwarding without extension headers was unchanged within run-to-run
noise.

Matt Vollrath (4):
  ipv6: fix transport header of forwarded GSO packets
  ipv6: sr: keep transport header after inline SRH insertion
  ipv6: rpl: keep transport header after SRH insertion
  ipv6: ioam: keep transport header after inline insertion

 net/ipv6/ioam6_iptunnel.c |  7 +++++++
 net/ipv6/ip6_output.c     | 17 +++++++++++++++++
 net/ipv6/rpl_iptunnel.c   |  7 +++++++
 net/ipv6/seg6_iptunnel.c  |  8 ++++++++
 4 files changed, 39 insertions(+)


base-commit: 0822d2fe38262e8b55c5ea55c970022bca8dd2a9
-- 
2.43.0

[PATCH net-next 1/4] ipv6: fix transport header of forwarded GSO packets

[Matt Vollrath]

ip6_rcv_core() sets transport_header just past the fixed IPv6 header,
and local input advances it over each extension header. Forwarding
skips that walk, so a GSO packet with a destination options or routing
header reaches ip6_forward() with transport_header at an extension
header. So does a packet forwarded again after the router processed its
routing header.

skb_gso_validate_network_len() then reads a bogus TCP header length. An
overestimate makes ip6_forward() answer with Packet Too Big at the
current MTU, and the flow stalls. Since commit 7fb4c1967011 ("net: pull
headers in qdisc_pkt_len_segs_init()"), a bogus doff below 5 also makes
__dev_queue_xmit() drop the packet as SKB_BAD_GSO.

Before the MTU check, set transport_header from csum_start for
CHECKSUM_PARTIAL packets without encapsulation. Otherwise, if the
packet has extension headers, skip them with ipv6_skip_exthdr().

Signed-off-by: Matt Vollrath <tactii@gmail.com>
---
The Packet Too Big stall dates back to commit fe6cc55f3a9a ("net: ip,
ipv6: handle gso skbs in forwarding path").
---
 net/ipv6/ip6_output.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 25fe0ba98b1a..60ac56afbd06 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -662,6 +662,23 @@ int ip6_forward(struct sk_buff *skb)
 
 	__IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTFORWDATAGRAMS);
 
+	if (skb_is_gso(skb)) {
+		if (skb->ip_summed == CHECKSUM_PARTIAL && !skb->encapsulation) {
+			skb_set_transport_header(skb,
+						 skb_checksum_start_offset(skb));
+		} else if (ipv6_ext_hdr(hdr->nexthdr)) {
+			u8 nexthdr = hdr->nexthdr;
+			__be16 frag_off;
+			int thoff;
+
+			thoff = ipv6_skip_exthdr(skb, skb_network_offset(skb) +
+						 sizeof(*hdr), &nexthdr,
+						 &frag_off);
+			if (thoff >= 0)
+				skb_set_transport_header(skb, thoff);
+		}
+	}
+
 	mtu = ip6_dst_mtu_maybe_forward(dst, true);
 	if (mtu < IPV6_MIN_MTU)
 		mtu = IPV6_MIN_MTU;
-- 
2.43.0

[PATCH net-next 2/4] ipv6: sr: keep transport header after inline SRH insertion

[Matt Vollrath]

In inline mode, seg6_do_srh() leaves transport_header at the inserted
SRH, although the L4 header has not moved. GSO length checks then read
the SRH as TCP, and since commit 7fb4c1967011 ("net: pull headers in
qdisc_pkt_len_segs_init()"), a bogus doff below 5 there makes
__dev_queue_xmit() drop each GSO packet of the flow as SKB_BAD_GSO.

On output, restore transport_header from its distance to the end of the
packet, which the insertion does not change. The input path keeps it at
the SRH, where local input parses extension headers from.

Signed-off-by: Matt Vollrath <tactii@gmail.com>
---
transport_header has pointed at the inserted SRH since commit
6c8702c60b88 ("ipv6: sr: add support for SRH encapsulation and injection
with lwtunnels"), where qdisc length accounting and ndo_features_check()
read it.
---
 net/ipv6/seg6_iptunnel.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index 0d39a38a5dd2..942704eca8db 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -676,6 +676,7 @@ static int seg6_output_core(struct net *net, struct sock *sk,
 	struct dst_entry *orig_dst = skb_dst(skb);
 	struct dst_entry *dst = NULL;
 	struct seg6_lwt *slwt;
+	int tail_len = -1;
 	int err;
 
 	slwt = seg6_lwt_lwtunnel(orig_dst->lwtstate);
@@ -684,10 +685,17 @@ static int seg6_output_core(struct net *net, struct sock *sk,
 	dst = dst_cache_get(&slwt->cache_output);
 	local_bh_enable();
 
+	if (slwt->tuninfo->mode == SEG6_IPTUN_MODE_INLINE &&
+	    skb_transport_header_was_set(skb))
+		tail_len = skb->len - skb_transport_offset(skb);
+
 	err = seg6_do_srh(skb, dst);
 	if (unlikely(err))
 		goto drop;
 
+	if (tail_len >= 0)
+		skb_set_transport_header(skb, skb->len - tail_len);
+
 	if (unlikely(!dst)) {
 		struct ipv6hdr *hdr = ipv6_hdr(skb);
 		struct flowi6 fl6;
-- 
2.43.0

[PATCH net-next 3/4] ipv6: rpl: keep transport header after SRH insertion

[Matt Vollrath]

rpl_do_srh_inline() leaves transport_header at the inserted RPL SRH,
although the L4 header has not moved. GSO length checks then read the
SRH as TCP, and since commit 7fb4c1967011 ("net: pull headers in
qdisc_pkt_len_segs_init()"), a bogus doff below 5 there makes
__dev_queue_xmit() drop each GSO packet of the flow as SKB_BAD_GSO.

In rpl_output(), restore transport_header from its distance to the end
of the packet, which the insertion does not change. rpl_input() keeps it
at the SRH, where local input parses extension headers from.

Signed-off-by: Matt Vollrath <tactii@gmail.com>
---
transport_header has pointed at the inserted SRH since commit
a7a29f9c361f ("net: ipv6: add rpl sr tunnel"), where qdisc length
accounting and ndo_features_check() read it.
---
 net/ipv6/rpl_iptunnel.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/ipv6/rpl_iptunnel.c b/net/ipv6/rpl_iptunnel.c
index 4e10adcd70e8..0556bb989041 100644
--- a/net/ipv6/rpl_iptunnel.c
+++ b/net/ipv6/rpl_iptunnel.c
@@ -206,6 +206,7 @@ static int rpl_output(struct net *net, struct sock *sk, struct sk_buff *skb)
 	struct dst_entry *orig_dst = skb_dst(skb);
 	struct dst_entry *dst = NULL;
 	struct rpl_lwt *rlwt;
+	int tail_len = -1;
 	int err;
 
 	rlwt = rpl_lwt_lwtunnel(orig_dst->lwtstate);
@@ -214,10 +215,16 @@ static int rpl_output(struct net *net, struct sock *sk, struct sk_buff *skb)
 	dst = dst_cache_get(&rlwt->cache);
 	local_bh_enable();
 
+	if (skb_transport_header_was_set(skb))
+		tail_len = skb->len - skb_transport_offset(skb);
+
 	err = rpl_do_srh(skb, rlwt, dst);
 	if (unlikely(err))
 		goto drop;
 
+	if (tail_len >= 0)
+		skb_set_transport_header(skb, skb->len - tail_len);
+
 	if (unlikely(!dst)) {
 		struct ipv6hdr *hdr = ipv6_hdr(skb);
 		struct flowi6 fl6;
-- 
2.43.0

[PATCH net-next 4/4] ipv6: ioam: keep transport header after inline insertion

[Matt Vollrath]

ioam6_do_inline() leaves transport_header at the inserted hop-by-hop
header, although the L4 header has not moved. GSO length checks then
read that header as TCP, and since commit 7fb4c1967011 ("net: pull
headers in qdisc_pkt_len_segs_init()"), a bogus doff below 5 there makes
__dev_queue_xmit() drop each GSO packet of the flow as SKB_BAD_GSO. This
depends on the trace type, and affects forwarded packets too, since the
insertion happens on output.

After ioam6_do_inline(), restore transport_header from its distance to
the end of the packet, which the insertion does not change.

Signed-off-by: Matt Vollrath <tactii@gmail.com>
---
transport_header has pointed at the inserted hop-by-hop header since
commit 3edede08ff37 ("ipv6: ioam: Support for IOAM injection with
lwtunnels"), where qdisc length accounting and ndo_features_check() read
it.
---
 net/ipv6/ioam6_iptunnel.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c
index cfb2c41634a0..27b47f0c8bf8 100644
--- a/net/ipv6/ioam6_iptunnel.c
+++ b/net/ipv6/ioam6_iptunnel.c
@@ -340,6 +340,7 @@ static int ioam6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
 	struct dst_entry *orig_dst = skb_dst(skb);
 	struct dst_entry *dst = NULL;
 	struct ioam6_lwt *ilwt;
+	int tail_len = -1;
 	int err = -EINVAL;
 	u32 pkt_cnt;
 
@@ -375,10 +376,16 @@ static int ioam6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
 		if (ipv6_hdr(skb)->nexthdr == NEXTHDR_HOP)
 			goto out;
 
+		if (skb_transport_header_was_set(skb))
+			tail_len = skb->len - skb_transport_offset(skb);
+
 		err = ioam6_do_inline(net, skb, &ilwt->tuninfo, dst);
 		if (unlikely(err))
 			goto drop;
 
+		if (tail_len >= 0)
+			skb_set_transport_header(skb, skb->len - tail_len);
+
 		break;
 	case IOAM6_IPTUNNEL_MODE_ENCAP:
 do_encap:
-- 
2.43.0

Re: [PATCH net-next 0/4] ipv6: fix stale transport header of GSO packets with extension headers

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net-next 0/4] ipv6: fix stale transport header of GSO packets with extension headers

[Matt Vollrath]

On 11/5/26 15:17, netdev-bot+sinfo@kernel.org wrote:
> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>   - How the issue was discovered, e.g. hit in production, hit during
>     development, syzbot report, manual code inspection, LLM or static
>     analysis tool scan.

This was encountered while working on an ndo_features_check
implementation to fix a bug in e1000e[1]. An LLM found that there
were more affected consumers of transport_header, so I expanded
the scope of a different fix[2].

[1] https://lore.kernel.org/netdev/20261028170239.28606-1-tactii@gmail.com/
[2] https://lore.kernel.org/netdev/20261104175517.2395704-1-tactii@gmail.com/
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Re: [PATCH net-next 1/4] ipv6: fix transport header of forwarded GSO packets

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] ipv6: unvalidated csum_start in ip6_forward() moves
  transport_header backwards
- [Medium] In ip6_forward() (net/ipv6/ip6_output.c), the new else-branch
  sets transport_header to whatever ipv6_skip_exthdr() returns.
- [Low] The commit fixes a user-visible bug: forwarded IPv6 GSO flows
  with destination-options or routing headers stall on Packet Too Big at…

--

ipv6: fix transport header of forwarded GSO packets

This patch resets transport_header in ip6_forward() for GSO packets before
the MTU check. CHECKSUM_PARTIAL packets without encapsulation take it from
csum_start, and other packets with extension headers walk the chain with
ipv6_skip_exthdr(). This keeps skb_gso_validate_network_len() from reading
a bogus TCP header length, which caused Packet Too Big stalls and
SKB_BAD_GSO drops.

> skb_gso_validate_network_len() then reads a bogus TCP header length. An
> overestimate makes ip6_forward() answer with Packet Too Big at the
> current MTU, and the flow stalls. Since commit 7fb4c1967011 ("net: pull
> headers in qdisc_pkt_len_segs_init()"), a bogus doff below 5 also makes
> __dev_queue_xmit() drop the packet as SKB_BAD_GSO.

[ ... ]

> Signed-off-by: Matt Vollrath <tactii@gmail.com>

[Severity: Low]
Should this patch carry a Fixes: tag and a stable Cc? The only pointer to
where the bug came from, fe6cc55f3a9a ("net: ip, ipv6: handle gso skbs in
forwarding path"), is in the notes below the --- separator, and git am
strips those.

Stable trees that pick up 7fb4c1967011 would start dropping these
forwarded packets as SKB_BAD_GSO. Nothing in the commit points the stable
tooling at this fix.

Would a trailer like this in the commit body work, along with a mention of
7fb4c1967011?

Fixes: fe6cc55f3a9a ("net: ip, ipv6: handle gso skbs in forwarding path")

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 25fe0ba98b1a6..60ac56afbd06b 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -662,6 +662,23 @@ int ip6_forward(struct sk_buff *skb)
>  
>  	__IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTFORWDATAGRAMS);
>  
> +	if (skb_is_gso(skb)) {
> +		if (skb->ip_summed == CHECKSUM_PARTIAL && !skb->encapsulation) {
> +			skb_set_transport_header(skb,
> +						 skb_checksum_start_offset(skb));
> +		} else if (ipv6_ext_hdr(hdr->nexthdr)) {
> +			u8 nexthdr = hdr->nexthdr;
> +			__be16 frag_off;
> +			int thoff;
> +
> +			thoff = ipv6_skip_exthdr(skb, skb_network_offset(skb) +
> +						 sizeof(*hdr), &nexthdr,
> +						 &frag_off);
> +			if (thoff >= 0)
> +				skb_set_transport_header(skb, thoff);

[Severity: High]
Can the CHECKSUM_PARTIAL branch move transport_header backwards, into
the fixed IPv6 header or even in front of skb->data?

csum_start is taken as is here. Nothing checks that
skb_checksum_start_offset(skb) is at least skb_network_offset(skb) +
sizeof(*hdr). Before this patch, the forwarding path kept the
transport_header set by ip6_rcv_core(), which never pointed inside the
fixed header.

For packets injected through tun/tap, macvtap or af_packet with
VIRTIO_NET_HDR_F_NEEDS_CSUM, csum_start comes from userspace. The check
in __virtio_net_hdr_to_skb() runs while skb->data still points at the
Ethernet header:

include/linux/virtio_net.h:__virtio_net_hdr_to_skb() {
    ...
		if (!skb_partial_csum_set(skb, start, off))
			return -EINVAL;
		if (skb_transport_offset(skb) < nh_min_len)
			return -EINVAL;
    ...
}

nh_min_len is sizeof(struct ipv6hdr) for VIRTIO_NET_HDR_GSO_TCPV6, so a
csum_start of 40 is accepted. After eth_type_trans() pulls the 14 byte
Ethernet header, that is offset 26 from the network header, in the
middle of the source address. A routed TCPV6 GSO packet built this way
takes this branch and gets transport_header set inside the IPv6
header. From there:

ip6_forward()
  ip6_pkt_too_big()
    skb_gso_validate_network_len()
      skb_gso_network_seglen()
        skb_gso_transport_seglen()
          thlen = tcp_hdrlen(skb);

This reads doff out of the address bytes. The network header length
comes out 14 bytes short and thlen is garbage, so the MTU checks here
and in ip6_finish_output_gso() use the wrong segment length.
qdisc_pkt_len_segs_init() later reads the same bogus doff.

It gets worse if the offset goes negative. bpf_prog_run_generic_xdp()
calls __skb_pull() after bpf_xdp_adjust_head() without adjusting
csum_start, so ip_summed can stay CHECKSUM_PARTIAL with csum_start
behind skb->data. A negative transport offset wraps the unsigned
arithmetic in skb_gso_network_seglen(), which lets the packet past the
MTU checks. Then, in qdisc_pkt_len_segs_init():

net/core/dev.c:qdisc_pkt_len_segs_init() {
    ...
		hdr_len = skb_transport_offset(skb);
    ...
		th = (const struct tcphdr *)(skb->data + hdr_len);
    ...
}

hdr_len is an unsigned int. With a transport offset between -20 and -1
the length passed to pskb_may_pull() wraps to a small value that
passes, and th ends up about 4 GiB past skb->data on 64-bit. Reading
th->doff there would oops.

Would it be enough to use csum_start only when
skb_checksum_start_offset(skb) >= skb_network_offset(skb) +
sizeof(*hdr), and fall back to the ipv6_skip_exthdr() walk otherwise?

[Severity: Medium]
Can this make tcp_hdrlen() read past the linear data of the skb?

For each extension header, ipv6_skip_exthdr() reads only the 2-byte
generic header through skb_header_pointer(). After the last one it adds
hdrlen without a bounds check, and for destination or routing options
hdrlen can be up to 2048 bytes:

net/ipv6/exthdrs_core.c:ipv6_skip_exthdr() {
    ...
		nexthdr = hp->nexthdr;
		start += hdrlen;
    ...
}

So thoff can point into the page frags or past skb->len. The only check
here is thoff >= 0.

Right after this, the MTU check reads the TCP header directly at the new
offset:

ip6_forward()
  ip6_pkt_too_big()
    skb_gso_validate_network_len()
      skb_gso_network_seglen()
        skb_gso_transport_seglen()
          thlen = tcp_hdrlen(skb);

tcp_hdrlen() reads tcp_hdr(skb)->doff with no pskb_may_pull() and no
skb_header_pointer().

The else branch runs for TCP GSO skbs that are neither CHECKSUM_PARTIAL
nor encapsulated. Two examples are fraglist GRO (CHECKSUM_UNNECESSARY) and
virtio_net_hdr GSO packets without NEEDS_CSUM.

On ingress, only the extension header chain that was actually parsed gets
linearized. Suppose the chain is rewritten before ip6_forward(), for
example by nftables payload mangling in PREROUTING or by a tc BPF
store_bytes. Then the new thoff can land far past skb_headlen(), possibly
past skb->end. Both of those need CAP_NET_ADMIN, which an unprivileged
user namespace has inside its own netns.

Before this patch, transport_header on this path pointed just past the
fixed header, or past a hop-by-hop header that ipv6_parse_hopopts() had
already pulled. Either way the read stayed inside the linear data.

Would a pskb_may_pull(skb, thoff + sizeof(struct tcphdr)) check for TCP
GSO cover this, keeping the old transport_header if the pull fails?

> +		}
> +	}
> +
>  	mtu = ip6_dst_mtu_maybe_forward(dst, true);
>  	if (mtu < IPV6_MIN_MTU)
>  		mtu = IPV6_MIN_MTU;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105201309.3182307-1-tactii%40gmail.com

Re: [PATCH net-next 2/4] ipv6: sr: keep transport header after inline SRH insertion

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit fixes a regression users can see but has no Fixes:
  tag.
- [Low] The new save/restore in seg6_output_core() assumes the transport
  header sits at or after offset sizeof(struct ipv6hdr), where…

Pre-existing issues:
- [Low] Concern: the same defect (transport_header left on a newly
  inserted IPv6 extension header) remains in…

--

ipv6: sr: keep transport header after inline SRH insertion

On the output path, this patch keeps transport_header pointing at the L4
header after seg6_do_srh() inserts an SRH in inline mode. Before the
insertion it saves the distance from the transport header to the end of
the packet, and afterwards it restores it. GSO length checks then no longer
read the SRH as a TCP header.

> In inline mode, seg6_do_srh() leaves transport_header at the inserted
> SRH, although the L4 header has not moved. GSO length checks then read
> the SRH as TCP, and since commit 7fb4c1967011 ("net: pull headers in
> qdisc_pkt_len_segs_init()"), a bogus doff below 5 there makes
> __dev_queue_xmit() drop each GSO packet of the flow as SKB_BAD_GSO.

[Severity: Low]
Should this have a Fixes: tag?

On an inline seg6 route, every GSO packet of the flow gets dropped here:

net/core/dev.c:qdisc_pkt_len_segs_init() {
    ...
		tlen = __tcp_hdrlen(th);
		if (tlen < sizeof(*th))
			return SKB_DROP_REASON_SKB_BAD_GSO;
    ...
}

The commit message mentions 7fb4c1967011 only in prose. Commit
6c8702c60b88 ("ipv6: sr: add support for SRH encapsulation and injection
with lwtunnels"), which placed transport_header at the SRH, appears only
below the --- line, and git-am strips that part.

Without a Fixes: trailer, stable backports may miss this. The other
patches in the series seem to have the same gap.

> diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
> index 0d39a38a5dd29..942704eca8db0 100644
> --- a/net/ipv6/seg6_iptunnel.c
> +++ b/net/ipv6/seg6_iptunnel.c

[ ... ]

> @@ -684,10 +685,17 @@ static int seg6_output_core(struct net *net, struct sock *sk,
>  	dst = dst_cache_get(&slwt->cache_output);
>  	local_bh_enable();
>  
> +	if (slwt->tuninfo->mode == SEG6_IPTUN_MODE_INLINE &&
> +	    skb_transport_header_was_set(skb))
> +		tail_len = skb->len - skb_transport_offset(skb);
> +
>  	err = seg6_do_srh(skb, dst);
>  	if (unlikely(err))
>  		goto drop;
>  
> +	if (tail_len >= 0)
> +		skb_set_transport_header(skb, skb->len - tail_len);
> +

[Severity: Low]
Does this restore assume the transport header is at or past offset
sizeof(struct ipv6hdr)?

__seg6_do_srh_inline() pulls the 40-byte IPv6 header and pushes 40 + hdrlen
bytes. It then moves the IPv6 header to the new front and copies the SRH in
at offset 40. Only bytes at offset 40 or later keep their distance to the
end of the packet.

rawv6_send_hdrinc() leaves the transport offset at 0:

net/ipv6/raw.c:rawv6_send_hdrinc() {
    ...
	skb->transport_header = skb->network_header;
    ...
}

The packet then goes through NF_HOOK(LOCAL_OUT, ..., dst_output) to
seg6_output_core(). With an inline seg6 route, tail_len becomes skb->len,
and after the insertion transport_header ends up at offset hdrlen:

- With a single-segment SRH (24 bytes), that is inside daddr of the IPv6
  header.
- With 3 segments (56 bytes), it is 16 bytes into the SRH.

Before this patch, seg6_do_srh() left it at offset 40, on a header
boundary.

These packets are neither GSO nor CHECKSUM_PARTIAL. The effect is
therefore limited to egress code that uses skb_transport_offset()
directly, such as act_pedit layered keys or BPF.

Could the guard also require skb_transport_offset(skb) >=
sizeof(struct ipv6hdr) before tail_len is saved?

The later series patches "ipv6: rpl: keep transport header after SRH
insertion" and "ipv6: ioam: keep transport header after inline insertion"
seem to copy the same unguarded check into rpl_output() and
ioam6_output().

[Severity: Low]
This is a pre-existing issue, but the same pattern exists in other places.
In each of them, transport_header is left on a newly inserted extension
header:

- rpl_output()->rpl_do_srh_inline()
- ioam6_output()->ioam6_do_inline()
- input_action_end_b6() in net/ipv6/seg6_local.c:

net/ipv6/seg6_local.c:input_action_end_b6() {
    ...
	err = seg6_do_srh_inline(skb, slwt->srh);
	if (err)
		goto drop;

	skb_set_transport_header(skb, sizeof(struct ipv6hdr));

	seg6_lookup_nexthop(skb, NULL, 0);
    ...
}

The rest of the series appears to fix these cases:

- "ipv6: rpl: keep transport header after SRH insertion" adds the same
  tail_len save/restore to rpl_output().
- "ipv6: ioam: keep transport header after inline insertion" adds it to
  ioam6_output() for inline mode.
- input_action_end_b6() is an input path. "ipv6: fix transport header of
  forwarded GSO packets" corrects transport_header in ip6_forward() for
  forwarded GSO packets, and local delivery parses extension headers
  starting at the SRH.

Is there another caller of these inline insertion helpers that the series
leaves uncovered, or are all of them handled once the remaining patches
are applied?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105201309.3182307-1-tactii%40gmail.com

Re: [PATCH net-next 3/4] ipv6: rpl: keep transport header after SRH insertion

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message has no Fixes: tag, although the patch fixes a
  user-visible regression.
- [Low] In rpl_output() (net/ipv6/rpl_iptunnel.c), the new restore keeps
  transport_header at the same distance from the end of the packet.

--

ipv6: rpl: keep transport header after SRH insertion

After rpl_do_srh() inserts the RPL SRH, rpl_output() now restores
skb->transport_header from its distance to the end of the packet.
Before this, transport_header was left at the SRH. Since commit
7fb4c1967011, that causes __dev_queue_xmit() to drop GSO packets of
the flow as SKB_BAD_GSO.

> rpl_do_srh_inline() leaves transport_header at the inserted RPL SRH,
> although the L4 header has not moved. GSO length checks then read the
> SRH as TCP, and since commit 7fb4c1967011 ("net: pull headers in
> qdisc_pkt_len_segs_init()"), a bogus doff below 5 there makes
> __dev_queue_xmit() drop each GSO packet of the flow as SKB_BAD_GSO.

[ ... ]

> Signed-off-by: Matt Vollrath <tactii@gmail.com>

[Severity: Low]
Should this patch have a Fixes: tag?

The note below the --- line says transport_header has pointed at the
inserted SRH since commit a7a29f9c361f ("net: ipv6: add rpl sr tunnel").
That note is dropped when the patch is applied, so git never records it.

Commit 7fb4c1967011 carries "Fixes: e876f208af18", so it is likely to be
backported to stable. Stable trees that take it would start dropping RPL
GSO flows in qdisc_pkt_len_segs_init(). Without a Fixes: tag, stable
tooling might not pick up this fix along with it.

Perhaps something like:

Fixes: a7a29f9c361f ("net: ipv6: add rpl sr tunnel")

The sibling patch "ipv6: sr: keep transport header after inline SRH
insertion" seems to have the same omission.

> diff --git a/net/ipv6/rpl_iptunnel.c b/net/ipv6/rpl_iptunnel.c
> index 4e10adcd70e89..0556bb9890410 100644
> --- a/net/ipv6/rpl_iptunnel.c
> +++ b/net/ipv6/rpl_iptunnel.c

[ ... ]

> @@ -214,10 +215,16 @@ static int rpl_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>  	dst = dst_cache_get(&rlwt->cache);
>  	local_bh_enable();
>  
> +	if (skb_transport_header_was_set(skb))
> +		tail_len = skb->len - skb_transport_offset(skb);
> +
>  	err = rpl_do_srh(skb, rlwt, dst);
>  	if (unlikely(err))
>  		goto drop;
>  
> +	if (tail_len >= 0)
> +		skb_set_transport_header(skb, skb->len - tail_len);
> +

[Severity: Low]
Does the distance to the end of the packet stay the same when the
original transport header was inside the 40-byte IPv6 header?

rpl_do_srh_inline() pulls the IPv6 header and pushes 40 + hdrlen bytes.
It then moves the old IPv6 header to the new front. Only bytes after the
original IPv6 header keep their distance to the end of the packet.

An IPV6_HDRINCL raw socket can get here with a transport offset of 0:

net/ipv6/raw.c:rawv6_send_hdrinc() {
    ...
	skb->transport_header = skb->network_header;
    ...
}

rawv6_send_hdrinc()
  NF_HOOK(NFPROTO_IPV6, NF_INET_LOCAL_OUT, ..., dst_output)
    dst_output()
      lwtunnel_output()
        rpl_output()

In that case tail_len is skb->len. After the insertion skb->len has grown
by hdrlen, so skb_set_transport_header() sets the offset to hdrlen. If
the compressed SRH is shorter than 40 bytes, that points into the IPv6
address fields. If it is longer, it points into the SRH.

Before this patch the offset was sizeof(struct ipv6hdr), the start of
the SRH, which is at least a header boundary:

net/ipv6/rpl_iptunnel.c:rpl_do_srh_inline() {
    ...
	skb_set_transport_header(skb, sizeof(struct ipv6hdr));
    ...
}

These raw packets are CHECKSUM_NONE and not GSO, so the SKB_BAD_GSO path
does not apply to them. Egress users of skb_transport_offset(), such as
act_pedit with TCP/UDP header types, would still see the new non-boundary
offset.

The SRH decap code in seg6_local handles a transport header that is
before, inside or after the removed header. Could the restore here be
limited to the case where the old skb_transport_offset(skb) is at least
sizeof(struct ipv6hdr)?

seg6_output_core() in "ipv6: sr: keep transport header after inline SRH
insertion" and ioam6_output() in "ipv6: ioam: keep transport header after
inline insertion" use the same unguarded tail_len pattern.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105201309.3182307-1-tactii%40gmail.com

Re: [PATCH net-next 4/4] ipv6: ioam: keep transport header after inline insertion

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The patch fixes a user-visible regression but has no Fixes: tag.
- [Low] In ioam6_output() (net/ipv6/ioam6_iptunnel.c:379-387), the new
  restore of transport_header assumes the original transport header sits…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it is the same
  failure in the same function ioam6_output().

--

ipv6: ioam: keep transport header after inline insertion

After ioam6_do_inline() runs, the patch puts transport_header back where
it was. It does this by saving the L4 header's distance from the end of
the packet before the insertion. The goal is to stop GSO length checks,
such as qdisc_pkt_len_segs_init(), from reading the inserted hop-by-hop
header as TCP and dropping packets as SKB_BAD_GSO.

> read that header as TCP, and since commit 7fb4c1967011 ("net: pull
> headers in qdisc_pkt_len_segs_init()"), a bogus doff below 5 there makes
> __dev_queue_xmit() drop each GSO packet of the flow as SKB_BAD_GSO. This

[Severity: Low]
Should this patch carry a Fixes: tag?

The commit message names 7fb4c1967011, which added the SKB_BAD_GSO drop
in qdisc_pkt_len_segs_init() in net/core/dev.c. The notes below the ---
line name 3edede08ff37 ("ipv6: ioam: Support for IOAM injection with
lwtunnels"). Those notes are discarded when the patch is applied, and
neither commit is listed as a Fixes: tag.

Without one, stable trees that contain 7fb4c1967011 may not pick up this
fix.

The sibling patches in the series also have no Fixes: tags: "ipv6: sr:
keep transport header after inline SRH insertion", "ipv6: rpl: keep
transport header after SRH insertion" and "ipv6: fix transport header of
forwarded GSO packets".

> diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c
> index cfb2c41634a0b..27b47f0c8bf8f 100644
> --- a/net/ipv6/ioam6_iptunnel.c
> +++ b/net/ipv6/ioam6_iptunnel.c

[ ... ]

> @@ -375,10 +376,16 @@ static int ioam6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>  		if (ipv6_hdr(skb)->nexthdr == NEXTHDR_HOP)
>  			goto out;
>  
> +		if (skb_transport_header_was_set(skb))
> +			tail_len = skb->len - skb_transport_offset(skb);
> +
>  		err = ioam6_do_inline(net, skb, &ilwt->tuninfo, dst);
>  		if (unlikely(err))
>  			goto drop;
>  
> +		if (tail_len >= 0)
> +			skb_set_transport_header(skb, skb->len - tail_len);
> +

[Severity: Low]
Does this restore in ioam6_output() assume that the original transport
header starts at or after offset sizeof(struct ipv6hdr)?

ioam6_do_inline() moves the first 40 bytes to the new data start and
inserts the hop-by-hop header right after them:

    hdr = ipv6_hdr(skb);
    memmove(hdr, oldhdr, sizeof(*oldhdr));

So only bytes at offset 40 or later keep their distance from the end of
the packet.

rawv6_send_hdrinc() in net/ipv6/raw.c sets:

    skb->transport_header = skb->network_header;

so skb_transport_offset() is 0 by the time the packet reaches
ioam6_output() via NF_HOOK(..., dst_output)->lwtunnel_output(). In inline
mode, and in auto mode because skb->dev is NULL, tail_len becomes
skb->len. After the insertion, transport_header ends up at hdrlen.

hdrlen is a multiple of 8 and at least 16. That puts transport_header in
the middle of the new IPv6 header, or inside the hop-by-hop TLVs, rather
than on a header boundary. Before this patch it pointed at the start of
the hop-by-hop header, at offset 40.

These packets are not GSO, so the wrong offset may only be seen by code
that reads skb->transport_header directly, such as tracing or BPF.

Would checking skb_transport_offset(skb) >= sizeof(struct ipv6hdr) before
saving tail_len keep the old placement in this case? The sibling seg6 and
rpl patches in this series appear to use the same pattern.

>  		break;
>  	case IOAM6_IPTUNNEL_MODE_ENCAP:
>  do_encap:

[Severity: High]
This isn't a bug introduced by this patch, but can the encap branch of
ioam6_output() still drop TCP GSO packets the same way?

ioam6_do_encap() does:

    skb_push(skb, len);
    skb_reset_network_header(skb);
    skb_mac_header_rebuild(skb);
    skb_set_transport_header(skb, sizeof(*hdr));

That leaves transport_header pointing at the outer hop-by-hop header.
ioam6_do_encap() never calls iptunnel_handle_offloads(), and it never sets
skb->encapsulation, the inner transport header, inner_protocol or
SKB_GSO_IPXIP6.

When skb->encapsulation is clear, qdisc_pkt_len_segs_init() does:

    hdr_len = skb_transport_offset(skb);
    ...
    th = (const struct tcphdr *)(skb->data + hdr_len);
    tlen = __tcp_hdrlen(th);
    if (tlen < sizeof(*th))
        return SKB_DROP_REASON_SKB_BAD_GSO;

Here __tcp_hdrlen() reads the first trace-type byte of the hop-by-hop
header. That gives the same trace-type dependent drop that the commit
message describes for inline mode. skb_gso_validate_network_len() in
ip6_finish_output_gso() reads the same bogus doff.

When software segmentation is needed, ipv6_gso_segment() skips the
hop-by-hop header, finds IPPROTO_IPV6 and calls ip6ip6_gso_segment(),
which does:

    if (!(skb_shinfo(skb)->gso_type & SKB_GSO_IPXIP6))
        return ERR_PTR(-EINVAL);

This fails whatever the trace type is.

The commit message says the bug "affects forwarded packets too". In
IOAM6_IPTUNNEL_MODE_AUTO, forwarded packets have skb->dev set and take
do_encap instead of do_inline:

    ip6_forward()->dst_output()->lwtunnel_output()->ioam6_output()
        ->ioam6_do_encap()

Would forwarded TCP GSO traffic in auto mode, and any traffic in encap
mode, still be dropped after this patch?

seg6_do_srh() in net/ipv6/seg6_iptunnel.c calls
iptunnel_handle_offloads(skb, SKB_GSO_IPXIP6) and sets the inner
transport header and inner protocol. Does ioam6_do_encap() need something
similar?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105201309.3182307-1-tactii%40gmail.com