Re: [PATCH net v2 2/2] seg6: check lwtunnel state after nf hooks in iptunnel

[Andrea Mayer]

On Tue, 28 Jul 2026 21:54:48 +0000
"Xiang Mei (Microsoft)" <xmei5@xxxxxxx> wrote:

The code looks good. As in patch 1/2, checking the encap type is reasonable
for this fix too (checking the instance would need new machinery).

> seg6_input_core() and seg6_output_core() are the okfns of the
> POST_ROUTING hook that seg6_input_nf() and seg6_output_nf() dispatch
> through when nf_hooks_lwtunnel is enabled, and they read
> skb_dst(skb)->lwtstate on the assumption fixed for seg6local in patch 1.
> A hook may drop the dst, replace it with a metadata dst, or leave a route
> whose lwtstate is NULL; the last is reachable with SNAT plus an XFRM
> policy.
> 

One thing on the commit message. It lists the cases (drop, metadata dst,
lwtstate == NULL) but not what they cause: they all end in a NULL
dereference and the kernel panics. For that the reader has to follow "the
assumption fixed for seg6local in patch 1", and once applied this message
has to stand on its own. Patch 1/2 explains the failure and shows a trace.
This one has neither, so I would expect at least one sentence saying what
happens.

> Validate the dst and the encap type before use. seg6_do_srh() reads
> skb_dst(skb)->lwtstate too, but these two are its only callers and
> neither touches the dst in between.
> 

Sashiko also asks whether the validated lwtstate could be handed down to
seg6_do_srh(), instead of the re-read it does now. That would help if the
dst were replaced by a later change, but not if it were dropped: in the
encap modes __seg6_do_srh_encap() reads skb_dst(skb) on its own. And
nothing changes the dst between seg6_lwtst_from_skb() and the call to
seg6_do_srh(), so that re-read is fine too.

> In seg6_output_core() the validated lwtstate also takes over the dst-loop
> comparison, so "orig_dst" is gone, matching seg6_input_core().
> 
> Fixes: 7a3f5b0de364 ("netfilter: add netfilter hooks to SRv6 data plane")
> Reported-by: Andrea Mayer <andrea.mayer@xxxxxxxxxxx>
> Signed-off-by: Xiang Mei (Microsoft) <xmei5@xxxxxxx>
> ---
> v2: new patch, split out from the seg6local fix (Andrea Mayer).
> 
>  net/ipv6/seg6_iptunnel.c | 33 +++++++++++++++++++++++++++------
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> [snip]

Thanks,

Ciao,
Andrea