netdev - Re: [PATCH net v2 3/3] net: ipv6: fix consecutive input and output transformation in lwtunnels

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <231856f0-deb8-4550-bdf3-b0ef065f7b7b@redhat.com>
Date: Thu, 13 Feb 2025 15:33:21 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Justin Iurman <justin.iurman@...ege.be>, netdev@...r.kernel.org
Cc: davem@...emloft.net, dsahern@...nel.org, edumazet@...gle.com,
 kuba@...nel.org, horms@...nel.org
Subject: Re: [PATCH net v2 3/3] net: ipv6: fix consecutive input and output
 transformation in lwtunnels

On 2/11/25 11:16 PM, Justin Iurman wrote:
> Some lwtunnel users implement both lwt input and output handlers. If the
> post-transformation destination on input is the same, the output handler
> is also called and the same transformation is applied (again). Here are
> the users: ila, bpf, rpl, seg6. The first one (ila) does not need this
> fix, since it already implements a check to avoid such a duplicate. The
> second (bpf) may need this fix, but I'm not familiar with that code path
> and will keep it out of this patch. The two others (rpl and seg6) do
> need this patch.
> 
> Due to the ila implementation (as an example), we cannot fix the issue
> in lwtunnel_input() and lwtunnel_output() directly. Instead, we need to
> do it on a case-by-case basis. This patch fixes both rpl_iptunnel and
> seg6_iptunnel users. The fix re-uses skb->redirected in input handlers
> to notify corresponding output handlers that the transformation was
> already applied and to skip it. The "redirected" field seems safe to be
> used here.
> 
> Fixes: a7a29f9c361f ("net: ipv6: add rpl sr tunnel")
> Fixes: 6c8702c60b88 ("ipv6: sr: add support for SRH encapsulation and injection with lwtunnels")
> Signed-off-by: Justin Iurman <justin.iurman@...ege.be>
> ---
>  net/ipv6/rpl_iptunnel.c  | 14 ++++++++++++--
>  net/ipv6/seg6_iptunnel.c | 16 +++++++++++++---
>  2 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/net/ipv6/rpl_iptunnel.c b/net/ipv6/rpl_iptunnel.c
> index dc004e9aa649..2dc1f2297e39 100644
> --- a/net/ipv6/rpl_iptunnel.c
> +++ b/net/ipv6/rpl_iptunnel.c
> @@ -208,6 +208,12 @@ static int rpl_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>  	struct rpl_lwt *rlwt;
>  	int err;
>  
> +	/* Don't re-apply the transformation when rpl_input() already did it */
> +	if (skb_is_redirected(skb)) {

This check looks false-positive prone, i.e. if packet lands on an LWT
tunnel due to an tc redirect from another non lwt device.

On the flip side I don't see any good method to propagate the relevant
information. A skb ext would work, but I would not call that a good method.

/P