[PATCH net 1/2] net/sched: bound TCA_STAB overhead to prevent qdisc soft lockup

Jamal Hadi Salim <[email protected]>
Newsgroups gmane.linux.network,gmane.linux.kernel.stable
Message-ID <[email protected]>
qdisc_get_stab() copies the TCA_STAB_BASE overhead from userspace
without bounding it. A huge overhead (e.g. 2147483000) combined with
a small DRR class quantum (1) makes qdisc_calculate_pkt_len() set a
~2 GiB accounting length, so drr_dequeue() spins ~2 billion times
adding quantum=1 to the deficit under the qdisc spinlock with BH
disabled, producing a soft lockup / RCU stall.

The same unbounded stab overhead also lets qdisc_pkt_len() overflow
the per-flow counters of other qdiscs (fq_codel, hhf, sfq), which
were recently clamped per-qdisc; bounding the overhead at the source
closes the root cause for all of them.

Reject overhead > 65535 (matching QFQ_MAX_LMAX and the max single
skb length) in qdisc_get_stab(); legitimate L1 overheads are a few
dozen bytes at most.

[email protected] provided PoCs for both DRR and ETS demonstrating the
soft lockup.

Conditions to recreate the bug:
- require CONFIG_SOFTLOCKUP_DETECTOR=y
- Create a veth pair (or use lo) and attach a root qdisc with a
  TCA_STAB overhead near INT_MAX, e.g.:
    tc qdisc add dev veth0 root handle 1: \
        stab mtu 2048 tsize 0 overhead 2147483000 drr
  (or ... ets bands 1 strict 0 quanta 1 priomap 0 0 ... for ETS).
- Add a class with a tiny quantum of 1:
    tc class add dev veth0 parent 1: classid 1:1 drr quantum 1
- Send one small packet (e.g. `ping` or a single UDP datagram) on the
  device; the ~2 GiB accounting length makes the deficit loop spin
  billions of times under the qdisc lock, tripping the softlockup
  detector (panic with kernel.softlockup_panic=1).
- Reachable as root, or from an unprivileged user in a fresh user+net
  namespace (`unshare -Urn`) with namespace-local CAP_NET_ADMIN
  (Level 2).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: [email protected]
Tested-by: Victor Nogueira <[email protected]>
Signed-off-by: Jamal Hadi Salim <[email protected]>
---
 include/net/pkt_sched.h | 1 +
 net/sched/sch_api.c     | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index 18a419cd9d94..a4c1b7b0263d 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -12,6 +12,7 @@

 #define DEFAULT_TX_QUEUE_LEN	1000
 #define STAB_SIZE_LOG_MAX	30
+#define STAB_OVERHEAD_MAX	65535

 struct qdisc_walker {
 	int	stop;
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 65b35528d125..cdcf861c85f5 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -544,6 +544,11 @@ static struct qdisc_size_table *qdisc_get_stab(struct nlattr *opt,
 		return ERR_PTR(-EINVAL);
 	}

+	if (abs(s->overhead) > STAB_OVERHEAD_MAX) {
+		NL_SET_ERR_MSG(extack, "Invalid size table overhead");
+		return ERR_PTR(-EINVAL);
+	}
+
 	stab = kmalloc_flex(*stab, data, tsize);
 	if (!stab)
 		return ERR_PTR(-ENOMEM);
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.