mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [BUG] net/sched: DualPI2 memory accounting underflows after TBF peek
@ 2026-10-22 14:47 shaos
  2026-11-09 16:36 ` [PATCH net] net/sched: sch_dualpi2: avoid double memory accounting on peek shaos
  0 siblings, 1 reply; 3+ messages in thread
From: shaos @ 2026-10-22 14:47 UTC (permalink / raw)
  To: jhs
  Cc: Jiri Pirko, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, netdev, linux-kernel

Hello,

A TBF parent peeks a DualPI2 child packet, causing DualPI2 to subtract its truesize while parking it in gso_skb. Reducing the DualPI2 memory limit drains the parked skb and subtracts its truesize again, wrapping the u32 memory_used counter.

HEAD commit:    c03c1a14507dfaf8b856c2608f9ac216021fc222
git tree:       30fd2ef82912c26cf3a58ad0ba6afc375fa851ac
source version: Linux 7.3.0, local master
compiler:       gcc 11.4.0, GNU ld 2.38

console output:
https://gist.githubusercontent.com/shaos26/6f3e3d3667401ad3325959e321928400/raw/c50aae5c021dee0b3ab97e76d260da585d1fde98/console-output.txt
kernel config:
https://gist.githubusercontent.com/shaos26/6f3e3d3667401ad3325959e321928400/raw/6b13332e308ed8ce37b28ea2d7c8fb50265a8b88/kernel-config.txt
reproducer:
https://gist.githubusercontent.com/shaos26/6f3e3d3667401ad3325959e321928400/raw/937e24e9b0dcfcd847a80e74ad2707f6b4781030/poc.c
auxiliary reproducer (init):
https://gist.githubusercontent.com/shaos26/6f3e3d3667401ad3325959e321928400/raw/17fb0deae08ba739b9e044cfd6c93be24eff2e6e/init

Build the tested kernel with the attached configuration and CONFIG_NET_SCH_DUALPI2 and CONFIG_NET_SCH_TBF enabled. Run poc.c as a static /init in the attached minimal initramfs inside disposable QEMU; the test creates a user/network namespace and reports the counter through rtnetlink.

Impact and prerequisites:
After the rejected/in-progress accounting state wraps, packets continue to fail with ENOBUFS even after restoring the original memory limit; replacing the qdisc is required. The test is reachable by a caller with CAP_NET_ADMIN in a user-created network namespace and is a namespace-local availability issue.

Validation: Two exact-revision fresh boots reproduced the same counter wrap and persistent enqueue failures; no-drain and close accounting controls passed.

Thanks,

shaos


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH net] net/sched: sch_dualpi2: avoid double memory accounting on peek
  2026-10-22 14:47 [BUG] net/sched: DualPI2 memory accounting underflows after TBF peek shaos
@ 2026-11-09 16:36 ` shaos
  2026-11-13 18:00   ` Simon Horman
  0 siblings, 1 reply; 3+ messages in thread
From: shaos @ 2026-11-09 16:36 UTC (permalink / raw)
  To: jhs
  Cc: Jiri Pirko, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, netdev, linux-kernel

qdisc_dequeue_internal() removes skbs from gso_skb without invoking
the qdisc dequeue callback. Their truesize was already subtracted
when they were first dequeued for peek or requeued after a transmit
failure. dualpi2_change() currently subtracts it again while trimming
the queue, wrapping the u32 memory_used counter.

Skip the second decrement when the skb comes from gso_skb. Packets
dequeued directly from the C-queue still need the accounting here;
the L-queue path is unchanged.

The issue was identified by LLM-assisted source analysis and
reproduced with a purpose-built rtnetlink/AF_PACKET test in QEMU.
The test uses CAP_NET_ADMIN in a user-created network namespace and
CAP_NET_RAW for its AF_PACKET sender. It does not modify the kernel
or use error injection.

On Linux 7.4-rc2, the unpatched reproducer wrapped memory_used to
4294964992 and subsequent packets remained rejected after restoring
the memory limit. With this patch, the counter remains valid and a
packet is accepted after restoring the limit.

The modified sch_dualpi2.o builds with W=1 under allmodconfig and
allyesconfig. Full-tree W=1 builds stop on unrelated warnings in
lib/tests/stackinit_kunit.c, arch/x86/kernel/callthunks.c, and
net/8021q/vlan.c. The QEMU regression passes; tc-testing was not run.

Fixes: 8f9516daedd6 ("sched: Add enqueue/dequeue of dualpi2 qdisc")
Link: https://lkml.iu.edu/hypermail/linux/kernel/2610.2/16461.html
Signed-off-by: shaos <shawnq1@163.com>
---
 net/sched/sch_dualpi2.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c
index 4947def7c49ef..ccf6359eeebc3 100644
--- a/net/sched/sch_dualpi2.c
+++ b/net/sched/sch_dualpi2.c
@@ -909,14 +909,20 @@ static int dualpi2_change(struct Qdisc *sch, struct nlattr *opt,
 	while (qdisc_qlen(sch) > sch->limit ||
 	       q->memory_used > q->memory_limit) {
 		struct sk_buff *skb = NULL;
+		bool already_dequeued;
 
 		if (qdisc_qlen(sch) > qdisc_qlen(q->l_queue)) {
+			already_dequeued = skb_peek(&sch->gso_skb);
+
 			skb = qdisc_dequeue_internal(sch, true);
 			if (unlikely(!skb)) {
 				WARN_ON_ONCE(1);
 				break;
 			}
-			WRITE_ONCE(q->memory_used, q->memory_used - skb->truesize);
+			/* A peeked or requeued skb was already accounted. */
+			if (!already_dequeued)
+				WRITE_ONCE(q->memory_used,
+					   q->memory_used - skb->truesize);
 			rtnl_qdisc_drop(skb, sch);
 		} else if (qdisc_qlen(q->l_queue)) {
 			skb = qdisc_dequeue_internal(q->l_queue, true);


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net/sched: sch_dualpi2: avoid double memory accounting on peek
  2026-11-09 16:36 ` [PATCH net] net/sched: sch_dualpi2: avoid double memory accounting on peek shaos
@ 2026-11-13 18:00   ` Simon Horman
  0 siblings, 0 replies; 3+ messages in thread
From: Simon Horman @ 2026-11-13 18:00 UTC (permalink / raw)
  To: shaos
  Cc: jhs, Jiri Pirko, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel

On Mon, Nov 09, 2026 at 04:36:55PM +0000, shaos wrote:
> qdisc_dequeue_internal() removes skbs from gso_skb without invoking
> the qdisc dequeue callback. Their truesize was already subtracted
> when they were first dequeued for peek or requeued after a transmit
> failure. dualpi2_change() currently subtracts it again while trimming
> the queue, wrapping the u32 memory_used counter.
> 
> Skip the second decrement when the skb comes from gso_skb. Packets
> dequeued directly from the C-queue still need the accounting here;
> the L-queue path is unchanged.
> 
> The issue was identified by LLM-assisted source analysis and
> reproduced with a purpose-built rtnetlink/AF_PACKET test in QEMU.
> The test uses CAP_NET_ADMIN in a user-created network namespace and
> CAP_NET_RAW for its AF_PACKET sender. It does not modify the kernel
> or use error injection.
> 
> On Linux 7.4-rc2, the unpatched reproducer wrapped memory_used to
> 4294964992 and subsequent packets remained rejected after restoring
> the memory limit. With this patch, the counter remains valid and a
> packet is accepted after restoring the limit.
> 
> The modified sch_dualpi2.o builds with W=1 under allmodconfig and
> allyesconfig. Full-tree W=1 builds stop on unrelated warnings in
> lib/tests/stackinit_kunit.c, arch/x86/kernel/callthunks.c, and
> net/8021q/vlan.c. The QEMU regression passes; tc-testing was not run.
> 
> Fixes: 8f9516daedd6 ("sched: Add enqueue/dequeue of dualpi2 qdisc")
> Link: https://lkml.iu.edu/hypermail/linux/kernel/2610.2/16461.html
> Signed-off-by: shaos <shawnq1@163.com>
> ---
>  net/sched/sch_dualpi2.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c
> index 4947def7c49ef..ccf6359eeebc3 100644
> --- a/net/sched/sch_dualpi2.c
> +++ b/net/sched/sch_dualpi2.c
> @@ -909,14 +909,20 @@ static int dualpi2_change(struct Qdisc *sch, struct nlattr *opt,
>  	while (qdisc_qlen(sch) > sch->limit ||
>  	       q->memory_used > q->memory_limit) {
>  		struct sk_buff *skb = NULL;
> +		bool already_dequeued;
>  
>  		if (qdisc_qlen(sch) > qdisc_qlen(q->l_queue)) {
> +			already_dequeued = skb_peek(&sch->gso_skb);
> +
>  			skb = qdisc_dequeue_internal(sch, true);
>  			if (unlikely(!skb)) {
>  				WARN_ON_ONCE(1);
>  				break;
>  			}
> -			WRITE_ONCE(q->memory_used, q->memory_used - skb->truesize);
> +			/* A peeked or requeued skb was already accounted. */
> +			if (!already_dequeued)
> +				WRITE_ONCE(q->memory_used,
> +					   q->memory_used - skb->truesize);

It strikes me that this change is incomplete.

I think the loads and stores of q->memory_used and q->memory_limit
should be consistently annotated, stashing the value of reads
in local variables to avoid re-reads as appropriate.

Perhaps the resulting change is too wide to be a strict fix for the bug
at hand. But that does at least seem necessary within this function.

>  			rtnl_qdisc_drop(skb, sch);
>  		} else if (qdisc_qlen(q->l_queue)) {
>  			skb = qdisc_dequeue_internal(q->l_queue, true);
> 

-- 
pw-bot: changes-requested

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-13 18:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-22 14:47 [BUG] net/sched: DualPI2 memory accounting underflows after TBF peek shaos
2026-11-09 16:36 ` [PATCH net] net/sched: sch_dualpi2: avoid double memory accounting on peek shaos
2026-11-13 18:00   ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®