netdev - Re: [PATCH net 0/6] net/sched: Fix packet loops in mirred and netem

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: <20260112222017.3d1da4c9@phoenix.local>
Date: Mon, 12 Jan 2026 22:20:17 -0800
From: Stephen Hemminger <stephen@...workplumber.org>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, horms@...nel.org, andrew+netdev@...n.ch,
 netdev@...r.kernel.org, xiyou.wangcong@...il.com, jiri@...nulli.us,
 victor@...atatu.com, dcaratti@...hat.com, lariel@...dia.com,
 daniel@...earbox.net, pablo@...filter.org, kadlec@...filter.org,
 fw@...len.de, phil@....cc, netfilter-devel@...r.kernel.org,
 coreteam@...filter.org, zyc199902@...omail.cn, lrGerlinde@...lfence.com,
 jschung2@...ton.me
Subject: Re: [PATCH net 0/6] net/sched: Fix packet loops in mirred and netem

On Sun, 11 Jan 2026 11:39:41 -0500
Jamal Hadi Salim <jhs@...atatu.com> wrote:

> We introduce a 2-bit global skb->ttl counter.Patch #1 describes how we puti
> together those bits. Patches #2 and patch #5 use these bits.
> I added Fixes tags to patch #1 in case it is useful for backporting.
> Patch #3 and #4 revert William's earlier netem commits. Patch #6 introduces
> tdc test cases.
> 
> Jamal Hadi Salim (5):
>   net: Introduce skb ttl field to track packet loops
>   net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop
>   Revert "net/sched: Restrict conditions for adding duplicating netems
>     to qdisc tree"
>   Revert "selftests/tc-testing: Add tests for restrictions on netem
>     duplication"
>   net/sched: fix packet loop on netem when duplicate is on
> 
> Victor Nogueira (1):
>   selftests/tc-testing: Add netem/mirred test cases exercising loops
> 
>  drivers/net/ifb.c                             |   2 +-
>  include/linux/skbuff.h                        |  24 +-
>  include/net/sch_generic.h                     |  22 +
>  net/netfilter/nft_fwd_netdev.c                |   1 +
>  net/sched/act_mirred.c                        |  45 +-
>  net/sched/sch_netem.c                         |  47 +-
>  .../tc-testing/tc-tests/actions/mirred.json   | 616 +++++++++++++++++-
>  .../tc-testing/tc-tests/infra/qdiscs.json     |   5 +-
>  .../tc-testing/tc-tests/qdiscs/netem.json     |  96 +--
>  9 files changed, 698 insertions(+), 160 deletions(-)
>

Reviewed-by: Stephen Hemminger <stephen@...workplumber.org>

This is a complex patch series so I decided to get a second opinion using AI.
It is worth reading (but not completely trusting). Review prompt is Chris Mason's
Claude review prompts.


Summary: Patch Series Analysis
Patches Reviewed
6-patch series from Jamal Hadi Salim fixing loop detection in mirred and netem:

Patch 1/6: Introduces skb->ttl (2-bit field) for cross-deferral loop tracking, moves from_ingress to qdisc_skb_cb
Patch 2/6: Fixes mirred ingress→egress→ingress loop detection using ttl
Patch 3/6: Reverts netem duplication restrictions (preparation for proper fix)
Patch 4/6: Reverts associated selftests
Patch 5/6: Fixes netem duplicate infinite loop using ttl
Patch 6/6: (Email thread discussion, not code)

Key Findings
No regressions identified. The analysis covered:
AreaResultSKB structure change (ttl field)✓ Safe - properly initialized via zeroingfrom_ingress relocation✓ Safe - written immediately before readLoop detection logic✓ Correct - ttl tracks across async boundariesNetem duplicate fix✓ Improvement over old q->duplicate hackLocking✓ Correct softirq/per-cpu patternsResource management✓ No leaks identified
Design Assessment
The approach is sound:

Egress paths: Continue using per-cpu sched_mirred_dev[] array for immediate loop detection
Ingress paths: Use skb->ttl to track loops across netif_rx() deferral boundaries
Netem: ttl-based dup prevention works across entire qdisc tree (better than old local-only fix)

Recommendation
Yes, the patch is OK to merge.
The series correctly fixes real bugs (CVE-worthy loop conditions) with a minimal, well-designed solution. The 2-bit ttl field is sufficient for the use case (limit of 3 ingress redirects), and the changes maintain backward compatibility for existing configurations while closing the loop detection gaps.