[PATCH net] net: reject flow_limit_table_len values below 2

[PATCH net] net: reject flow_limit_table_len values below 2

[Dudu Lu]

flow_limit_table_len_sysctl() rejects non-powers-of-two but accepts 1,
which is 2^0.  A length of 1 then gives log_buckets = ilog2(1) = 0, and
skb_flow_limit() computes hash_32(hash, 0):

  include/linux/hash.h:68
	return __hash_32(val) >> (32 - bits);   /* bits == 0 -> shift by 32 */

A shift by 32 is undefined and, on arm64/x86, the "hash" becomes the full
32-bit product val * GOLDEN_RATIO_32 -- an unbounded index into a
buckets[] flex array that is one byte long inside a 289-byte allocation:

  UBSAN: shift-out-of-bounds in ./include/linux/hash.h:68:24
  shift exponent 32 is too large for 32-bit type 'unsigned int'
   __ubsan_handle_shift_out_of_bounds+0x11c/0x208
   enqueue_to_backlog+0xbe4/0xc44
   netif_rx_internal+0x130/0x4d4

The subsequent read-modify-write at dev.c:5443-5446 is an out-of-bounds
write into the allocation (also observed as use-after-free).  Require at
least two buckets so log_buckets is never 0.

Fixes: 99bbc7074190 ("rps: selective flow shedding during softnet overflow")
Assisted-by: LLM
Signed-off-by: Dudu Lu <phx0fer@gmail.com>
---
Tested base
-----------
Detected on Linux 7.3-rc4 (commit 93f51579e7df), arm64 QEMU guest.
Re-checked against mainline f47d223627f8 (v7.3-341): the affected code
is unchanged between the two, so the bug is still present there.
Checked against linux-next next-20261021 (commit 9a482184d910): the
affected code is unchanged there as well, so the bug is still present in
linux-next too.

Reproducer status
-----------------
A reproducer was written and executed on an arm64 QEMU guest (KASAN + UBSAN);
UBSAN fired at the claimed sink.  The reproducer source is not posted here per
Documentation/process/security-bugs.rst; it is available on request.

Triggering conditions
---------------------
root (flow_limit_table_len and netdev_max_backlog live only in the init-net
net_core_table, absent from the netns table).

Notes
-----
Built and checkpatch-clean: this patch was applied on top of 7.3-rc4
(commit 93f51579e7df) and a full arm64 kernel (defconfig) was built with
it -- no new warnings or errors.  scripts/checkpatch.pl reports 0 errors,
0 warnings.

 net/core/sysctl_net_core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index eb35da3556f4..bb0fa69f94f5 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -283,7 +283,12 @@ static int flow_limit_table_len_sysctl(const struct ctl_table *table, int write,
 	ptr = table->data;
 	old = *ptr;
 	ret = proc_dointvec(table, write, buffer, lenp, ppos);
-	if (!ret && write && !is_power_of_2(*ptr)) {
+	/* A length of 1 is a power of two but gives log_buckets == 0,
+	 * which makes skb_flow_limit() compute hash_32(hash, 0) -- a
+	 * shift by 32 -- and index the one-byte buckets[] array with the
+	 * full 32-bit hash.  Require at least two buckets.
+	 */
+	if (!ret && write && (*ptr < 2 || !is_power_of_2(*ptr))) {
 		*ptr = old;
 		ret = -EINVAL;
 	}

Re: [PATCH net] net: reject flow_limit_table_len values below 2

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net: reject flow_limit_table_len values below 2

[Jiayuan Chen]

On 10/21/26 11:55 PM, Dudu Lu wrote:
> flow_limit_table_len_sysctl() rejects non-powers-of-two but accepts 1,
> which is 2^0.  A length of 1 then gives log_buckets = ilog2(1) = 0, and
> skb_flow_limit() computes hash_32(hash, 0):
>
>    include/linux/hash.h:68
> 	return __hash_32(val) >> (32 - bits);   /* bits == 0 -> shift by 32 */
>
> A shift by 32 is undefined and, on arm64/x86, the "hash" becomes the full
> 32-bit product val * GOLDEN_RATIO_32 -- an unbounded index into a
> buckets[] flex array that is one byte long inside a 289-byte allocation:
>
>    UBSAN: shift-out-of-bounds in ./include/linux/hash.h:68:24
>    shift exponent 32 is too large for 32-bit type 'unsigned int'
>     __ubsan_handle_shift_out_of_bounds+0x11c/0x208
>     enqueue_to_backlog+0xbe4/0xc44
>     netif_rx_internal+0x130/0x4d4
>
> The subsequent read-modify-write at dev.c:5443-5446 is an out-of-bounds
> write into the allocation (also observed as use-after-free).  Require at
> least two buckets so log_buckets is never 0.
>
> Fixes: 99bbc7074190 ("rps: selective flow shedding during softnet overflow")
> Assisted-by: LLM
> Signed-off-by: Dudu Lu <phx0fer@gmail.com>
> ---
> Tested base
> -----------
> Detected on Linux 7.3-rc4 (commit 93f51579e7df), arm64 QEMU guest.
> Re-checked against mainline f47d223627f8 (v7.3-341): the affected code
> is unchanged between the two, so the bug is still present there.
> Checked against linux-next next-20261021 (commit 9a482184d910): the
> affected code is unchanged there as well, so the bug is still present in
> linux-next too.
>
> Reproducer status
> -----------------
> A reproducer was written and executed on an arm64 QEMU guest (KASAN + UBSAN);
> UBSAN fired at the claimed sink.  The reproducer source is not posted here per
> Documentation/process/security-bugs.rst; it is available on request.
>
> Triggering conditions
> ---------------------
> root (flow_limit_table_len and netdev_max_backlog live only in the init-net
> net_core_table, absent from the netns table).
>
> Notes
> -----
> Built and checkpatch-clean: this patch was applied on top of 7.3-rc4
> (commit 93f51579e7df) and a full arm64 kernel (defconfig) was built with
> it -- no new warnings or errors.  scripts/checkpatch.pl reports 0 errors,
> 0 warnings.
>
>   net/core/sysctl_net_core.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index eb35da3556f4..bb0fa69f94f5 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -283,7 +283,12 @@ static int flow_limit_table_len_sysctl(const struct ctl_table *table, int write,
>   	ptr = table->data;
>   	old = *ptr;
>   	ret = proc_dointvec(table, write, buffer, lenp, ppos);
> -	if (!ret && write && !is_power_of_2(*ptr)) {
> +	/* A length of 1 is a power of two but gives log_buckets == 0,
> +	 * which makes skb_flow_limit() compute hash_32(hash, 0) -- a
> +	 * shift by 32 -- and index the one-byte buckets[] array with the
> +	 * full 32-bit hash.  Require at least two buckets.
> +	 */
> +	if (!ret && write && (*ptr < 2 || !is_power_of_2(*ptr))) {
>   		*ptr = old;
>   		ret = -EINVAL;
>   	}




1. Need more thought about flow_limit_table_len=1.
A single bucket still has well-defined semantics: once the backlog
is half full, every packet hitting skb_flow_limit() is dropped. The
same effect can be achieved by reducing netdev_max_backlog, so
rejecting 1 may be acceptable.

2. ptr is unsigned, so -2147483648 (0x80000000) still passes your check, 
as it is >= 2 and a power of two.

3. sd_flow_limit::history is u16, and the bucket index is saved into it:

skb_flow_limit():
      /* bucket index, 0 .. flow_limit_table_len - 1 */
      new_flow = hash_32(skb_get_hash(skb), fl->log_buckets);
      /* saved into a u16 */
      fl->history[fl->history_head] = new_flow;

So with flow_limit_table_len > 65536 the bucket index gets truncated
when stored, and 128 packets later buckets[old_flow]-- decrements the
wrong bucket. It is not an out-of-bounds access, but the accounting
becomes wrong. An upper bound of 65536 seems needed as well.

Re: [PATCH net] net: reject flow_limit_table_len values below 2

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 3 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The Fixes: tag blames the wrong commit.
- [Medium] The new lower-bound check does not reject every value below 2,
  even though the patch title says it does.
- [Medium] net: signedness mismatch in flow_limit_table_len_sysctl()
  accepts INT_MIN

Pre-existing issues:
- [Low] This problem wasn't introduced by this patch, but
  Documentation/networking/scaling.rst does not match the code.

--

net: reject flow_limit_table_len values below 2

This patch makes flow_limit_table_len_sysctl() reject a length of 1 as
well as values that are not powers of two. A length of 1 gave
log_buckets == 0, so skb_flow_limit() computed hash_32(hash, 0). That is a
shift by 32, and it produced an out-of-bounds index into the buckets[]
array.

> Fixes: 99bbc7074190 ("rps: selective flow shedding during softnet overflow")

[Severity: Medium]
Is this the right commit for the Fixes: tag?

Before c3025e94daa9 ("net: rps: change skb_flow_limit() hash function"),
skb_flow_limit() computed the bucket index with a mask:

    new_flow = skb_get_hash(skb) & (fl->num_buckets - 1);

and flow_limit_cpu_sysctl() stored:

    cur->num_buckets = netdev_flow_limit_table_len;

With a length of 1 the mask is 0. The index was always 0, which is inside
the one-byte buckets[] array.

The shift by 32 only became possible when c3025e94daa9 changed this to:

    new_flow = hash_32(skb_get_hash(skb), fl->log_buckets);
    cur->log_buckets = ilog2(netdev_flow_limit_table_len);

With the current tag, stable branches older than c3025e94daa9 would pick
this patch up. In those branches it fixes nothing, but writing 1 to
net.core.flow_limit_table_len would start returning -EINVAL.

Should this be the following instead?

Fixes: c3025e94daa9 ("net: rps: change skb_flow_limit() hash function")

> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index eb35da3556f4a..bb0fa69f94f5a 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -283,7 +283,12 @@ static int flow_limit_table_len_sysctl(const struct ctl_table *table, int write,
>  	ptr = table->data;
>  	old = *ptr;
>  	ret = proc_dointvec(table, write, buffer, lenp, ppos);
> -	if (!ret && write && !is_power_of_2(*ptr)) {
> +	/* A length of 1 is a power of two but gives log_buckets == 0,
> +	 * which makes skb_flow_limit() compute hash_32(hash, 0) -- a
> +	 * shift by 32 -- and index the one-byte buckets[] array with the
> +	 * full 32-bit hash.  Require at least two buckets.
> +	 */
> +	if (!ret && write && (*ptr < 2 || !is_power_of_2(*ptr))) {

[Severity: Medium]
Does the new lower bound hold for every value proc_dointvec() can
store here?

netdev_flow_limit_table_len is a signed int, and proc_dointvec()
parses writes through proc_int_u2k_conv_uop(), which accepts
"-2147483648" and stores INT_MIN. flow_limit_table_len_sysctl() then
reads the result through unsigned int *ptr and sees 0x80000000. That
is not below 2, and is_power_of_2() is true for it, so the write is
accepted even though the subject says values below 2 are rejected.

If a CPU is then enabled through net.core.flow_limit_cpu_bitmap,
flow_limit_cpu_sysctl() computes

    len = sizeof(*cur) + netdev_flow_limit_table_len;

which comes out negative, and kzalloc_node(len, GFP_KERNEL, ...) sees
it as a huge size_t. Without __GFP_NOWARN, that reaches
WARN_ON_ONCE_GFP(order > MAX_PAGE_ORDER, gfp) in alloc_order_allowed().

The check also has no upper bound, and that matters beyond the
allocation size. struct sd_flow_limit records recent bucket indices in
u16 history[FLOW_LIMIT_HISTORY], but skb_flow_limit() computes

    new_flow = hash_32(skb_get_hash(skb), fl->log_buckets);

For a table length above 65536, log_buckets is above 16 and new_flow
is truncated when it is stored into fl->history[]. When that entry
ages out, fl->buckets[old_flow] is decremented for a different bucket
than the one that was incremented. Buckets at index 65536 and above
are incremented but never decremented, so flows that hash there end
up being limited no matter what their real rate is. Lengths from 2^17
through 2^21 still allocate successfully, so this happens without any
warning. Larger values hit the same allocator WARN as the INT_MIN
case.

None of this reads or writes out of bounds, and both the INT_MIN case
and the missing upper bound predate this patch. But since the patch is
tightening exactly this range check, could it be closed completely?
For example, proc_dointvec_minmax() with extra1 = 2 and
extra2 = 1 << 16, together with the existing is_power_of_2() check,
would compare as a signed int and match the u16 history[] limit. Or
is there a reason to keep accepting lengths above 65536?

[Severity: Medium]
Does this check reject every value below 2?

netdev_flow_limit_table_len is a signed int that proc_dointvec() parses.
flow_limit_table_len_sysctl() reads it through an unsigned pointer:

    unsigned int old, *ptr;

so *ptr < 2 is an unsigned comparison.

proc_int_u2k_conv_uop() accepts a write of "-2147483648":

    if (u > (ulong) INT_MAX + 1)
        return -EINVAL;
    WRITE_ONCE(*k_ptr, -u);

and this stores INT_MIN. As an unsigned value, 0x80000000 is not below 2,
and is_power_of_2(0x80000000) is true, so the write succeeds.

The check also has no upper bound, so powers of two up to 2^30 are
accepted.

Later, when a CPU is enabled through net.core.flow_limit_cpu_bitmap,
flow_limit_cpu_sysctl() does:

    len = sizeof(*cur) + netdev_flow_limit_table_len;
    ...
    cur = kzalloc_node(len, GFP_KERNEL, cpu_to_node(i));

For INT_MIN, len is negative and turns into a huge size_t. For values of
2^22 or more, the request needs an order above MAX_PAGE_ORDER. In both
cases the call goes through
kzalloc_node()->___kmalloc_large_node()->__alloc_frozen_pages_noprof()
and reaches alloc_order_allowed():

    return !(WARN_ON_ONCE_GFP(order > MAX_PAGE_ORDER, gfp));

This WARNs, because __GFP_NOWARN is not set. flow_limit_cpu_sysctl() then
returns -ENOMEM partway through its per-CPU loop. The CPUs it already
changed are not rolled back.

The allocation fails, so nothing is read or written out of bounds. The
INT_MIN case and the missing upper bound were also already there before
this patch. But the subject line says values below 2 are rejected, and
INT_MIN still gets through.

Would proc_dointvec_minmax() with extra1 = 2 and a reasonable extra2 cover
both cases? One example is 1 << 16, given the u16 history[] limit.
Comparing as a signed int would also fix the lower bound.

>  		*ptr = old;
>  		ret = -EINVAL;
>  	}

[Severity: Low]
This is a pre-existing issue and was not introduced by this patch, but
Documentation/networking/scaling.rst doesn't match the code. Its Flow Limit
section says the count is kept:

    count over the last 256 packets. If a flow exceeds a set ratio (by

but net/core/dev.h defines:

    #define FLOW_LIMIT_HISTORY	(1 << 7)  /* must be ^2 and !overflow buckets */

That is 128, and it is the window skb_flow_limit() actually uses.

The description of net.core.flow_limit_table_len in the same file also
says nothing about valid values. It mentions neither the power-of-two
requirement nor the new minimum of 2, and writes of any other value fail
with -EINVAL.

Could the documentation be updated along with this change?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261021155505.67208-1-phx0fer%40gmail.com