[PATCH net v2 1/2] net/sched: pfifo_fast: reject oversized ring and account to memcg
Jamal Hadi Salim <[email protected]>
| Newsgroups | org.kernel.vger.stable,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
pfifo_fast_init() and pfifo_fast_change_tx_queue_len() allocate skb
ring arrays sized by dev->tx_queue_len with GFP_KERNEL and no upper
bound. An unprivileged user (via unshare -Urn) can set a huge
tx_queue_len and attach many pfifo_fast qdiscs to exhaust global
memory, causing a system-wide OOM.
Reject tx_queue_len values exceeding S16_MAX (32767) with -ERANGE
in both pfifo_fast_init() and pfifo_fast_change_tx_queue_len().
Note: For the init path, NL_SET_ERR_MSG_FMT_MOD reports the error
via extack whereas for the resize path, the error propagates to
netif_change_tx_queue_len() which rolls back dev->tx_queue_len to
the original value. Use GFP_KERNEL_ACCOUNT so the ring allocations
are charged to the allocating process's memory cgroup.
S16_MAX is the virtio virtqueue size limit: the virtio specification
stores the queue size as a u16 with a maximum of 32768, so 32767 is
the largest tx_queue_len any in-tree driver can meaningfully use.
Conditions to recreate the bug:
- CONFIG_NET_SCHED=y, CONFIG_VETH=y, CONFIG_USER_NS=y, CONFIG_NET_NS=y.
- Unprivileged user in a fresh user+net namespace (unshare -Urn).
- Create a veth pair, set tx_queue_len to a huge value (e.g. 500000)
while the devices are down.
- Attach mq at root, then replace each child queue with pfifo_fast:
tc qdisc replace dev veth0 root handle 1: mq
tc qdisc replace dev veth0 parent 1:1 pfifo_fast
tc qdisc replace dev veth0 parent 1:2 pfifo_fast ...
- Repeat across many veth pairs. Each pfifo_fast allocates 3 skb_array
rings of tx_queue_len entries (~12MB per qdisc at QLEN=500000).
- On the unfixed kernel this exhausts global memory in ~28 iterations
on a 2GB guest -> OOM panic. On the fixed kernel the oversized
tx_queue_len is rejected with -ERANGE.
Fixes: c5ad119fb6c0 ("net: sched: pfifo_fast use skb_array")
Reported-by: [email protected]
Tested-by: Victor Nogueira <[email protected]>
Signed-off-by: Jamal Hadi Salim <[email protected]>
---
v1 -> v2:
- Replaced silent clamp + pr_warn_ratelimited with reject (-ERANGE) (Jakub)
- Changed cap from 65535 to S16_MAX (32767), matching virtio's
virtio16 ring size limit.
- Dropped the doubled module prefix in extack (NL_SET_ERR_MSG_FMT_MOD
already prepends KBUILD_MODNAME).
- Added resize-path tdc test case (Sashiko nipa gpt-5-6-sol-1-2).
- Fixed tdc teardown to use JSON list form for acceptable exit codes.
---
net/sched/sch_generic.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index ef2b4bf51564..eb5c0d3f67c2 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -910,11 +910,18 @@ static int pfifo_fast_init(struct Qdisc *qdisc, struct nlattr *opt,
if (!qlen)
return -EINVAL;
+ if (qlen > S16_MAX) {
+ NL_SET_ERR_MSG_FMT_MOD(extack,
+ "ring size %u too large (max %d)",
+ qlen, S16_MAX);
+ return -ERANGE;
+ }
+
for (prio = 0; prio < PFIFO_FAST_BANDS; prio++) {
struct skb_array *q = band2list(priv, prio);
int err;
- err = skb_array_init(q, qlen, GFP_KERNEL);
+ err = skb_array_init(q, qlen, GFP_KERNEL_ACCOUNT);
if (err)
return -ENOMEM;
}
@@ -957,8 +964,11 @@ static int pfifo_fast_change_tx_queue_len(struct Qdisc *sch,
bands[prio] = q;
}
+ if (new_len > S16_MAX)
+ return -ERANGE;
+
return skb_array_resize_multiple_bh(bands, PFIFO_FAST_BANDS, new_len,
- GFP_KERNEL);
+ GFP_KERNEL_ACCOUNT);
}
struct Qdisc_ops pfifo_fast_ops __read_mostly = {
--
2.43.0