mirror of https://lore.kernel.org/lkml/
 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: 12+ 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-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  9:13 ` [PATCH net 3/5] ipv6: restrict mld_qrv to 255 Jiayuan Chen
2026-10-23 14:13   ` Fernando Fernandez Mancera
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-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

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®