diff for duplicates of <QDISC-2899.v2.20260901233641@mojatatu.com> diff --git a/a/1.txt b/N1/1.txt index 7ad4d20..24f4a87 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,68 +1,84 @@ -Several subsystems allocate ring buffers sized by dev->tx_queue_len -with no upper bound. An unprivileged user (via unshare -Urn) can set a -huge tx_queue_len and exhaust global memory with ring allocations: - -- pfifo_fast: pfifo_fast_init() and pfifo_fast_change_tx_queue_len() - allocate 3 skb_array rings of tx_queue_len entries each. -- tun: tun_queue_resize() and the queue-attach path resize ptr_rings - to tx_queue_len on the NETDEV_CHANGE_TX_QUEUE_LEN notifier. -- tap (macvtap/ipvtap): tap_queue_resize() and tap_init() resize/init - ptr_rings to tx_queue_len on the same notifier. - -netif_change_tx_queue_len() is the single entry point for IFLA_TXQLEN, -sysfs, and the SIOCSIFTXQLEN ioctl. Cap new_len at S16_MAX (32767) -there so the oversized value is rejected at set time. This takes -effect whether the device is up or down, before dev->tx_queue_len is -written, before any notifier fires, and before any ring is allocated. -The "> S16_MAX" check also subsumes the previous unsigned-long -truncation test, and a negative ifr_qlen from the ioctl lands far -above the cap after conversion, so both old failure modes are covered -by the one comparison. - -tx_queue_len is ambigious: both a per-ring sizing multiplier and a -default queue-length/limit knob for consumers that allocate -nothing at set time (pfifo/bfifo/gred/plug/sfb limits, htb -direct_qlen, qfq max_classes, teql). 32767 is chosen as the largest -value NLA_POLICY_FULL_RANGE can express for the u32 IFLA_TXQLEN -policy in patch 2/3 while staying a legitimate queue length on -high-BDP paths; the ring-memory trade-off of a shared knob is -disclosed below. - -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). -- pfifo_fast: create veth pairs, set tx_queue_len to 500000, attach - mq+pfifo_fast. ~28 iterations OOMs a 2GB guest. -- tun: create 50 tun devices with IFF_MULTI_QUEUE, set tx_queue_len to - 500000, open 8 queues each. ~1.6GB of ptr_ring allocations OOMs a - 512MB guest. -- tap: same as tun with IFF_TAP. ~960MB OOMs a 512MB guest. -- On the fixed kernel the oversized tx_queue_len is rejected with - -ERANGE at set time (all four paths: RTM_SETLINK, RTM_NEWLINK - create, sysfs, ioctl - the latter two via this check, the former - two via this check and the 2/3 parse policy respectively). - -Fixes: 6a643ddb5624 ("net: introduce helper dev_change_tx_queue_len()") -Reported-by: Vega <vega@nebusec.ai> -Closes: https://lore.kernel.org/netdev/20260828121902.66837-1-jhs@mojatatu.com/ -Tested-by: Victor Nogueira <victor@mojatatu.com> -Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> ---- - net/core/dev.c | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) - -diff --git a/net/core/dev.c b/net/core/dev.c -index 38336858c168..1d3fc0a268a5 100644 ---- a/net/core/dev.c -+++ b/net/core/dev.c -@@ -9982,7 +9982,7 @@ int netif_change_tx_queue_len(struct net_device *dev, unsigned long new_len) - unsigned int orig_len = dev->tx_queue_len; - int res; - -- if (new_len != (unsigned int)new_len) -+ if (new_len > S16_MAX) - return -ERANGE; - - if (new_len != orig_len) { +An unprivileged user (via unshare -Urn) can set a huge tx_queue_len +and exhaust global memory through ring allocations sized from it +(pfifo_fast skb_arrays, tun/tap ptr_rings). +The reproducer from vega@nebusec.ai set the following params for +illustration: txqlen of 500000 -> ~32 GiB/ring attempts, 1.6 GB tun, +~960 MB tap. Gets worse when you consider qdiscs like mq. + +What we fix: every path an unprivileged user can use to install +an oversized tx_queue_len is rejected with -ERANGE before any ring is +allocated; per-ring memory is bounded at 256 KiB. + +This is for you sashikos: What we deliberately _do not fix_ +bound the NUMBER of rings. With the cap in place the worst case moves +from "one knob" to the aggregate of ring x queues x devices, example: + + ip link add v0 numtxqueues 4096 txqueuelen 32767 type veth + tc qdisc add dev v0 root mq + -> 4096 * 3 * 32767 * 8 = ~3.0 GiB (one command) + 50 tun devices x 256 queues x 32767 x 8 = ~3.1 GiB + +Unfortunately tx_queue_len is a bit ambigious in meaning: +In some cases it means a ring size (which is pre-allocated, ex: +tun, tap, and pfifo_fast); a cap of 4096 seems reasonable here. +but in other cases it is used to indicate a queue limit ex: +the qdisc consumers that allocate nothing (pfifo/bfifo/gred/plug/sfb, +htb direct_qlen, qfq, teql). 32767 is a legitimate high-BDP queue +length, so we are going to keep that value. + +Getting back to you sashikos, after this is merged and shows up +in net-next we will send followup patches as follows: +this series is not misread as "closes the OOM class"): + +a) Per-site ring limits at six identified locations + - pfifo_fast init/resize, + - tun attach/resize, + - tap minor/resize) + + if you can spot more in your review we will take care of those as well. + +b) memcg accounting (GFP_KERNEL_ACCOUNT) for those ring + allocations: contains a memcg-limited container's ring memory. + Not GFP_KERNEL_ACCOUNT has no effect on the unshare attacker + but will protect against containers (memory.max in its cgroup) + + +Patches: +-------- + + 1/3 net: cap tx_queue_len at S16_MAX in netif_change_tx_queue_len() + (netlink set, sysfs, SIOCSIFTXQLEN choke point) + 2/3 net: reject oversized tx_queue_len at netlink parse time + (IFLA_TXQLEN policy: closes the create path + veth peer nest) + 3/3 selftests: tdc regression tests (netlink, sysfs, create paths) + +Changes: +-------- +v1 -> v2: +- new patch 2/3: close the device-creation path (Jakub Kicinski + flagged that rtnl_create_link() bypasses the cap) +- rationale reworded: the 32767 ceiling citing NLA u32 range + policy can express (s16 bounds), replacing the invalid virtio + ring-depth claim +- rebuilt tdc coverage (sysfs path now tested; nondeterministic + resize-rollback case dropped) + +Sashiko v1 review links: +https://sashiko.dev/#/patchset/20260828121902.66837-1-jhs@mojatatu.com +https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260828121902.66837-1-jhs@mojatatu.com + +v1: https://lore.kernel.org/netdev/20260828121902.66837-1-jhs@mojatatu.com/ + +Jamal Hadi Salim (3): + net: cap tx_queue_len at S16_MAX to prevent oversized ring + allocations + net: reject oversized tx_queue_len at netlink parse time + selftests: tc-testing: add tx_queue_len cap regression tests + + .../tc-testing/tc-tests/qdiscs/pfifo_fast.json | 209 +++++++++++++++++- + net/core/dev.c | 2 +- + net/core/rtnetlink.c | 9 +- + 3 files changed, 211 insertions(+), 2 deletions(-) + -- 2.43.0 diff --git a/a/content_digest b/N1/content_digest index 125bb79..41b9151 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,6 +1,6 @@ "From\0Jamal Hadi Salim <jhs@mojatatu.com>\0" - "Subject\0[PATCH net v2 1/3] net: cap tx_queue_len at S16_MAX to prevent oversized ring allocations\0" - "Date\0Wed, 2 Sep 2026 17:29:08 -0400\0" + "Subject\0[PATCH net v2 0/3] net: cap tx_queue_len at S16_MAX to prevent oversized ring allocations\0" + "Date\0Wed, 2 Sep 2026 17:29:07 -0400\0" "To\0netdev@vger.kernel.org\0" "Cc\0Jamal Hadi Salim <jhs@mojatatu.com>" stable@vger.kernel.org @@ -15,73 +15,89 @@ " Victor Nogueira <victor@mojatatu.com>\0" "\00:1\0" "b\0" - "Several subsystems allocate ring buffers sized by dev->tx_queue_len\n" - "with no upper bound. An unprivileged user (via unshare -Urn) can set a\n" - "huge tx_queue_len and exhaust global memory with ring allocations:\n" - "\n" - "- pfifo_fast: pfifo_fast_init() and pfifo_fast_change_tx_queue_len()\n" - " allocate 3 skb_array rings of tx_queue_len entries each.\n" - "- tun: tun_queue_resize() and the queue-attach path resize ptr_rings\n" - " to tx_queue_len on the NETDEV_CHANGE_TX_QUEUE_LEN notifier.\n" - "- tap (macvtap/ipvtap): tap_queue_resize() and tap_init() resize/init\n" - " ptr_rings to tx_queue_len on the same notifier.\n" - "\n" - "netif_change_tx_queue_len() is the single entry point for IFLA_TXQLEN,\n" - "sysfs, and the SIOCSIFTXQLEN ioctl. Cap new_len at S16_MAX (32767)\n" - "there so the oversized value is rejected at set time. This takes\n" - "effect whether the device is up or down, before dev->tx_queue_len is\n" - "written, before any notifier fires, and before any ring is allocated.\n" - "The \"> S16_MAX\" check also subsumes the previous unsigned-long\n" - "truncation test, and a negative ifr_qlen from the ioctl lands far\n" - "above the cap after conversion, so both old failure modes are covered\n" - "by the one comparison.\n" - "\n" - "tx_queue_len is ambigious: both a per-ring sizing multiplier and a\n" - "default queue-length/limit knob for consumers that allocate\n" - "nothing at set time (pfifo/bfifo/gred/plug/sfb limits, htb\n" - "direct_qlen, qfq max_classes, teql). 32767 is chosen as the largest\n" - "value NLA_POLICY_FULL_RANGE can express for the u32 IFLA_TXQLEN\n" - "policy in patch 2/3 while staying a legitimate queue length on\n" - "high-BDP paths; the ring-memory trade-off of a shared knob is\n" - "disclosed below.\n" - "\n" - "Conditions to recreate the bug:\n" - "- CONFIG_NET_SCHED=y, CONFIG_VETH=y, CONFIG_USER_NS=y, CONFIG_NET_NS=y.\n" - "- Unprivileged user in a fresh user+net namespace (unshare -Urn).\n" - "- pfifo_fast: create veth pairs, set tx_queue_len to 500000, attach\n" - " mq+pfifo_fast. ~28 iterations OOMs a 2GB guest.\n" - "- tun: create 50 tun devices with IFF_MULTI_QUEUE, set tx_queue_len to\n" - " 500000, open 8 queues each. ~1.6GB of ptr_ring allocations OOMs a\n" - " 512MB guest.\n" - "- tap: same as tun with IFF_TAP. ~960MB OOMs a 512MB guest.\n" - "- On the fixed kernel the oversized tx_queue_len is rejected with\n" - " -ERANGE at set time (all four paths: RTM_SETLINK, RTM_NEWLINK\n" - " create, sysfs, ioctl - the latter two via this check, the former\n" - " two via this check and the 2/3 parse policy respectively).\n" - "\n" - "Fixes: 6a643ddb5624 (\"net: introduce helper dev_change_tx_queue_len()\")\n" - "Reported-by: Vega <vega@nebusec.ai>\n" - "Closes: https://lore.kernel.org/netdev/20260828121902.66837-1-jhs@mojatatu.com/\n" - "Tested-by: Victor Nogueira <victor@mojatatu.com>\n" - "Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>\n" - "---\n" - " net/core/dev.c | 2 +-\n" - " 1 file changed, 1 insertion(+), 1 deletion(-)\n" - "\n" - "diff --git a/net/core/dev.c b/net/core/dev.c\n" - "index 38336858c168..1d3fc0a268a5 100644\n" - "--- a/net/core/dev.c\n" - "+++ b/net/core/dev.c\n" - "@@ -9982,7 +9982,7 @@ int netif_change_tx_queue_len(struct net_device *dev, unsigned long new_len)\n" - " \tunsigned int orig_len = dev->tx_queue_len;\n" - " \tint res;\n" - " \n" - "-\tif (new_len != (unsigned int)new_len)\n" - "+\tif (new_len > S16_MAX)\n" - " \t\treturn -ERANGE;\n" - " \n" - " \tif (new_len != orig_len) {\n" + "An unprivileged user (via unshare -Urn) can set a huge tx_queue_len\n" + "and exhaust global memory through ring allocations sized from it\n" + "(pfifo_fast skb_arrays, tun/tap ptr_rings).\n" + "The reproducer from vega@nebusec.ai set the following params for\n" + "illustration: txqlen of 500000 -> ~32 GiB/ring attempts, 1.6 GB tun,\n" + "~960 MB tap. Gets worse when you consider qdiscs like mq.\n" + "\n" + "What we fix: every path an unprivileged user can use to install\n" + "an oversized tx_queue_len is rejected with -ERANGE before any ring is\n" + "allocated; per-ring memory is bounded at 256 KiB.\n" + "\n" + "This is for you sashikos: What we deliberately _do not fix_\n" + "bound the NUMBER of rings. With the cap in place the worst case moves\n" + "from \"one knob\" to the aggregate of ring x queues x devices, example:\n" + "\n" + " ip link add v0 numtxqueues 4096 txqueuelen 32767 type veth\n" + " tc qdisc add dev v0 root mq\n" + " -> 4096 * 3 * 32767 * 8 = ~3.0 GiB (one command)\n" + " 50 tun devices x 256 queues x 32767 x 8 = ~3.1 GiB\n" + "\n" + "Unfortunately tx_queue_len is a bit ambigious in meaning:\n" + "In some cases it means a ring size (which is pre-allocated, ex:\n" + "tun, tap, and pfifo_fast); a cap of 4096 seems reasonable here.\n" + "but in other cases it is used to indicate a queue limit ex:\n" + "the qdisc consumers that allocate nothing (pfifo/bfifo/gred/plug/sfb,\n" + "htb direct_qlen, qfq, teql). 32767 is a legitimate high-BDP queue\n" + "length, so we are going to keep that value.\n" + "\n" + "Getting back to you sashikos, after this is merged and shows up\n" + "in net-next we will send followup patches as follows:\n" + "this series is not misread as \"closes the OOM class\"):\n" + "\n" + "a) Per-site ring limits at six identified locations\n" + " - pfifo_fast init/resize,\n" + " - tun attach/resize,\n" + " - tap minor/resize)\n" + "\n" + " if you can spot more in your review we will take care of those as well.\n" + "\n" + "b) memcg accounting (GFP_KERNEL_ACCOUNT) for those ring\n" + " allocations: contains a memcg-limited container's ring memory.\n" + " Not GFP_KERNEL_ACCOUNT has no effect on the unshare attacker\n" + " but will protect against containers (memory.max in its cgroup)\n" + "\n" + "\n" + "Patches:\n" + "--------\n" + "\n" + " 1/3 net: cap tx_queue_len at S16_MAX in netif_change_tx_queue_len()\n" + " (netlink set, sysfs, SIOCSIFTXQLEN choke point)\n" + " 2/3 net: reject oversized tx_queue_len at netlink parse time\n" + " (IFLA_TXQLEN policy: closes the create path + veth peer nest)\n" + " 3/3 selftests: tdc regression tests (netlink, sysfs, create paths)\n" + "\n" + "Changes:\n" + "--------\n" + "v1 -> v2:\n" + "- new patch 2/3: close the device-creation path (Jakub Kicinski\n" + " flagged that rtnl_create_link() bypasses the cap)\n" + "- rationale reworded: the 32767 ceiling citing NLA u32 range\n" + " policy can express (s16 bounds), replacing the invalid virtio\n" + " ring-depth claim\n" + "- rebuilt tdc coverage (sysfs path now tested; nondeterministic\n" + " resize-rollback case dropped)\n" + "\n" + "Sashiko v1 review links:\n" + "https://sashiko.dev/#/patchset/20260828121902.66837-1-jhs@mojatatu.com\n" + "https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260828121902.66837-1-jhs@mojatatu.com\n" + "\n" + "v1: https://lore.kernel.org/netdev/20260828121902.66837-1-jhs@mojatatu.com/\n" + "\n" + "Jamal Hadi Salim (3):\n" + " net: cap tx_queue_len at S16_MAX to prevent oversized ring\n" + " allocations\n" + " net: reject oversized tx_queue_len at netlink parse time\n" + " selftests: tc-testing: add tx_queue_len cap regression tests\n" + "\n" + " .../tc-testing/tc-tests/qdiscs/pfifo_fast.json | 209 +++++++++++++++++-\n" + " net/core/dev.c | 2 +-\n" + " net/core/rtnetlink.c | 9 +-\n" + " 3 files changed, 211 insertions(+), 2 deletions(-)\n" + "\n" "-- \n" 2.43.0 -a300be00835f27944d1bd9760770785c565716e02be27c0c1eb360af72c45d5b +49aac03190f459291f8e2368777694a197f152fe4de7a31c10271f7273b484c8
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.