Re: [PATCH net v3] ipv6: seg6: clear IPv4 control block on IPIP decapsulation

[Andrea Mayer <andrea.mayer@uniroma2.it>]

On Mon, 10 Aug 2026 15:47:31 +0000
David Lee <david.lee@trailofbits.com> wrote:

Thanks Kyle and David for the v3. The code looks good. What follows is
about the commit message.

I think the commit message is missing some information that the review
needs now, and that anyone reading git log will need later, since this
patch is Cc: stable.

> From: Kyle Zeng <kylebot@openai.com>
> 
> End.DX4 and End.DT4 decapsulate an IPv4 packet through
> decap_and_validate() and send it directly to IPv4 routing. The inner
> packet therefore bypasses ip_rcv_core(), which normally clears IPCB
> before IPv4 interprets skb->cb.
> 
> The skb instead retains IP6CB data from the outer packet. IPv6
> extension-header offsets overlap IPv4 option fields, so IPv4 can treat
> those offsets as saved option metadata. __ip_options_echo() can then
> copy beyond the allocation for saved options.
> 

This says the offsets overlap, but not which stale field has to be
non-zero for the copy to be attempted. IPCB->opt.optlen shares a byte
with IP6CB->lastopt, and __ip_options_echo() returns before any copy
when the optlen it is given is zero. What the outer packet has to look
like for that byte to be non-zero, and whether the sender controls it,
are not stated.

> Separate End.DX4 and End.DT4 reproducers on the unpatched v7.2-rc5
> kernel both produced:
> 
>   BUG: KASAN: slab-out-of-bounds in __ip_options_echo()
>   Write of size 255
> 
> The End.DX4 trace passes through input_action_end_dx4_finish() and
> input_action_end_dx4(), while the End.DT4 trace passes through
> input_action_end_dt4().
> 

Pasting the call stack would show how __ip_options_echo() is reached.

> When decap_and_validate() handles IPPROTO_IPIP, save the ingress
> interface from IP6CB, clear IPCB, and restore the saved value. Doing
> this in the common decapsulation path covers End.DX4, End.DT4, and
> End.DT46's IPv4 arm.
> 
> Use IP6CB(skb)->iif rather than skb->skb_iif. These actions run after
> l3mdev processing, which can replace skb_iif with the L3 master;
> IP6CB iif still records the receiving interface set at IPv6 ingress.
> 
> Fixes: 891ef8dd2a8d ("ipv6: sr: implement additional seg6local actions")
> Cc: stable@vger.kernel.org
> Suggested-by: Andrea Mayer <andrea.mayer@uniroma2.it>
> Assisted-by: Codex:gpt-5.6-sol Codex:gpt-5.5-cyber
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> Co-developed-by: David Lee <david.lee@trailofbits.com>
> Signed-off-by: David Lee <david.lee@trailofbits.com>
> ---
> Changes in v3:
> - Clear IPCB in the common IPPROTO_IPIP decapsulation path so End.DX4,
>   End.DT4, and End.DT46's IPv4 arm are covered.
> - Preserve the ingress interface from IP6CB instead of skb->skb_iif,
>   which can identify the VRF master after l3mdev processing.
> - Update the Fixes tag to the commit that introduced End.DX4.
> - Include the End.DX4 and End.DT4 KASAN evidence.
> 
> v2: https://lore.kernel.org/netdev/20260804094625.715305-1-david.lee@trailofbits.com/
> v1: https://lore.kernel.org/all/20260731140832.567669-1-david.lee@trailofbits.com/ 
> 
>  net/ipv6/seg6_local.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
> index 2b41e4c0dddd..95ea0b62729a 100644
> --- a/net/ipv6/seg6_local.c
> +++ b/net/ipv6/seg6_local.c
> @@ -256,6 +256,13 @@ static bool decap_and_validate(struct sk_buff *skb, int proto)
>  	if (iptunnel_pull_offloads(skb))
>  		return false;
>  
> +	if (proto == IPPROTO_IPIP) {
> +		int iif = IP6CB(skb)->iif;
> +
> +		memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
> +		IPCB(skb)->iif = iif;
> +	}
> +
>  	return true;
>  }
>  

IMHO, the commit message is worth a new revision.

Thanks,

Ciao,
Andrea