netdev - Re: [PATCH net-next 1/2] net/sched: act

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM0EoMm4UZ9cM6zOTH+uT1kwyMdgEsP2BPR3C+d_-nmbXfrYyQ@mail.gmail.com>
Date: Wed, 26 Nov 2025 13:14:07 -0500
From: Jamal Hadi Salim <jhs@...atatu.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net, pabeni@...hat.com, 
	jiri@...nulli.us, xiyou.wangcong@...il.com, netdev@...r.kernel.org, 
	dcaratti@...hat.com
Subject: Re: [PATCH net-next 1/2] net/sched: act_mirred: Fix infinite loop

On Wed, Nov 26, 2025 at 11:41 AM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Wed, Nov 26, 2025 at 8:26 AM Jamal Hadi Salim <jhs@...atatu.com> wrote:
> >
> > On Tue, Nov 25, 2025 at 11:20 AM Jamal Hadi Salim <jhs@...atatu.com> wrote:
> > >
> > > On Mon, Nov 24, 2025 at 5:51 PM Jakub Kicinski <kuba@...nel.org> wrote:
> > > >
> > > > On Mon, 24 Nov 2025 15:08:24 -0500 Jamal Hadi Salim wrote:
> > > > > When doing multiport mirroring we dont detect infinite loops.
> > > > >
> > > > > Example (see the first accompanying tdc test):
> > > > > packet showing up on port0 ingress mirred redirect --> port1 egress
> > > > > packet showing up on port1 egress mirred redirect --> port0 ingress
> > > > >
> > > > > Example 2 (see the second accompanying tdc test)
> > > > > port0 egress --> port1 ingress --> port0 egress
> > > > >
> > > > > Fix this by remembering the source dev where mirred ran as opposed to
> > > > > destination/target dev
> > > > >
> > > > > Fixes: fe946a751d9b ("net/sched: act_mirred: add loop detection")
> > > > > Signed-off-by: Jamal Hadi Salim <jhs@...atatu.com>
> > > >
> > > > Hm, this breaks net/fib_tests.sh:
> > > >
> > > > # 23.80 [+0.00] IPv4 rp_filter tests
> > > > # 25.63 [+1.84]     TEST: rp_filter passes local packets                                [FAIL]
> > > > # 26.65 [+1.02]     TEST: rp_filter passes loopback packets                             [FAIL]
> > > >
> > > > https://netdev-3.bots.linux.dev/vmksft-net/results/400301/10-fib-tests-sh/stdout
> > > >
> > > > Not making a statement on whether the fix itself is acceptable
> > > > but if it is we gotta fix that test too..
> > >
> > > Sigh. I will look into it later.
> > > Note: Fixing this (and the netem loop issue) would have been trivial
> > > if we had those two skb ttl fields that were taken away.
> > > The human hours spent trying to detect and prevent infinite loops!
> > >
> >
> > Ok, I spent time on this and frankly cant find a way to fix the
> > infinite loop that avoids adding _a lot more_ complexity.
> > We need loop state to be associated with the skb. I will restore the
> > two skb bits and test. From inspection, i see one bit free but i may
> > be able to steal a bit from somewhere. I will post an RFC and at
> > minimal that will start a discussion and maybe someone will come up
> > with a better way of solving this.
> >
> > Eric, there's another issue as well involving example
> > port0:egress->port0:egress  - I have a patch but will post it later
> > after some testing.
>
> Adding bits for mirred in skb would be quite unfortunate.
> Argument of 'we have available space, let's use/waste it' is not very appealing.
>
> Do we really need to accept more than one mirred ?
>
> What is a legitimate/realistic use for MIRRED_NEST_LIMIT == 4 ?

It's the multiport redirection, particularly to ingress. When it get
redirected to ingress it will get queued and then transitioned back.
xmit struct wont catch this as a recursion, so MIRRED_NEST_LIMIT will
not help you.
Example (see the first accompanying tdc test):
packet showing up on port0:ingress mirred redirect --> port1:egress
packet showing up on port1:egress mirred redirect --> port0:ingress

cheers,
jamal