lkml.org 
[lkml]   [2017]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] locking/refcounts: Change WARN() to WARN_ONCE()

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Mon, Feb 27, 2017 at 11:57 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > Note that the uninlining allowed us to enable the underflow/overflow warnings
> > unconditionally and remove the debug Kconfig switch: this might trigger new
> > warnings in buggy code and turn crashes/use-after-free bugs into less harmful
> > memory leaks.
>
> I'm ok with this, but that WARN() really needs to be a WARN_ON_ONCE().
>
> Because once an underflow (or overflow) is happening, it tends to
> _keep_ happening. And you may just have essentially DoS'ed the machine
> that is now spending all its time writing those logs to disk.
>
> Yes, yes, quiet independently of this we should limit WARN printouts
> (and do the reverse: turn a "once" to mean "once in a blue moon"
> rather than actually just once), but particularly for this kind of
> "never happens" thing, it really is better to just warn once.

Good point - I've done this in the attached patch.

Thanks,

Ingo

=================>
From 9dcfe2c75b51f454f39c2de4756e841228865b47 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@kernel.org>
Date: Wed, 1 Mar 2017 09:25:55 +0100
Subject: [PATCH] locking/refcounts: Change WARN() to WARN_ONCE()

Linus noticed that the new refcount.h APIs used WARN(), which would turn
into a dmesg DoS if it triggers frequently on some buggy driver.

So make sure we only warn once. These warnings are never supposed to happen,
so it's typically not a problem to lose subsequent warnings.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Elena Reshetova <elena.reshetova@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/CA+55aFzbYUTZ=oqZ2YgDjY0C2_n6ODhTfqj6V+m5xWmDxsuB0w@mail.gmail.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
lib/refcount.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/refcount.c b/lib/refcount.c
index 1d33366189d1..aa09ad3c30b0 100644
--- a/lib/refcount.c
+++ b/lib/refcount.c
@@ -58,7 +58,7 @@ bool refcount_add_not_zero(unsigned int i, refcount_t *r)
val = old;
}

- WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
+ WARN_ONCE(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");

return true;
}
@@ -66,7 +66,7 @@ EXPORT_SYMBOL_GPL(refcount_add_not_zero);

void refcount_add(unsigned int i, refcount_t *r)
{
- WARN(!refcount_add_not_zero(i, r), "refcount_t: addition on 0; use-after-free.\n");
+ WARN_ONCE(!refcount_add_not_zero(i, r), "refcount_t: addition on 0; use-after-free.\n");
}
EXPORT_SYMBOL_GPL(refcount_add);

@@ -97,7 +97,7 @@ bool refcount_inc_not_zero(refcount_t *r)
val = old;
}

- WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
+ WARN_ONCE(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");

return true;
}
@@ -111,7 +111,7 @@ EXPORT_SYMBOL_GPL(refcount_inc_not_zero);
*/
void refcount_inc(refcount_t *r)
{
- WARN(!refcount_inc_not_zero(r), "refcount_t: increment on 0; use-after-free.\n");
+ WARN_ONCE(!refcount_inc_not_zero(r), "refcount_t: increment on 0; use-after-free.\n");
}
EXPORT_SYMBOL_GPL(refcount_inc);

@@ -125,7 +125,7 @@ bool refcount_sub_and_test(unsigned int i, refcount_t *r)

new = val - i;
if (new > val) {
- WARN(new > val, "refcount_t: underflow; use-after-free.\n");
+ WARN_ONCE(new > val, "refcount_t: underflow; use-after-free.\n");
return false;
}

@@ -164,7 +164,7 @@ EXPORT_SYMBOL_GPL(refcount_dec_and_test);

void refcount_dec(refcount_t *r)
{
- WARN(refcount_dec_and_test(r), "refcount_t: decrement hit 0; leaking memory.\n");
+ WARN_ONCE(refcount_dec_and_test(r), "refcount_t: decrement hit 0; leaking memory.\n");
}
EXPORT_SYMBOL_GPL(refcount_dec);

@@ -204,7 +204,7 @@ bool refcount_dec_not_one(refcount_t *r)

new = val - 1;
if (new > val) {
- WARN(new > val, "refcount_t: underflow; use-after-free.\n");
+ WARN_ONCE(new > val, "refcount_t: underflow; use-after-free.\n");
return true;
}

\
 
 \ /
  Last update: 2017-03-01 09:40    [W:0.132 / U:0.324 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site