The CoDel control law schedules the next drop one interval/sqrt(count) after the previous drop, using the configured interval (codel_params.interval). For very small intervals the scheduled step rounds down to zero, so the dropping loop in codel_dequeue() never advances and drains the entire backlog under the qdisc lock in one call - an unprivileged user can trigger a soft lockup this way. Fix in the shared codel code used by both codel and fq_codel: 1. Make the control-law step at least 1 tick so the dropping loop always moves forward. 2. Cap the dropping loop at CODEL_MAX_DROPS_PER_DEQUEUE (256) drops per codel_dequeue() call, resyncing drop_next to now when the cap is hit: the catch-up owed to the loop grows with the idle gap and the backlog, which no interval threshold can bound. This is a deliberate behaviour change after long idle gaps. The cap applies to fq_codel (4b549a2ef4be) and the mac80211 TXQ path (fixed interval, cap only). The target sojourn delay (codel_params.target) is not validated: it does not feed the control law, so a sub-tick value is aggressive rather than deadlock-prone. Conditions to recreate the bug (repro from vega@nebusec.ai): - tc qdisc add dev lo root handle 1: tbf rate 1kbit burst 2kb limit 1000000 - tc qdisc add dev lo parent 1:1 handle 10: codel interval 2us target 1ms noecn limit 1000000 (same for fq_codel) - unpatched kernel: tc accepts it; a UDP flood under the 1kbit tbf soft-lockups (watchdog: BUG: soft lockup) while one codel_dequeue() call drops the backlog under the qdisc lock - patched kernel: same setup, at most 256 drops per dequeue call, no soft lockup Fixes: 76e3cc126bb2 ("codel: Controlled Delay AQM") Reported-by: Vega Tested-by: Victor Nogueira Signed-off-by: Jamal Hadi Salim --- include/net/codel.h | 5 +++++ include/net/codel_impl.h | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/include/net/codel.h b/include/net/codel.h index aa80f744826c..183d43c2bd43 100644 --- a/include/net/codel.h +++ b/include/net/codel.h @@ -140,6 +140,11 @@ struct codel_vars { /* needed shift to get a Q0.32 number from rec_inv_sqrt */ #define REC_INV_SQRT_SHIFT (32 - REC_INV_SQRT_BITS) +/* Cap on drops per codel_dequeue() call: the loop's work depends on the + * idle gap and backlog, both outside our control; resync when exceeded. + */ +#define CODEL_MAX_DROPS_PER_DEQUEUE 256 + /** * struct codel_stats - contains codel shared variables and stats * @maxpacket: largest packet we've seen so far diff --git a/include/net/codel_impl.h b/include/net/codel_impl.h index 2c1f0ec309e9..9e779cd4d6de 100644 --- a/include/net/codel_impl.h +++ b/include/net/codel_impl.h @@ -93,12 +93,17 @@ static void codel_Newton_step(struct codel_vars *vars) * CoDel control_law is t + interval/sqrt(count) * We maintain in rec_inv_sqrt the reciprocal value of sqrt(count) to avoid * both sqrt() and divide operation. + * + * Clamp the increment to at least 1 tick: a very small interval (or a + * large count) can truncate it to zero, stalling the dropping loop. */ static codel_time_t codel_control_law(codel_time_t t, codel_time_t interval, u32 rec_inv_sqrt) { - return t + reciprocal_scale(interval, rec_inv_sqrt << REC_INV_SQRT_SHIFT); + return t + max_t(u32, 1, + reciprocal_scale(interval, + rec_inv_sqrt << REC_INV_SQRT_SHIFT)); } static bool codel_should_drop(const struct sk_buff *skb, @@ -156,6 +161,7 @@ static struct sk_buff *codel_dequeue(void *ctx, struct sk_buff *skb = dequeue_func(vars, ctx); codel_time_t now; bool drop; + unsigned int drops = 0; if (!skb) { vars->first_above_time = 0; @@ -180,6 +186,14 @@ static struct sk_buff *codel_dequeue(void *ctx, */ while (vars->dropping && codel_time_after_eq(now, vars->drop_next)) { + if (++drops > CODEL_MAX_DROPS_PER_DEQUEUE) { + /* fell far behind the schedule */ + WRITE_ONCE(vars->drop_next, + codel_control_law(now, + params->interval, + vars->rec_inv_sqrt)); + break; + } /* dont care of possible wrap * since there is no more divide. */ -- 2.43.0 Add tdc cases locking the codel/fq_codel small-interval uAPI after the dropping-loop bound (previous patch): sub-tick and two-tick intervals are ACCEPTED (the loop bound makes them safe), the 1024us boundary is accepted, and a sub-tick target sojourn delay is accepted (it does not participate in the control law): codel: 6e44/a8c3/a695/9793 - interval 1us/3us/1024us and target 1us accepted (rendered 0us/2us/1.02ms/0us by tc) fq_codel: 1b4d/3540/49c5/3e0f - interval 1us/3us/1024us and target 1us accepted The positive cases match the full rendered qdisc line (tc renders interval 1us as 0us, 3us as 2us, 1024us as 1.02ms), mirroring the existing tests in these files. These cases do not test the dropping-loop bound itself: tdc cannot observe per-dequeue drop counts. c797 (fq_codel target 1 interval 1) passes unmodified on the patched kernel, which is the uAPI evidence for the previous patch. Signed-off-by: Jamal Hadi Salim --- .../tc-testing/tc-tests/qdiscs/codel.json | 72 +++++++++++++++++++ .../tc-testing/tc-tests/qdiscs/fq_codel.json | 72 +++++++++++++++++++ 2 files changed, 144 insertions(+) diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/codel.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/codel.json index 6d515d0e5ed6..a894e6f0e267 100644 --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/codel.json +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/codel.json @@ -213,5 +213,77 @@ "matchPattern": "qdisc codel 1: root refcnt [0-9]+ limit 1p target 5ms interval 100ms", "matchCount": "1", "teardown": ["$TC qdisc del dev $DEV1 handle 1: root"] + }, + { + "id": "6e44", + "name": "Create CODEL with 1us interval, accepted (sub-tick, uAPI locked)", + "category": [ + "qdisc", + "codel" + ], + "plugins": { + "requires": "nsPlugin" + }, + "setup": [], + "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root codel interval 1us", + "expExitCode": "0", + "verifyCmd": "$TC qdisc show dev $DUMMY", + "matchPattern": "qdisc codel 1: root refcnt [0-9]+ limit 1000p target 5ms interval 0us", + "matchCount": "1", + "teardown": ["$TC qdisc del dev $DUMMY handle 1: root"] + }, + { + "id": "a8c3", + "name": "Create CODEL with 3us interval, accepted (two ticks)", + "category": [ + "qdisc", + "codel" + ], + "plugins": { + "requires": "nsPlugin" + }, + "setup": [], + "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root codel interval 3us", + "expExitCode": "0", + "verifyCmd": "$TC qdisc show dev $DUMMY", + "matchPattern": "qdisc codel 1: root refcnt [0-9]+ limit 1000p target 5ms interval 2us", + "matchCount": "1", + "teardown": ["$TC qdisc del dev $DUMMY handle 1: root"] + }, + { + "id": "a695", + "name": "Create CODEL with 1024us interval boundary accepted", + "category": [ + "qdisc", + "codel" + ], + "plugins": { + "requires": "nsPlugin" + }, + "setup": [], + "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root codel interval 1024us", + "expExitCode": "0", + "verifyCmd": "$TC qdisc show dev $DUMMY", + "matchPattern": "qdisc codel 1: root refcnt [0-9]+ limit 1000p target 5ms interval 1.02ms", + "matchCount": "1", + "teardown": ["$TC qdisc del dev $DUMMY handle 1: root"] + }, + { + "id": "9793", + "name": "Create CODEL with 1us target, accepted (target not in control law)", + "category": [ + "qdisc", + "codel" + ], + "plugins": { + "requires": "nsPlugin" + }, + "setup": [], + "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root codel target 1us", + "expExitCode": "0", + "verifyCmd": "$TC qdisc show dev $DUMMY", + "matchPattern": "qdisc codel 1: root refcnt [0-9]+ limit 1000p target 0us interval 100ms", + "matchCount": "1", + "teardown": ["$TC qdisc del dev $DUMMY handle 1: root"] } ] diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq_codel.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq_codel.json index 4ce62b857fd7..de6a1b8d954a 100644 --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq_codel.json +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq_codel.json @@ -316,5 +316,77 @@ "matchPattern": "qdisc fq_codel 1: root refcnt [0-9]+ limit 1p flows 1024 quantum.*target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64", "matchCount": "1", "teardown": ["$TC qdisc del dev $DEV1 handle 1: root"] + }, + { + "id": "1b4d", + "name": "Create FQ_CODEL with 1us interval, accepted (sub-tick, uAPI locked)", + "category": [ + "qdisc", + "fq_codel" + ], + "plugins": { + "requires": "nsPlugin" + }, + "setup": [], + "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root fq_codel interval 1us", + "expExitCode": "0", + "verifyCmd": "$TC qdisc show dev $DUMMY", + "matchPattern": "qdisc fq_codel 1: root refcnt [0-9]+ limit 10240p flows 1024 quantum [0-9]+ target 5ms interval 0us memory_limit 32Mb ecn drop_batch 64", + "matchCount": "1", + "teardown": ["$TC qdisc del dev $DUMMY handle 1: root"] + }, + { + "id": "3540", + "name": "Create FQ_CODEL with 3us interval, accepted (two ticks)", + "category": [ + "qdisc", + "fq_codel" + ], + "plugins": { + "requires": "nsPlugin" + }, + "setup": [], + "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root fq_codel interval 3us", + "expExitCode": "0", + "verifyCmd": "$TC qdisc show dev $DUMMY", + "matchPattern": "qdisc fq_codel 1: root refcnt [0-9]+ limit 10240p flows 1024 quantum [0-9]+ target 5ms interval 2us memory_limit 32Mb ecn drop_batch 64", + "matchCount": "1", + "teardown": ["$TC qdisc del dev $DUMMY handle 1: root"] + }, + { + "id": "49c5", + "name": "Create FQ_CODEL with 1024us interval boundary accepted", + "category": [ + "qdisc", + "fq_codel" + ], + "plugins": { + "requires": "nsPlugin" + }, + "setup": [], + "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root fq_codel interval 1024us", + "expExitCode": "0", + "verifyCmd": "$TC qdisc show dev $DUMMY", + "matchPattern": "qdisc fq_codel 1: root refcnt [0-9]+ limit 10240p flows 1024 quantum [0-9]+ target 5ms interval 1.02ms memory_limit 32Mb ecn drop_batch 64", + "matchCount": "1", + "teardown": ["$TC qdisc del dev $DUMMY handle 1: root"] + }, + { + "id": "3e0f", + "name": "Create FQ_CODEL with 1us target, accepted (target not in control law)", + "category": [ + "qdisc", + "fq_codel" + ], + "plugins": { + "requires": "nsPlugin" + }, + "setup": [], + "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root fq_codel target 1us", + "expExitCode": "0", + "verifyCmd": "$TC qdisc show dev $DUMMY", + "matchPattern": "qdisc fq_codel 1: root refcnt [0-9]+ limit 10240p flows 1024 quantum [0-9]+ target 0us interval 100ms memory_limit 32Mb ecn drop_batch 64", + "matchCount": "1", + "teardown": ["$TC qdisc del dev $DUMMY handle 1: root"] } ] -- 2.43.0