[PATCH] seg6: fix OOB read in advance_nextseg when segments_left is 0

[PATCH] seg6: fix OOB read in advance_nextseg when segments_left is 0

[Tang Bin]

advance_nextseg() decrements srh->segments_left and uses it to index
srh->segments without checking it is non-zero. seg6_validate_srh() permits
segments_left == 0, so a packet with SRH segments_left==0 reaching an End*
action underflows the u8 (255) and reads srh->segments[255] (~4KB out of
bounds); the read bytes are then used as the new IPv6 destination address
(info leak) or crash on unmapped memory. The decap path
(decap_and_validate) already checks segments_left > 0, but the standard
End/End.X/End.T/End.B6.Encap/End.BPF paths do not. Reject segments_left ==
0 before decrementing.

Fixes: d7a669dd2f8b ("ipv6: sr: add helper functions for seg6local")
Cc: stable@vger.kernel.org
Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
---
 net/ipv6/seg6_local.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index 53f7b2452..1d8312467 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -310,6 +310,10 @@ static struct ipv6_sr_hdr *advance_nextseg(struct sk_buff *skb,
 	/* skb_ensure_writable() may change skb pointers; evaluate srh again */
 	srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
 
+	if (srh->segments_left == 0) {
+		*reason = SKB_DROP_REASON_NOT_SPECIFIED;
+		return NULL;
+	}
 	srh->segments_left--;
 	addr = srh->segments + srh->segments_left;
 	ipv6_hdr(skb)->daddr = *addr;
-- 
2.52.0.windows.1

Re: [PATCH] seg6: fix OOB read in advance_nextseg when segments_left is 0

[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.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

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] seg6: fix OOB read in advance_nextseg when segments_left is 0

[Gabriel Goller]

On 28.10.2026 19:31, Tang Bin wrote:
> advance_nextseg() decrements srh->segments_left and uses it to index
> srh->segments without checking it is non-zero. seg6_validate_srh() permits
> segments_left == 0, so a packet with SRH segments_left==0 reaching an End*
> action underflows the u8 (255) and reads srh->segments[255] (~4KB out of
> bounds); the read bytes are then used as the new IPv6 destination address
> (info leak) or crash on unmapped memory. The decap path
> (decap_and_validate) already checks segments_left > 0, but the standard
> End/End.X/End.T/End.B6.Encap/End.BPF paths do not. Reject segments_left ==
> 0 before decrementing.
> 
> Fixes: d7a669dd2f8b ("ipv6: sr: add helper functions for seg6local")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
> ---
>  net/ipv6/seg6_local.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
> index 53f7b2452..1d8312467 100644
> --- a/net/ipv6/seg6_local.c
> +++ b/net/ipv6/seg6_local.c
> @@ -310,6 +310,10 @@ static struct ipv6_sr_hdr *advance_nextseg(struct sk_buff *skb,
>  	/* skb_ensure_writable() may change skb pointers; evaluate srh again */
>  	srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
>  
> +	if (srh->segments_left == 0) {
> +		*reason = SKB_DROP_REASON_NOT_SPECIFIED;
> +		return NULL;
> +	}
>  	srh->segments_left--;
>  	addr = srh->segments + srh->segments_left;
>  	ipv6_hdr(skb)->daddr = *addr;
> -- 
> 2.52.0.windows.1

Can you show a codepath where it is possible to call advance_nextseg() with
srh->segments_left being 0?

End/End.X/End.T/End.B6.Encap/End.BPF behaviors call get_and_validate_srh(),
which calls seg6_get_srh(skb, IP6_FH_F_SKIP_RH), which uses ipv6_find_hdr(),
which skips all the segments_left=0 headers.

Thanks,
Gabriel

Re: [PATCH] seg6: fix OOB read in advance_nextseg when segments_left is 0

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · 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).

- [Medium] The commit message describes a bug that the current tree does
  not have.
- [Low] The patch adds a second NULL return to advance_nextseg() in
  net/ipv6/seg6_local.c, for segments_left == 0, with drop reason…

--

seg6: fix OOB read in advance_nextseg when segments_left is 0

This commit adds a segments_left == 0 check to advance_nextseg() in
net/ipv6/seg6_local.c. The check runs before the field is decremented and
used to index srh->segments. The commit message says the End, End.X,
End.T, End.B6.Encap and End.BPF paths can reach advance_nextseg() with
segments_left == 0 and read out of bounds.

> advance_nextseg() decrements srh->segments_left and uses it to index
> srh->segments without checking it is non-zero. seg6_validate_srh() permits
> segments_left == 0, so a packet with SRH segments_left==0 reaching an End*
> action underflows the u8 (255) and reads srh->segments[255] (~4KB out of
> bounds); the read bytes are then used as the new IPv6 destination address
> (info leak) or crash on unmapped memory. The decap path
> (decap_and_validate) already checks segments_left > 0, but the standard
> End/End.X/End.T/End.B6.Encap/End.BPF paths do not. Reject segments_left ==
> 0 before decrementing.

[Severity: Medium]
Can any of these paths actually reach advance_nextseg() with
segments_left == 0?

input_action_end_core(), input_action_end_x_core(), input_action_end_t(),
input_action_end_b6_encap() and input_action_end_bpf() all get the SRH
from get_and_validate_srh(), which asks for IP6_FH_F_SKIP_RH:

net/ipv6/seg6_local.c:get_and_validate_srh() {
	srh = seg6_get_srh(skb, IP6_FH_F_SKIP_RH);
	if (!srh)
		return NULL;
    ...
}

When that flag is set, ipv6_find_hdr() skips any routing header with
segments_left == 0:

net/ipv6/exthdrs_core.c:ipv6_find_hdr() {
    ...
			if (flags && (*flags & IP6_FH_F_SKIP_RH) &&
			    rh->segments_left == 0)
				found = false;
    ...
}

So these callers never get back an SRH with segments_left == 0. Nothing
between the lookup and advance_nextseg() writes segments_left, and the
HMAC check only reads it.

end_flv8986_core() is the only caller that uses seg6_get_srh(skb, 0).
There, seg6_get_srh_pktinfo() maps segments_left == 0 to
SEG6_LOCAL_PKTINFO_SL_ZERO. flv8986_act_tbl only has entries for SL_ONE
and SL_MORE:

static const u8 flv8986_act_tbl[FLV8986_ACT_TBL_SIZE] = {
	/* PSP variant for packet where SRH with SL = 1 */
	tbl_cfg(PSP, SL_ONE, F_PSP),
	/* End for packet where the SRH with SL > 1*/
	tbl_cfg(END, SL_MORE, F_PSP),
};

So SL_ZERO resolves to SEG6_LOCAL_FLV_ACT_UNSPEC, and the packet is
dropped before advance_nextseg() is called.

The commit named in the Fixes: tag, d7a669dd2f8b ("ipv6: sr: add helper
functions for seg6local"), also already rejected segments_left == 0
explicitly in get_and_validate_srh().

Given that, is it accurate to say the standard End* paths "do not" check
segments_left? The SKIP_RH lookup flag appears to do that check.

If no path reaches this code with segments_left == 0, should the patch be
described as hardening, without the Fixes: and Cc: stable tags? As
written, it would likely be backported and triaged as a remotely
triggerable OOB read and info leak.

> diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
> index 0c3d1be7f1299..0f72b7fe3ac14 100644
> --- a/net/ipv6/seg6_local.c
> +++ b/net/ipv6/seg6_local.c
> @@ -310,6 +310,10 @@ static struct ipv6_sr_hdr *advance_nextseg(struct sk_buff *skb,
>  	/* skb_ensure_writable() may change skb pointers; evaluate srh again */
>  	srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
>  
> +	if (srh->segments_left == 0) {
> +		*reason = SKB_DROP_REASON_NOT_SPECIFIED;
> +		return NULL;
> +	}

[Severity: Low]
Should the kernel-doc above advance_nextseg() be updated for this new
NULL return? It still says:

 *  - NULL when the skb cannot be made writable. In this case, the function
 *    sets the drop reason to SKB_DROP_REASON_NOMEM.

The callers follow that contract and turn any NULL into -ENOMEM. For
example, in input_action_end_core():

	srh = advance_nextseg(skb, srh, &reason);
	if (!srh) {
		err = -ENOMEM;
		goto drop;
	}

The same pattern appears in input_action_end_x_core(), in both the END
and PSP cases of end_flv8986_core(), in input_action_end_t() and in
input_action_end_b6_encap(). input_action_end_bpf() returns -ENOMEM
directly.

If this branch ever fired, a malformed SRH would be reported as -ENOMEM
with drop reason SKB_DROP_REASON_NOT_SPECIFIED. As noted above, no
current caller appears able to reach it.

>  	srh->segments_left--;
>  	addr = srh->segments + srh->segments_left;
>  	ipv6_hdr(skb)->daddr = *addr;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261028113141.2111-1-tangbin%40cmss.chinamobile.com