mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: kernel-hardening@lists.openwall.com
Cc: Kees Cook <keescook@chromium.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Rik van Riel <riel@redhat.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Jakub Kicinski <jakub.kicinski@netronome.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Dmitry Vyukov <dvyukov@google.com>,
	Olof Johansson <olof@lixom.net>,
	Peter Zijlstra <peterz@infradead.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 6/6] refcount: Check bad states with CHECK_DATA_CORRUPTION
Date: Mon,  6 Mar 2017 11:09:46 -0800	[thread overview]
Message-ID: <1488827386-87193-7-git-send-email-keescook@chromium.org> (raw)
In-Reply-To: <1488827386-87193-1-git-send-email-keescook@chromium.org>

This converts from WARN() to CHECK_DATA_CORRUPTION() (so that system
builders can choose between WARN and BUG). Additionally moves refcount_t
sanity-check conditionals into regular function flow.

Now when built with CONFIG_BUG_ON_DATA_CORRUPTION, the LKDTM REFCOUNT_*
tests correctly kill offending processes.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 lib/refcount.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/lib/refcount.c b/lib/refcount.c
index 1d33366189d1..54aff1e0582f 100644
--- a/lib/refcount.c
+++ b/lib/refcount.c
@@ -37,6 +37,13 @@
 #include <linux/refcount.h>
 #include <linux/bug.h>
 
+/*
+ * CHECK_DATA_CORRUPTION() is defined with __must_check, but we have a
+ * couple places where we want to report a condition that has already
+ * been checked, so this lets us cheat __must_check.
+ */
+#define REFCOUNT_CHECK(cond, str) unlikely(CHECK_DATA_CORRUPTION(cond, str))
+
 bool refcount_add_not_zero(unsigned int i, refcount_t *r)
 {
 	unsigned int old, new, val = atomic_read(&r->refs);
@@ -58,7 +65,8 @@ bool refcount_add_not_zero(unsigned int i, refcount_t *r)
 		val = old;
 	}
 
-	WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
+	REFCOUNT_CHECK(new == UINT_MAX,
+			"refcount_t: add saturated; leaking memory.\n");
 
 	return true;
 }
@@ -66,7 +74,8 @@ 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");
+	REFCOUNT_CHECK(!refcount_add_not_zero(i, r),
+			"refcount_t: addition on 0; use-after-free.\n");
 }
 EXPORT_SYMBOL_GPL(refcount_add);
 
@@ -97,7 +106,8 @@ bool refcount_inc_not_zero(refcount_t *r)
 		val = old;
 	}
 
-	WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
+	REFCOUNT_CHECK(new == UINT_MAX,
+			"refcount_t: inc saturated; leaking memory.\n");
 
 	return true;
 }
@@ -111,7 +121,8 @@ 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");
+	REFCOUNT_CHECK(!refcount_inc_not_zero(r),
+			"refcount_t: increment on 0; use-after-free.\n");
 }
 EXPORT_SYMBOL_GPL(refcount_inc);
 
@@ -124,10 +135,9 @@ bool refcount_sub_and_test(unsigned int i, refcount_t *r)
 			return false;
 
 		new = val - i;
-		if (new > val) {
-			WARN(new > val, "refcount_t: underflow; use-after-free.\n");
+		if (REFCOUNT_CHECK(new > val,
+				"refcount_t: sub underflow; use-after-free.\n"))
 			return false;
-		}
 
 		old = atomic_cmpxchg_release(&r->refs, val, new);
 		if (old == val)
@@ -164,7 +174,8 @@ 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");
+	REFCOUNT_CHECK(refcount_dec_and_test(r),
+			"refcount_t: decrement hit 0; leaking memory.\n");
 }
 EXPORT_SYMBOL_GPL(refcount_dec);
 
@@ -203,10 +214,9 @@ bool refcount_dec_not_one(refcount_t *r)
 			return false;
 
 		new = val - 1;
-		if (new > val) {
-			WARN(new > val, "refcount_t: underflow; use-after-free.\n");
+		if (REFCOUNT_CHECK(new > val,
+				"refcount_t: dec underflow; use-after-free.\n"))
 			return true;
-		}
 
 		old = atomic_cmpxchg_release(&r->refs, val, new);
 		if (old == val)
@@ -264,4 +274,3 @@ bool refcount_dec_and_lock(refcount_t *r, spinlock_t *lock)
 	return true;
 }
 EXPORT_SYMBOL_GPL(refcount_dec_and_lock);
-
-- 
2.7.4

  parent reply	other threads:[~2017-03-06 19:10 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-06 19:09 [PATCH 0/6] bug: further enhance use of BUG_ON_DATA_CORRUPTION Kees Cook
2017-03-06 19:09 ` [PATCH 1/6] bug: Clarify help text for BUG_ON_DATA_CORRUPTION Kees Cook
2017-03-06 19:09 ` [PATCH 2/6] bug: Improve unlikely() in data corruption check Kees Cook
2017-03-06 19:09 ` [PATCH 3/6] bug: Enable DEBUG_CREDENTIALS under BUG_ON_DATA_CORRUPTION Kees Cook
2017-03-06 19:09 ` [PATCH 4/6] bug: Enable DEBUG_SG " Kees Cook
2017-03-06 19:09 ` [PATCH 5/6] notifiers: Use CHECK_DATA_CORRUPTION() on checks Kees Cook
2017-03-22 19:29   ` Kees Cook
2017-03-22 19:32     ` Arjan van de Ven
2017-03-22 19:55       ` Kees Cook
2017-03-06 19:09 ` Kees Cook [this message]
2017-03-22 19:30   ` [PATCH 6/6] refcount: Check bad states with CHECK_DATA_CORRUPTION Kees Cook

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=1488827386-87193-7-git-send-email-keescook@chromium.org \
    --to=keescook@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=dvyukov@google.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=jpoimboe@redhat.com \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=olof@lixom.net \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=viresh.kumar@linaro.org \
    /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®