All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joel Granados <joel.granados@kernel.org>
To: Jiayuan Chen <jiayuan.chen@linux.dev>
Cc: netdev@vger.kernel.org, stable@vger.kernel.org,
	 "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@kernel.org>,
	 Jakub Kicinski <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
	 David Ahern <dsahern@kernel.org>,
	Ido Schimmel <idosch@nvidia.com>,
	 Kuniyuki Iwashima <kuniyu@google.com>,
	Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>,
	 Wyatt Feng <bronzed_45_vested@icloud.com>,
	Yung Chih Su <yuuchihsu@gmail.com>,
	 Fernando Fernandez Mancera <fmancera@suse.de>,
	Hannes Frederic Sowa <hannes@stressinduktion.org>,
	 Flavio Leitner <fbl@redhat.com>, Petr Mladek <pmladek@suse.com>,
	 "Paul E. McKenney" <paulmck@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH net 5/5] net: restrict message_cost and message_burst to non-negative values
Date: Fri, 23 Oct 2026 14:19:53 +0200	[thread overview]
Message-ID: <attQGnJGhzTPFsMd@hobbes> (raw)
In-Reply-To: <20261023091419.167088-6-jiayuan.chen@linux.dev>

[-- Attachment #1: Type: text/plain, Size: 6798 bytes --]

On Fri, Oct 23, 2026 at 05:13:31PM +0800, Jiayuan Chen wrote:
> net.core.message_cost and net.core.message_burst feed the interval and
> burst of net_ratelimit_state, but neither has a lower bound:
> message_burst uses plain proc_dointvec, and message_cost uses
> proc_dointvec_jiffies, which happily stores -1 second as -HZ jiffies.
> 
> Since commit a940d145cc38 ("ratelimit: Warn if ->interval or ->burst are
> negative"), ___ratelimit() splats on such values, so the next
> net_ratelimit() caller warns, e.g. a martian source log:
> 
>   Negative interval (-250) or burst (10): Uninitialized ratelimit_state structure?
>   WARNING: lib/ratelimit.c:43 at ___ratelimit+0x56e/0x960, CPU#1: ping/3218
>   RIP: 0010:___ratelimit+0x578/0x960
>   Call Trace:
>    <IRQ>
>    net_ratelimit+0x1c/0x30
>    ip_handle_martian_source+0x1bb/0x340
>    ip_route_input_slow+0x1057/0x3650
>    ip_route_input_noref+0x104/0x260
>    arp_process+0x109b/0x2510
>    arp_rcv+0x381/0x460
>    __netif_receive_skb_one_core+0x173/0x1c0
>    __netif_receive_skb+0x21/0x150
>    process_backlog+0x37e/0x1200
>    __napi_poll.constprop.0+0xa5/0x5e0
>    net_rx_action+0x4c0/0xf70
>    handle_softirqs+0x204/0x9b0
>    __do_softirq+0x10/0x18
>    do_softirq.part.0+0x49/0xd0
>    </IRQ>
> 
> Negative values never meant anything here: ___ratelimit() treats them
> like 0, and 0 already expresses both "no limit" (interval) and
> "suppress everything" (burst).
> 
> Reject them at write time. message_burst can simply use
> proc_dointvec_minmax with extra1 = 0. proc_dointvec_jiffies ignores
> extra1/extra2, so give message_cost a small handler that parses into a
> temporary and only stores the value if it is not negative.
> 
> Fixes: a940d145cc38 ("ratelimit: Warn if ->interval or ->burst are negative")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
>  net/core/sysctl_net_core.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index 473c162736d6..8146ed819695 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -378,6 +378,22 @@ static int proc_do_skb_defer_max(const struct ctl_table *table, int write,
>  	return ret;
>  }
>  
> +static int proc_do_message_cost(const struct ctl_table *table, int write,
> +				void *buffer, size_t *lenp, loff_t *ppos)
> +{
> +	int ret, interval = READ_ONCE(*(int *)table->data);
> +	struct ctl_table tmp = *table;
> +
> +	tmp.data = &interval;
> +	ret = proc_dointvec_jiffies(&tmp, write, buffer, lenp, ppos);
> +	if (write && !ret) {
> +		if (interval < 0)
> +			return -EINVAL;
> +		WRITE_ONCE(*(int *)table->data, interval);
> +	}
> +	return ret;
> +}
> +
>  #ifdef CONFIG_BPF_JIT
>  static int proc_dointvec_minmax_bpf_enable(const struct ctl_table *table, int write,
>  					   void *buffer, size_t *lenp,
> @@ -544,14 +560,15 @@ static struct ctl_table net_core_table[] = {
>  		.data		= &net_ratelimit_state.interval,
>  		.maxlen		= sizeof(int),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec_jiffies,
> +		.proc_handler	= proc_do_message_cost,
>  	},
>  	{
>  		.procname	= "message_burst",
>  		.data		= &net_ratelimit_state.burst,
>  		.maxlen		= sizeof(int),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec,
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		= SYSCTL_ZERO,
>  	},
>  #ifdef CONFIG_RPS
>  	{
> -- 
> 2.43.0
> 

I would prefer something like this [1] (not tested and not compiled).
This is more in line with the patterns inside the jiffies.c file. I put
SYSCTL_ZERO there, but I have a doubt if you actually need SYSCTL_ONE.

If the interval variable can be zero, then leave it as SYSCTL_ZERO, but
if it cant be zero and can be one the use SYSCTL_ONE

best


[1] 

diff --git i/include/linux/jiffies.h w/include/linux/jiffies.h
index bbd57061802c..288ba81ed53e 100644
--- i/include/linux/jiffies.h
+++ w/include/linux/jiffies.h
@@ -646,6 +646,8 @@ extern unsigned long nsecs_to_jiffies(u64 n);
 struct ctl_table;
 int proc_dointvec_jiffies(const struct ctl_table *table, int dir, void *buffer,
                          size_t *lenp, loff_t *ppos);
+int proc_dointvec_jiffies_minmax(const struct ctl_table *table, int dir, void *buffer,
+                                size_t *lenp, loff_t *ppos);
 int proc_dointvec_ms_jiffies_minmax(const struct ctl_table *table, int dir,
                                    void *buffer, size_t *lenp, loff_t *ppos);
 int proc_dointvec_userhz_jiffies(const struct ctl_table *table, int dir,
diff --git i/kernel/time/jiffies.c w/kernel/time/jiffies.c
index 9b3487d40cd6..cf91996fe4e3 100644
--- i/kernel/time/jiffies.c
+++ w/kernel/time/jiffies.c
@@ -165,6 +165,14 @@ static int do_proc_int_conv_jiffies(bool *negp, ulong *u_ptr, int *k_ptr,
                             sysctl_u2k_int_conv_hz, sysctl_k2u_int_conv_hz);
 }

+static int do_proc_int_conv_jiffies_minmax(bool *negp, ulong *u_ptr, int *k_ptr,
+                                   int dir, const struct ctl_table *tbl)
+{
+       return proc_int_conv(negp, u_ptr, k_ptr, dir, tbl, true,
+                            sysctl_u2k_int_conv_hz, sysctl_k2u_int_conv_hz);
+}
+
+
 static int do_proc_int_conv_userhz_jiffies(bool *negp, ulong *u_ptr,
                                           int *k_ptr, int dir,
                                           const struct ctl_table *tbl)
@@ -263,6 +271,15 @@ int proc_dointvec_jiffies(const struct ctl_table *table, int dir,
 }
 EXPORT_SYMBOL(proc_dointvec_jiffies);

+int proc_dointvec_jiffies_minmax(const struct ctl_table *table, int dir,
+                         void *buffer, size_t *lenp, loff_t *ppos)
+{
+       return proc_dointvec_conv(table, dir, buffer, lenp, ppos,
+                                 do_proc_int_conv_jiffies_minmax);
+}
+EXPORT_SYMBOL(proc_dointvec_jiffies);
+
+
 /**
  * proc_dointvec_userhz_jiffies - read a vector of integers as 1/USER_HZ seconds
  * @table: the sysctl table
diff --git i/net/core/sysctl_net_core.c w/net/core/sysctl_net_core.c
index eb35da3556f4..7226697bf7c1 100644
--- i/net/core/sysctl_net_core.c
+++ w/net/core/sysctl_net_core.c
@@ -538,7 +538,8 @@ static struct ctl_table net_core_table[] = {
                .data           = &net_ratelimit_state.interval,
                .maxlen         = sizeof(int),
                .mode           = 0644,
-               .proc_handler   = proc_dointvec_jiffies,
+               .proc_handler   = proc_dointvec_jiffies_minmax,
+               .extra1         = SYSCTL_ZERO,
        },
        {
                .procname       = "message_burst",

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

  reply	other threads:[~2026-10-23 12:19 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-23  9:13 [PATCH net 0/5] net: fix misc signed overflows caused by bad sysctl values Jiayuan Chen
2026-10-23  9:13 ` [PATCH net 1/5] ipv6: restrict gc_elasticity to 31 Jiayuan Chen
2026-10-23 13:31   ` Fernando Fernandez Mancera
2026-10-26 12:15   ` netdev-bot+sashiko
2026-10-27  5:18   ` Kuniyuki Iwashima
2026-10-23  9:13 ` [PATCH net 2/5] ipv4: fix shift-out-of-bounds in ip_rt_send_redirect() Jiayuan Chen
2026-10-23 14:11   ` Fernando Fernandez Mancera
2026-10-23 14:38     ` Jiayuan Chen
2026-10-26 12:15   ` netdev-bot+sashiko
2026-10-23  9:13 ` [PATCH net 3/5] ipv6: restrict mld_qrv to 255 Jiayuan Chen
2026-10-23 14:13   ` Fernando Fernandez Mancera
2026-10-26 12:15   ` netdev-bot+sashiko
2026-10-27  5:24   ` Kuniyuki Iwashima
2026-10-23  9:13 ` [PATCH net 4/5] ipv4: restrict igmp_qrv " Jiayuan Chen
2026-10-23 14:14   ` Fernando Fernandez Mancera
2026-10-26 12:15   ` netdev-bot+sashiko
2026-10-27  5:25   ` Kuniyuki Iwashima
2026-10-23  9:13 ` [PATCH net 5/5] net: restrict message_cost and message_burst to non-negative values Jiayuan Chen
2026-10-23 12:19   ` Joel Granados [this message]
2026-10-23 12:55     ` Fernando Fernandez Mancera
2026-10-23 14:30       ` Jiayuan Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=attQGnJGhzTPFsMd@hobbes \
    --to=joel.granados@kernel.org \
    --cc=bronzed_45_vested@icloud.com \
    --cc=chia-yu.chang@nokia-bell-labs.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@kernel.org \
    --cc=fbl@redhat.com \
    --cc=fmancera@suse.de \
    --cc=hannes@stressinduktion.org \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=jiayuan.chen@linux.dev \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=paulmck@kernel.org \
    --cc=pmladek@suse.com \
    --cc=stable@vger.kernel.org \
    --cc=yuuchihsu@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.