On Thu, Oct 15, 2026 at 6:39 PM Florian Westphal <[email protected]> wrote: > > Zihan Xi <[email protected]> wrote: > > Hi Linux kernel maintainers, > > > > We found and validated an issue in netfilter packet duplication. The bug > > is reachable by an unprivileged user in user and network namespaces. > > Bounded tests exercised the fixes. Non-duplication packet semantics are > > intended to remain unchanged. MIRRED flow offload for affected netdev dup > > rules is rejected because it cannot enforce the duplication-depth limit. > > > > We will provide detailed information about the bug > > in this email, along with a PoC to trigger it. > > Series makes no sense to me, sorry. I dislike the 2nd patch, I see no > reason why we need to make changes all over the place for this...? > > Also, if we go with 2nd patch, why do we still need the first? > > > The IPv4 and IPv6 duplication helpers use a task-local guard while a cloned > > packet traverses output hooks. If NFQUEUE suspends that traversal, the > > output call can return and clear the guard before a later verdict resumes > > the remaining hooks. The resumed packet can reach another duplication > > consumer without the original guard. This can cause repeated packet > > processing and consume CPU and network resources. > > "Don't do that, then"? > > I think the kernel should not crash when misused, but I don't think > kernel should have to do extreme stunts to detect such loops. > > There are probably many other ways to make kernel allocate lots of > memory or waste cpu cycles. > > E.g. via create use of veth and bridges, sched plug, teql, ... > > > Patch 1 records the > > guard in the queue entry and restores it during reinjection. It keeps > > bottom halves disabled during the reinjection traversal, restoring the > > verdict task state before reenabling them. > > Its small enough, so I have no objections to this patch. > > > The netdev nft dup path has a separate synchronous transmit recursion > > counter. It does not accompany a duplicated skb through asynchronous > > software receive paths. Patch 2 carries a bounded duplication depth through > > skb and supported XDP conversions, resets it on reused buffers, and rejects > > MIRRED flow offload where the software depth cannot be enforced. This scope > > covers software skb/XDP continuity, not packet loops through physical > > hardware or userspace device round trips. > > I think it would be better to first go to the drawing board and make ONE > facility that governs the recursion part. > > From a quick glance, the difference is historical from time when > per-module pcpu counters were used, so nf_dup netdev could not see the state > of nft_dup toggle. Now that its in task struct anyway, maybe its time to > just zap the egress recursion counter and first make a patch that > uses the bool toggle for both egress and normal dup? > > I would like to remove code, not add more. > > > At PRE_ROUTING and LOCAL_IN, the IPv4 and IPv6 helpers decrement TTL or > > hop limit on the copy. Patch 3 stops creating copies when the value is > > <= 1, so the decrement cannot reach zero or wrap a zero value to the > > maximum. > > Thats unrelated problem. There are other places that do TTL/HL > decrements. I think it makes sense to clean this up in -next and > add common helpers and then use those.
Hi Florian, Thanks for taking the time to review this series. Your comments make clear that I have not yet presented a sufficiently simple, coherent design, particularly why both patches 1 and 2 are needed. I will go back to the recursion-guard design and first evaluate whether the existing current->in_nf_duplicate flag can be used by both the IPv4/IPv6 and netdev duplication paths, replacing the separate egress recursion counter. I will check the effects on the counter’s other users and verify the relevant egress, bottom-half, and asynchronous receive paths before deciding whether a task-local flag is sufficient. I will not assume that it covers asynchronous paths without evidence. I understand that you have no objection to patch 1 on its own. I will nevertheless check whether the unified design makes its NFQUEUE queue-entry state redundant; if not, I will keep that change limited to the NFQUEUE state-lifetimeissue. I agree that the TTL/hop-limit change is separate from this series. I will leave it out and consider it independently alongside the common-helper work in -next. I also take your point about scope: the goal should be a simple, coherent recursion guard for this duplication behavior, not a general mechanism for detecting every possible source of CPU or memory consumption. I will return with the path analysis and a smaller design before sending another series. Best regards, Zihan
