mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Kees Cook <keescook@chromium.org>,
	elena.reshetova@intel.com, gregkh@linuxfoundation.org,
	arnd@arndb.de, tglx@linutronix.de, mingo@kernel.org,
	h.peter.anvin@intel.com, will.deacon@arm.com, dwindsor@gmail.com,
	Hans Liljestrand <ishkamiel@gmail.com>,
	dhowells@redhat.com, linux-kernel@vger.kernel.org,
	kernel-hardening@lists.openwall.com
Subject: [PATCH 4/4] refcount: Report failures through CHECK_DATA_CORRUPTION
Date: Fri,  3 Feb 2017 15:26:52 -0800	[thread overview]
Message-ID: <1486164412-7338-5-git-send-email-keescook@chromium.org> (raw)
In-Reply-To: <1486164412-7338-1-git-send-email-keescook@chromium.org>

This converts from WARN_ON() to CHECK_DATA_CORRUPTION() in the
CONFIG_DEBUG_REFCOUNT case. Additionally moves refcount_t sanity check
conditionals into regular function flow. Since CHECK_DATA_CORRUPTION()
is marked __much_check, we override few cases where the failure has
already been handled but we want to explicitly report it.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/refcount.h | 35 ++++++++++++++++++++++-------------
 lib/Kconfig.debug        |  2 ++
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/include/linux/refcount.h b/include/linux/refcount.h
index 5b89cad62237..ef32910c7dd8 100644
--- a/include/linux/refcount.h
+++ b/include/linux/refcount.h
@@ -43,10 +43,10 @@
 #include <linux/spinlock.h>
 
 #if CONFIG_DEBUG_REFCOUNT
-#define REFCOUNT_WARN(cond, str) WARN_ON(cond)
+#define REFCOUNT_CHECK(cond, str) CHECK_DATA_CORRUPTION(cond, str)
 #define __refcount_check	__must_check
 #else
-#define REFCOUNT_WARN(cond, str) (void)(cond)
+#define REFCOUNT_CHECK(cond, str) (!!(cond))
 #define __refcount_check
 #endif
 
@@ -86,14 +86,18 @@ bool refcount_add_not_zero(unsigned int i, refcount_t *r)
 			break;
 	}
 
-	REFCOUNT_WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
+	val = REFCOUNT_CHECK(new == UINT_MAX,
+			     "refcount_t: add saturated; leaking memory.\n");
 
 	return true;
 }
 
 static inline void refcount_add(unsigned int i, refcount_t *r)
 {
-	REFCOUNT_WARN(!refcount_add_not_zero(i, r), "refcount_t: addition on 0; use-after-free.\n");
+	bool __always_unused b;
+
+	b = REFCOUNT_CHECK(!refcount_add_not_zero(i, r),
+			   "refcount_t: addition on 0; use-after-free.\n");
 }
 
 /*
@@ -121,7 +125,8 @@ bool refcount_inc_not_zero(refcount_t *r)
 			break;
 	}
 
-	REFCOUNT_WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
+	val = REFCOUNT_CHECK(new == UINT_MAX,
+			     "refcount_t: inc saturated; leaking memory.\n");
 
 	return true;
 }
@@ -134,7 +139,10 @@ bool refcount_inc_not_zero(refcount_t *r)
  */
 static inline void refcount_inc(refcount_t *r)
 {
-	REFCOUNT_WARN(!refcount_inc_not_zero(r), "refcount_t: increment on 0; use-after-free.\n");
+	bool __always_unused b;
+
+	b = REFCOUNT_CHECK(!refcount_inc_not_zero(r),
+			   "refcount_t: increment on 0; use-after-free.\n");
 }
 
 /*
@@ -155,10 +163,9 @@ bool refcount_sub_and_test(unsigned int i, refcount_t *r)
 			return false;
 
 		new = val - i;
-		if (new > val) {
-			REFCOUNT_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;
-		}
 
 		if (atomic_try_cmpxchg_release(&r->refs, &val, new))
 			break;
@@ -183,7 +190,10 @@ bool refcount_dec_and_test(refcount_t *r)
 static inline
 void refcount_dec(refcount_t *r)
 {
-	REFCOUNT_WARN(refcount_dec_and_test(r), "refcount_t: decrement hit 0; leaking memory.\n");
+	bool __always_unused b;
+
+	b = REFCOUNT_CHECK(refcount_dec_and_test(r),
+			   "refcount_t: decrement hit 0; leaking memory.\n");
 }
 
 /*
@@ -224,10 +234,9 @@ bool refcount_dec_not_one(refcount_t *r)
 			return false;
 
 		new = val - 1;
-		if (new > val) {
-			REFCOUNT_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;
-		}
 
 		if (atomic_try_cmpxchg_release(&r->refs, &val, new))
 			break;
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 20fde8d4523a..01e7aa578456 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -731,6 +731,7 @@ source "lib/Kconfig.kasan"
 
 config DEBUG_REFCOUNT
 	bool "Verbose refcount checks"
+	depends on DEBUG_KERNEL || BUG_ON_DATA_CORRUPTION
 	help
 	  Say Y here if you want reference counters (refcount_t and kref) to
 	  generate WARNs on dubious usage. Without this refcount_t will still
@@ -2011,6 +2012,7 @@ config TEST_STATIC_KEYS
 config BUG_ON_DATA_CORRUPTION
 	bool "Trigger a BUG when data corruption is detected"
 	select DEBUG_LIST
+	select DEBUG_REFCOUNT
 	help
 	  Select this option if the kernel should BUG when it encounters
 	  data corruption in kernel memory structures when they get checked
-- 
2.7.4

  parent reply	other threads:[~2017-02-03 23:27 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-03 23:26 [PATCH 0/4] refcount_t followups Kees Cook
2017-02-03 23:26 ` [PATCH 1/4] refcount_t: fix Kconfig help Kees Cook
2017-02-03 23:26 ` [PATCH 2/4] lkdtm: convert to refcount_t testing Kees Cook
2017-02-10  8:32   ` [tip:locking/core] lkdtm: Convert " tip-bot for Kees Cook
2017-02-03 23:26 ` [PATCH 3/4] bug: Switch data corruption check to __must_check Kees Cook
2017-02-03 23:26 ` Kees Cook [this message]
2017-02-05 15:40   ` [PATCH 4/4] refcount: Report failures through CHECK_DATA_CORRUPTION Peter Zijlstra
2017-02-05 23:33     ` Kees Cook
2017-02-06  8:57       ` Peter Zijlstra
2017-02-06 16:54         ` Kees Cook
2017-02-07  8:34           ` Peter Zijlstra
2017-02-07 11:10             ` [kernel-hardening] " Mark Rutland
2017-02-07 12:36               ` Peter Zijlstra
2017-02-07 13:50                 ` Mark Rutland
2017-02-07 15:07                   ` Peter Zijlstra
2017-02-07 16:03                     ` Mark Rutland
2017-02-07 17:30                       ` Peter Zijlstra
2017-02-07 17:55                         ` Mark Rutland
2017-02-08  9:12                           ` Peter Zijlstra
2017-02-08  9:43                             ` Peter Zijlstra
2017-02-08 14:10                             ` Mark Rutland
2017-02-08 21:20                             ` Kees Cook
2017-02-09 10:27                               ` Peter Zijlstra
2017-02-10 23:39                                 ` 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=1486164412-7338-5-git-send-email-keescook@chromium.org \
    --to=keescook@chromium.org \
    --cc=arnd@arndb.de \
    --cc=dhowells@redhat.com \
    --cc=dwindsor@gmail.com \
    --cc=elena.reshetova@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=h.peter.anvin@intel.com \
    --cc=ishkamiel@gmail.com \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=will.deacon@arm.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®