[PATCH net] net: sched: fix quantum/backlog overflow in fq, fq_codel, hhf, sfq
Jamal Hadi Salim <[email protected]>
| Newsgroups | gmane.linux.kernel.stable,gmane.linux.network |
|---|---|
| Message-ID | <[email protected]> |
Several qdiscs derive their per-flow quantum or backlog from psched_mtu() or accumulate qdisc_pkt_len() into a u32/int counter without an overflow or zero clamp, which can drive the dequeue/credit-refill loop into a soft lockup or a NULL deref. [email protected] provided reports and PoCs which illustrated the following: - sch_fq: fq_dequeue() credit-refill loop with a small quantum spins ~1B iterations under the qdisc lock (soft lockup); fq_init() computes quantum = 2 * psched_mtu() with no overflow check. - sch_fq_codel: fq_codel_enqueue() accumulates qdisc_pkt_len() into a u32 per-flow backlog; a crafted TCA_STAB inflates pkt_len to 1 GiB so a few packets wrap the counter to 0, and fq_codel_drop() then picks an empty flow and derefs NULL. - sch_hhf: hhf_init() sets quantum = psched_mtu() with no overflow check; a huge MTU makes it 0x80000000, and hhf_dequeue()'s deficit += weight * quantum loops forever. - sch_sfq: sfq_init() sets quantum = psched_mtu() (unsigned); a huge MTU makes allot = INT_MIN, and INT_MIN + INT_MIN is UB that toggles between INT_MIN and 0 forever. Clamp the quantum to a sane minimum and promote the backlog/credit sums to avoid the wrap, so the dequeue loops terminate and the drop path never selects an empty flow. Reported-by: [email protected] Tested-by: Victor Nogueira <[email protected]> Signed-off-by: Jamal Hadi Salim <[email protected]> --- net/sched/sch_fq.c | 9 ++++++--- net/sched/sch_fq_codel.c | 14 +++++++++++++- net/sched/sch_hhf.c | 12 +++++++++++ net/sched/sch_sfq.c | 7 +++++++ 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c index 7cae082a9847..66e9c6e530d8 100644 --- a/net/sched/sch_fq.c +++ b/net/sched/sch_fq.c @@ -750,7 +750,10 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch) f = head->first; retry = 0; if (f->credit <= 0) { - f->credit += q->quantum; + if (f->credit + (int)q->quantum > 0) + f->credit += q->quantum; + else + f->credit = q->quantum; head->first = f->next; fq_flow_add_tail(q, f, OLD_FLOW); goto begin; @@ -1226,8 +1229,8 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt, sch->limit = 10000; q->flow_plimit = 100; - q->quantum = 2 * psched_mtu(qdisc_dev(sch)); - q->initial_quantum = 10 * psched_mtu(qdisc_dev(sch)); + q->quantum = max_t(u32, 2 * psched_mtu(qdisc_dev(sch)), 256); + q->initial_quantum = max_t(u32, 10 * psched_mtu(qdisc_dev(sch)), 256); q->flow_refill_delay = msecs_to_jiffies(40); q->flow_max_rate = ~0UL; q->time_next_delayed_flow = ~0ULL; diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c index 6cce86ba383c..990f8b1cc57e 100644 --- a/net/sched/sch_fq_codel.c +++ b/net/sched/sch_fq_codel.c @@ -152,12 +152,24 @@ static unsigned int fq_codel_drop(struct Qdisc *sch, unsigned int max_packets, * amortizing this linear lookup to one cache line per drop. */ for (i = 0; i < q->flows_cnt; i++) { - if (q->backlogs[i] > maxbacklog) { + if (q->backlogs[i] > maxbacklog && q->flows[i].head) { maxbacklog = q->backlogs[i]; idx = i; } } + /* TCA_STAB can inflate qdisc_pkt_len enough to wrap per-flow + * backlogs (u32) to zero; fall back to a flow with packets. + */ + if (maxbacklog == 0) { + for (i = 0; i < q->flows_cnt; i++) { + if (q->flows[i].head) { + idx = i; + break; + } + } + } + /* Our goal is to drop half of this fat flow backlog */ threshold = maxbacklog >> 1; diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c index d85cb0263b67..7634d6cbeed8 100644 --- a/net/sched/sch_hhf.c +++ b/net/sched/sch_hhf.c @@ -624,6 +624,18 @@ static int hhf_init(struct Qdisc *sch, struct nlattr *opt, q->hhf_evict_timeout = HZ; /* 1 sec */ q->hhf_non_hh_weight = 2; + if (!opt) { + u64 non_hh_quantum = (u64)q->quantum * q->hhf_non_hh_weight; + + /* A device with max_mtu == 0 (e.g. dummy) accepts an MTU that + * makes weight * quantum overflow the signed deficit and spin + * hhf_dequeue() forever. Clamp to the same minimum quantum + * floor fq_codel uses (max(256U, ...)). + */ + if (non_hh_quantum == 0 || non_hh_quantum > INT_MAX) + q->quantum = 256; + } + if (opt) { int err = hhf_change(sch, opt, extack); diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c index 77675f9a4c46..b84aa7614cc9 100644 --- a/net/sched/sch_sfq.c +++ b/net/sched/sch_sfq.c @@ -800,6 +800,13 @@ static int sfq_init(struct Qdisc *sch, struct nlattr *opt, q->divisor = SFQ_DEFAULT_HASH_DIVISOR; q->maxflows = SFQ_DEFAULT_FLOWS; q->quantum = psched_mtu(qdisc_dev(sch)); + /* A device with max_mtu == 0 (e.g. dummy) accepts an MTU that makes + * psched_mtu() wrap into the sign bit; that would set slot->allot + * negative and spin sfq_dequeue() forever. Fall back to a sane + * quantum (10 KiB) that fits a signed int and is >= one MTU. + */ + if ((int)q->quantum <= 0) + q->quantum = 10 * 1024; q->perturb_period = 0; get_random_bytes(&q->perturbation, sizeof(q->perturbation)); -- 2.34.1