linux-kernel - Re: [PATCH v3 15/20] ratelimit: Warn if ->interval or ->burst are negative

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1cc77290-ec8d-4c51-85c6-4f0bbcaaa8b4@paulmck-laptop>
Date: Tue, 29 Apr 2025 11:52:04 -0700
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Petr Mladek <pmladek@...e.com>
Cc: linux-kernel@...r.kernel.org, kernel-team@...a.com,
	Andrew Morton <akpm@...ux-foundation.org>,
	Kuniyuki Iwashima <kuniyu@...zon.com>,
	Mateusz Guzik <mjguzik@...il.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	John Ogness <john.ogness@...utronix.de>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Jon Pan-Doh <pandoh@...gle.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Karolina Stolarek <karolina.stolarek@...cle.com>
Subject: Re: [PATCH v3 15/20] ratelimit: Warn if ->interval or ->burst are
 negative

On Tue, Apr 29, 2025 at 02:23:17PM +0200, Petr Mladek wrote:
> On Thu 2025-04-24 17:28:21, Paul E. McKenney wrote:
> > From: Petr Mladek <pmladek@...e.com>
> > 
> > Currently, ___ratelimit() treats a negative ->interval or ->burst as
> > if it was zero, but this is an accident of the current implementation.
> > Therefore, splat in this case, which might have the benefit of detecting
> > use of uninitialized ratelimit_state structures on the one hand or easing
> > addition of new features on the other.
> > 
> > Link: https://lore.kernel.org/all/fbe93a52-365e-47fe-93a4-44a44547d601@paulmck-laptop/
> > Link: https://lore.kernel.org/all/20250423115409.3425-1-spasswolf@web.de/
> > Signed-off-by: Petr Mladek <pmladek@...e.com>
> > Signed-off-by: Paul E. McKenney <paulmck@...nel.org>
> > Cc: Andrew Morton <akpm@...ux-foundation.org>
> > Cc: Kuniyuki Iwashima <kuniyu@...zon.com>
> > Cc: Mateusz Guzik <mjguzik@...il.com>
> > Cc: Steven Rostedt <rostedt@...dmis.org>
> > Cc: John Ogness <john.ogness@...utronix.de>
> > Cc: Sergey Senozhatsky <senozhatsky@...omium.org>
> > ---
> >  lib/ratelimit.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/lib/ratelimit.c b/lib/ratelimit.c
> > index 4f5d8fb6919f7..63efb1191d71a 100644
> > --- a/lib/ratelimit.c
> > +++ b/lib/ratelimit.c
> > @@ -40,6 +40,7 @@ int ___ratelimit(struct ratelimit_state *rs, const char *func)
> >  	 * says always limit.
> >  	 */
> >  	if (interval <= 0 || burst <= 0) {
> > +		WARN_ONCE(interval < 0 || burst < 0, "Negative interval (%d) or burst (%d): Uninitialized ratelimit_state structure?\n", interval, burst);
> 
> Just for record, I agree with having this patch in this form
> in this series.
> 
> That said, I think that we should warn even about using zero
> initialized structure in the long term because of a possible use of
> to-be-initialized spin lock. But it would require fixing
> existing users and it is beyond scope of this patchset.
> It is related to the discussion at
> https://lore.kernel.org/r/aA-f0jpBBbdfsmn7@pathway.suse.cz .

Agreed, it would be good to get to a more principled destination.

In contrast, this series is what I can come up with that deals with the
current ratelimit clients without too much violence to the current code
base, but we can clearly do better longer term.

And thank you for the Reviewed-by on the other patches!  I will apply
these later today, Pacific Time.

							Thanx, Paul