| Message ID | 20250403211033.166059-1-xiyou.wangcong@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | net_sched: make ->qlen_notify() idempotent | expand |
On Thu, Apr 3, 2025 at 5:10 PM Cong Wang <xiyou.wangcong@gmail.com> wrote: > > Gerrard reported a vulnerability exists in fq_codel where manipulating > the MTU can cause codel_dequeue() to drop all packets. The parent qdisc's > sch->q.qlen is only updated via ->qlen_notify() if the fq_codel queue > remains non-empty after the drops. This discrepancy in qlen between > fq_codel and its parent can lead to a use-after-free condition. > > Let's fix this by making all existing ->qlen_notify() idempotent so that > the sch->q.qlen check will be no longer necessary. > > Patch 1~5 make all existing ->qlen_notify() idempotent to prepare for > patch 6 which removes the sch->q.qlen check. They are followed by 5 > selftests for each type of Qdisc's we touch here. > > All existing and new Qdisc selftests pass after this patchset. > > Fixes: 4b549a2ef4be ("fq_codel: Fair Queue Codel AQM") > Fixes: 76e3cc126bb2 ("codel: Controlled Delay AQM") > For the patches: Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> cheers, jamal
On 4/3/25 11:10 PM, Cong Wang wrote: > Gerrard reported a vulnerability exists in fq_codel where manipulating > the MTU can cause codel_dequeue() to drop all packets. The parent qdisc's > sch->q.qlen is only updated via ->qlen_notify() if the fq_codel queue > remains non-empty after the drops. This discrepancy in qlen between > fq_codel and its parent can lead to a use-after-free condition. > > Let's fix this by making all existing ->qlen_notify() idempotent so that > the sch->q.qlen check will be no longer necessary. > > Patch 1~5 make all existing ->qlen_notify() idempotent to prepare for > patch 6 which removes the sch->q.qlen check. They are followed by 5 > selftests for each type of Qdisc's we touch here. > > All existing and new Qdisc selftests pass after this patchset. > > Fixes: 4b549a2ef4be ("fq_codel: Fair Queue Codel AQM") > Fixes: 76e3cc126bb2 ("codel: Controlled Delay AQM") FTR, I think it would be better to include the fixes tag in the relevant commit message, as such I propagated the above tags in patch 6. /P
Hello: This series was applied to netdev/net.git (main) by Paolo Abeni <pabeni@redhat.com>: On Thu, 3 Apr 2025 14:10:22 -0700 you wrote: > Gerrard reported a vulnerability exists in fq_codel where manipulating > the MTU can cause codel_dequeue() to drop all packets. The parent qdisc's > sch->q.qlen is only updated via ->qlen_notify() if the fq_codel queue > remains non-empty after the drops. This discrepancy in qlen between > fq_codel and its parent can lead to a use-after-free condition. > > Let's fix this by making all existing ->qlen_notify() idempotent so that > the sch->q.qlen check will be no longer necessary. > > [...] Here is the summary with links: - [net,v2,01/11] sch_htb: make htb_qlen_notify() idempotent https://git.kernel.org/netdev/net/c/5ba8b837b522 - [net,v2,02/11] sch_drr: make drr_qlen_notify() idempotent https://git.kernel.org/netdev/net/c/df008598b3a0 - [net,v2,03/11] sch_hfsc: make hfsc_qlen_notify() idempotent https://git.kernel.org/netdev/net/c/51eb3b65544c - [net,v2,04/11] sch_qfq: make qfq_qlen_notify() idempotent https://git.kernel.org/netdev/net/c/55f9eca4bfe3 - [net,v2,05/11] sch_ets: make est_qlen_notify() idempotent https://git.kernel.org/netdev/net/c/a7a15f39c682 - [net,v2,06/11] codel: remove sch->q.qlen check before qdisc_tree_reduce_backlog() https://git.kernel.org/netdev/net/c/342debc12183 - [net,v2,07/11] selftests/tc-testing: Add a test case for FQ_CODEL with HTB parent https://git.kernel.org/netdev/net/c/cbe9588b12d0 - [net,v2,08/11] selftests/tc-testing: Add a test case for FQ_CODEL with QFQ parent https://git.kernel.org/netdev/net/c/4cb1837ac537 - [net,v2,09/11] selftests/tc-testing: Add a test case for FQ_CODEL with HFSC parent https://git.kernel.org/netdev/net/c/72b05c1bf7ea - [net,v2,10/11] selftests/tc-testing: Add a test case for FQ_CODEL with DRR parent https://git.kernel.org/netdev/net/c/0d5c27ecb60c - [net,v2,11/11] selftests/tc-testing: Add a test case for FQ_CODEL with ETS parent https://git.kernel.org/netdev/net/c/ce94507f5fe0 You are awesome, thank you!
Gerrard reported a vulnerability exists in fq_codel where manipulating the MTU can cause codel_dequeue() to drop all packets. The parent qdisc's sch->q.qlen is only updated via ->qlen_notify() if the fq_codel queue remains non-empty after the drops. This discrepancy in qlen between fq_codel and its parent can lead to a use-after-free condition. Let's fix this by making all existing ->qlen_notify() idempotent so that the sch->q.qlen check will be no longer necessary. Patch 1~5 make all existing ->qlen_notify() idempotent to prepare for patch 6 which removes the sch->q.qlen check. They are followed by 5 selftests for each type of Qdisc's we touch here. All existing and new Qdisc selftests pass after this patchset. Fixes: 4b549a2ef4be ("fq_codel: Fair Queue Codel AQM") Fixes: 76e3cc126bb2 ("codel: Controlled Delay AQM") --- v2: drop the unstable CODEL selftest Cong Wang (11): sch_htb: make htb_qlen_notify() idempotent sch_drr: make drr_qlen_notify() idempotent sch_hfsc: make hfsc_qlen_notify() idempotent sch_qfq: make qfq_qlen_notify() idempotent sch_ets: make est_qlen_notify() idempotent codel: remove sch->q.qlen check before qdisc_tree_reduce_backlog() selftests/tc-testing: Add a test case for FQ_CODEL with HTB parent selftests/tc-testing: Add a test case for FQ_CODEL with QFQ parent selftests/tc-testing: Add a test case for FQ_CODEL with HFSC parent selftests/tc-testing: Add a test case for FQ_CODEL with DRR parent selftests/tc-testing: Add a test case for FQ_CODEL with ETS parent net/sched/sch_codel.c | 5 +- net/sched/sch_drr.c | 7 +- net/sched/sch_ets.c | 8 +- net/sched/sch_fq_codel.c | 6 +- net/sched/sch_hfsc.c | 8 +- net/sched/sch_htb.c | 2 + net/sched/sch_qfq.c | 7 +- .../tc-testing/tc-tests/infra/qdiscs.json | 155 ++++++++++++++++++ 8 files changed, 179 insertions(+), 19 deletions(-)