[PATCH net v2 1/2] net/sched: pfifo_fast: reject oversized ring and account to memcg

[PATCH net v2 1/2] net/sched: pfifo_fast: reject oversized ring and account to memcg

[Jamal Hadi Salim]

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: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
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

[PATCH net v2 2/2] selftests: tc-testing: add pfifo_fast ring size cap regression tests

[Jamal Hadi Salim]

Add two test cases for the pfifo_fast ring size cap:

1. Init path: set txqueuelen to 100000000, attempt to attach pfifo_fast.
   On a fixed kernel the qdisc add is rejected with -ERANGE (exit code 2).
   On an unfixed kernel the allocation either succeeds or triggers OOM.

2. Resize path: attach pfifo_fast with default txqueuelen, then raise
   txqueuelen to 100000000. On a fixed kernel the resize is rejected
   with -ERANGE and txqueuelen rolls back to the original value (1000).
   On an unfixed kernel the ring is resized to the oversized value.

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 .../tc-tests/qdiscs/pfifo_fast.json           | 60 +++++++++++++++++--
 1 file changed, 54 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/pfifo_fast.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/pfifo_fast.json
index 30da27fe8806..46b54ae37a91 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/pfifo_fast.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/pfifo_fast.json
@@ -9,8 +9,7 @@
         "plugins": {
             "requires": "nsPlugin"
         },
-        "setup": [
-        ],
+        "setup": [],
         "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root pfifo_fast",
         "expExitCode": "0",
         "verifyCmd": "$TC qdisc show dev $DUMMY",
@@ -30,8 +29,7 @@
         "plugins": {
             "requires": "nsPlugin"
         },
-        "setup": [
-        ],
+        "setup": [],
         "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root pfifo_fast",
         "expExitCode": "0",
         "verifyCmd": "$TC -s qdisc show dev $DUMMY",
@@ -81,8 +79,7 @@
         "verifyCmd": "$TC qdisc show dev $DUMMY",
         "matchPattern": "qdisc pfifo_fast 1: root refcnt [0-9]+ bands 3 priomap",
         "matchCount": "0",
-        "teardown": [
-        ]
+        "teardown": []
     },
     {
         "id": "4385",
@@ -105,5 +102,56 @@
         "teardown": [
             "$TC qdisc del dev $DUMMY handle 1: root"
         ]
+    },
+    {
+        "id": "7d12",
+        "name": "Reject pfifo_fast init with tx_queue_len exceeding S16_MAX",
+        "category": [
+            "qdisc",
+            "pfifo_fast"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$IP link set dev $DUMMY txqueuelen 100000000"
+        ],
+        "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root pfifo_fast",
+        "expExitCode": "2",
+        "verifyCmd": "$TC qdisc show dev $DUMMY",
+        "matchPattern": "qdisc pfifo_fast 1: root",
+        "matchCount": "0",
+        "teardown": [
+            "$IP link set dev $DUMMY txqueuelen 1000",
+            [
+                "$TC qdisc del dev $DUMMY handle 1: root",
+                0,
+                1,
+                2,
+                255
+            ]
+        ]
+    },
+    {
+        "id": "d09b",
+        "name": "Reject pfifo_fast resize when tx_queue_len exceeds S16_MAX",
+        "category": [
+            "qdisc",
+            "pfifo_fast"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DUMMY handle 1: root pfifo_fast"
+        ],
+        "cmdUnderTest": "$IP link set dev $DUMMY txqueuelen 100000000",
+        "expExitCode": "2",
+        "verifyCmd": "$IP link show dev $DUMMY",
+        "matchPattern": "qlen 1000$",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY handle 1: root"
+        ]
     }
 ]
-- 
2.43.0

Re: [PATCH net v2 1/2] net/sched: pfifo_fast: reject oversized ring and account to memcg

[Paolo Abeni]

On 8/25/26 10:17 AM, Jamal Hadi Salim wrote:
> 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: vega@nebusec.ai
> Tested-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
> 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;

Sashiko noted that setting a large tx_queue_len to a down
interface gives an inconsistent behavior:

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260825081751.134086-1-jhs%40mojatatu.com

(all other comments are IMHO noise and should be ignored)

I don't see and effective way to avoid that, short of falling
back to v1, WDYT?

/P

Re: [PATCH net v2 1/2] net/sched: pfifo_fast: reject oversized ring and account to memcg

[Jamal Hadi Salim]

On Thu, Aug 27, 2026 at 7:26 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 8/25/26 10:17 AM, Jamal Hadi Salim wrote:
> > 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: vega@nebusec.ai
> > Tested-by: Victor Nogueira <victor@mojatatu.com>
> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > ---
> > 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;
>
> Sashiko noted that setting a large tx_queue_len to a down
> interface gives an inconsistent behavior:
>
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260825081751.134086-1-jhs%40mojatatu.com
>
> (all other comments are IMHO noise and should be ignored)
>
> I don't see and effective way to avoid that, short of falling
> back to v1, WDYT?

I changed my mind on this. See this thread and let me know what you think:
https://lore.kernel.org/netdev/CAM0EoMk8fAACt11fe1pUsFeG9np3SdsV0YUNmEevLu=i=b+xKg@mail.gmail.com/

cheers,
jamal
> /P
>

Re: [PATCH net v2 1/2] net/sched: pfifo_fast: reject oversized ring and account to memcg

[Paolo Abeni]

On 8/27/26 7:39 PM, Jamal Hadi Salim wrote:
> On Thu, Aug 27, 2026 at 7:26 AM Paolo Abeni <pabeni@redhat.com> wrote:
>> On 8/25/26 10:17 AM, Jamal Hadi Salim wrote:
>>> 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: vega@nebusec.ai
>>> Tested-by: Victor Nogueira <victor@mojatatu.com>
>>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>>> ---
>>> 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;
>>
>> Sashiko noted that setting a large tx_queue_len to a down
>> interface gives an inconsistent behavior:
>>
>> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260825081751.134086-1-jhs%40mojatatu.com
>>
>> (all other comments are IMHO noise and should be ignored)
>>
>> I don't see and effective way to avoid that, short of falling
>> back to v1, WDYT?
> 
> I changed my mind on this. See this thread and let me know what you think:
> https://lore.kernel.org/netdev/CAM0EoMk8fAACt11fe1pUsFeG9np3SdsV0YUNmEevLu=i=b+xKg@mail.gmail.com/
Whoops, I missed the last message in such a thread. I think it makes
sense and will cover all the sashiko-reported concerns. You can probably
also additionally get rid of the old test:

	 if (new_len != (unsigned int)new_len)
                return -ERANGE;

/P

Re: [PATCH net v2 1/2] net/sched: pfifo_fast: reject oversized ring and account to memcg

[Jamal Hadi Salim]

On Fri, Aug 28, 2026 at 2:28 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 8/27/26 7:39 PM, Jamal Hadi Salim wrote:
> > On Thu, Aug 27, 2026 at 7:26 AM Paolo Abeni <pabeni@redhat.com> wrote:
> >> On 8/25/26 10:17 AM, Jamal Hadi Salim wrote:
> >>> 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: vega@nebusec.ai
> >>> Tested-by: Victor Nogueira <victor@mojatatu.com>
> >>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> >>> ---
> >>> 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;
> >>
> >> Sashiko noted that setting a large tx_queue_len to a down
> >> interface gives an inconsistent behavior:
> >>
> >> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260825081751.134086-1-jhs%40mojatatu.com
> >>
> >> (all other comments are IMHO noise and should be ignored)
> >>
> >> I don't see and effective way to avoid that, short of falling
> >> back to v1, WDYT?
> >
> > I changed my mind on this. See this thread and let me know what you think:
> > https://lore.kernel.org/netdev/CAM0EoMk8fAACt11fe1pUsFeG9np3SdsV0YUNmEevLu=i=b+xKg@mail.gmail.com/
> Whoops, I missed the last message in such a thread. I think it makes
> sense and will cover all the sashiko-reported concerns. You can probably
> also additionally get rid of the old test:
>
>          if (new_len != (unsigned int)new_len)
>                 return -ERANGE;

Good catch; that check becomes dead once the S16_MAX cap is in place.
I'll drop it
How do i "withdraw" this patch? Is it sufficient to say:
pw-bot: cr

Based on what you said to Victor: Should i keep GFP_KERNEL_ACCOUNT or
make it a followup to net-next?
The tdc test patch is still valid.

cheers,
jamal


>

Re: [PATCH net v2 1/2] net/sched: pfifo_fast: reject oversized ring and account to memcg

[Jamal Hadi Salim]

On Fri, Aug 28, 2026 at 6:29 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Fri, Aug 28, 2026 at 2:28 AM Paolo Abeni <pabeni@redhat.com> wrote:
> >
> > On 8/27/26 7:39 PM, Jamal Hadi Salim wrote:
> > > On Thu, Aug 27, 2026 at 7:26 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > >> On 8/25/26 10:17 AM, Jamal Hadi Salim wrote:
> > >>> 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: vega@nebusec.ai
> > >>> Tested-by: Victor Nogueira <victor@mojatatu.com>
> > >>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > >>> ---
> > >>> 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;
> > >>
> > >> Sashiko noted that setting a large tx_queue_len to a down
> > >> interface gives an inconsistent behavior:
> > >>
> > >> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260825081751.134086-1-jhs%40mojatatu.com
> > >>
> > >> (all other comments are IMHO noise and should be ignored)
> > >>
> > >> I don't see and effective way to avoid that, short of falling
> > >> back to v1, WDYT?
> > >
> > > I changed my mind on this. See this thread and let me know what you think:
> > > https://lore.kernel.org/netdev/CAM0EoMk8fAACt11fe1pUsFeG9np3SdsV0YUNmEevLu=i=b+xKg@mail.gmail.com/
> > Whoops, I missed the last message in such a thread. I think it makes
> > sense and will cover all the sashiko-reported concerns. You can probably
> > also additionally get rid of the old test:
> >
> >          if (new_len != (unsigned int)new_len)
> >                 return -ERANGE;
>
> Good catch; that check becomes dead once the S16_MAX cap is in place.
> I'll drop it
> How do i "withdraw" this patch? Is it sufficient to say:
> pw-bot: cr
>
> Based on what you said to Victor: Should i keep GFP_KERNEL_ACCOUNT or
> make it a followup to net-next?

Ignore this part -  i am not going to make any changes to pfifo.

cheers,
jamal
> The tdc test patch is still valid.
>
> cheers,
> jamal
>
>
> >