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

[Jiayuan Chen <jiayuan.chen@linux.dev>]

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.