All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: linux-kernel@vger.kernel.org
Cc: Will Deacon <will@kernel.org>, Eric Dumazet <edumazet@google.com>,
	Jann Horn <jannh@google.com>, Kees Cook <keescook@chromium.org>,
	Maddie Stone <maddiestone@google.com>,
	Marco Elver <elver@google.com>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	kernel-team@android.com, kernel-hardening@lists.openwall.com
Subject: [RFC PATCH 15/21] list_bl: Use CHECK_DATA_CORRUPTION instead of custom BUG_ON() wrapper
Date: Tue, 24 Mar 2020 15:36:37 +0000	[thread overview]
Message-ID: <20200324153643.15527-16-will@kernel.org> (raw)
In-Reply-To: <20200324153643.15527-1-will@kernel.org>

CHECK_DATA_CORRUPTION() allows detected data corruption to result
consistently in either a BUG() or a WARN() depending on
CONFIG_BUG_ON_DATA_CORRUPTION.

Use CHECK_DATA_CORRUPTION() to report list_bl integrity checking failures,
rather than a custom wrapper around BUG_ON().

Cc: Kees Cook <keescook@chromium.org>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Will Deacon <will@kernel.org>
---
 include/linux/list_bl.h    | 55 +++++++++++++++++++++++++-------------
 include/linux/rculist_bl.h | 17 ++++--------
 2 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/include/linux/list_bl.h b/include/linux/list_bl.h
index 9f8e29142324..f48d8acb15b4 100644
--- a/include/linux/list_bl.h
+++ b/include/linux/list_bl.h
@@ -24,13 +24,6 @@
 #define LIST_BL_LOCKMASK	0UL
 #endif
 
-#ifdef CONFIG_CHECK_INTEGRITY_LIST
-#define LIST_BL_BUG_ON(x) BUG_ON(x)
-#else
-#define LIST_BL_BUG_ON(x)
-#endif
-
-
 struct hlist_bl_head {
 	struct hlist_bl_node *first;
 };
@@ -38,6 +31,37 @@ struct hlist_bl_head {
 struct hlist_bl_node {
 	struct hlist_bl_node *next, **pprev;
 };
+
+#ifdef CONFIG_CHECK_INTEGRITY_LIST
+static inline bool __hlist_bl_add_head_valid(struct hlist_bl_head *h,
+					     struct hlist_bl_node *n)
+{
+	unsigned long hlock = (unsigned long)h->first & LIST_BL_LOCKMASK;
+	unsigned long nlock = (unsigned long)n & LIST_BL_LOCKMASK;
+
+	return !(CHECK_DATA_CORRUPTION(nlock,
+			"hlist_bl_add_head: node is locked\n") ||
+		 CHECK_DATA_CORRUPTION(hlock != LIST_BL_LOCKMASK,
+			"hlist_bl_add_head: head is unlocked\n"));
+}
+
+static inline bool __hlist_bl_del_valid(struct hlist_bl_node *n)
+{
+	unsigned long nlock = (unsigned long)n & LIST_BL_LOCKMASK;
+	return !CHECK_DATA_CORRUPTION(nlock, "hlist_bl_del_valid: node locked");
+}
+#else
+static inline bool __hlist_bl_add_head_valid(struct hlist_bl_head *h,
+					     struct hlist_bl_node *n)
+{
+	return true;
+}
+static inline bool __hlist_bl_del_valid(struct hlist_bl_node *n)
+{
+	return true;
+}
+#endif
+
 #define INIT_HLIST_BL_HEAD(ptr) \
 	((ptr)->first = NULL)
 
@@ -60,15 +84,6 @@ static inline struct hlist_bl_node *hlist_bl_first(struct hlist_bl_head *h)
 		((unsigned long)h->first & ~LIST_BL_LOCKMASK);
 }
 
-static inline void hlist_bl_set_first(struct hlist_bl_head *h,
-					struct hlist_bl_node *n)
-{
-	LIST_BL_BUG_ON((unsigned long)n & LIST_BL_LOCKMASK);
-	LIST_BL_BUG_ON(((unsigned long)h->first & LIST_BL_LOCKMASK) !=
-							LIST_BL_LOCKMASK);
-	h->first = (struct hlist_bl_node *)((unsigned long)n | LIST_BL_LOCKMASK);
-}
-
 static inline bool hlist_bl_empty(const struct hlist_bl_head *h)
 {
 	unsigned long first = data_race((unsigned long)READ_ONCE(h->first));
@@ -80,11 +95,14 @@ static inline void hlist_bl_add_head(struct hlist_bl_node *n,
 {
 	struct hlist_bl_node *first = hlist_bl_first(h);
 
+	if (!__hlist_bl_add_head_valid(h, n))
+		return;
+
 	n->next = first;
 	if (first)
 		first->pprev = &n->next;
 	n->pprev = &h->first;
-	hlist_bl_set_first(h, n);
+	h->first = (struct hlist_bl_node *)((unsigned long)n | LIST_BL_LOCKMASK);
 }
 
 static inline void hlist_bl_add_before(struct hlist_bl_node *n,
@@ -118,7 +136,8 @@ static inline void __hlist_bl_del(struct hlist_bl_node *n)
 	struct hlist_bl_node *next = n->next;
 	struct hlist_bl_node **pprev = n->pprev;
 
-	LIST_BL_BUG_ON((unsigned long)n & LIST_BL_LOCKMASK);
+	if (!__hlist_bl_del_valid(n))
+		return;
 
 	/* pprev may be `first`, so be careful not to lose the lock bit */
 	WRITE_ONCE(*pprev,
diff --git a/include/linux/rculist_bl.h b/include/linux/rculist_bl.h
index 0b952d06eb0b..553ce3cde104 100644
--- a/include/linux/rculist_bl.h
+++ b/include/linux/rculist_bl.h
@@ -8,16 +8,6 @@
 #include <linux/list_bl.h>
 #include <linux/rcupdate.h>
 
-static inline void hlist_bl_set_first_rcu(struct hlist_bl_head *h,
-					struct hlist_bl_node *n)
-{
-	LIST_BL_BUG_ON((unsigned long)n & LIST_BL_LOCKMASK);
-	LIST_BL_BUG_ON(((unsigned long)h->first & LIST_BL_LOCKMASK) !=
-							LIST_BL_LOCKMASK);
-	rcu_assign_pointer(h->first,
-		(struct hlist_bl_node *)((unsigned long)n | LIST_BL_LOCKMASK));
-}
-
 static inline struct hlist_bl_node *hlist_bl_first_rcu(struct hlist_bl_head *h)
 {
 	return (struct hlist_bl_node *)
@@ -73,6 +63,9 @@ static inline void hlist_bl_add_head_rcu(struct hlist_bl_node *n,
 {
 	struct hlist_bl_node *first;
 
+	if (!__hlist_bl_add_head_valid(h, n))
+		return;
+
 	/* don't need hlist_bl_first_rcu because we're under lock */
 	first = hlist_bl_first(h);
 
@@ -81,8 +74,8 @@ static inline void hlist_bl_add_head_rcu(struct hlist_bl_node *n,
 		first->pprev = &n->next;
 	n->pprev = &h->first;
 
-	/* need _rcu because we can have concurrent lock free readers */
-	hlist_bl_set_first_rcu(h, n);
+	rcu_assign_pointer(h->first,
+		(struct hlist_bl_node *)((unsigned long)n | LIST_BL_LOCKMASK));
 }
 /**
  * hlist_bl_for_each_entry_rcu - iterate over rcu list of given type
-- 
2.20.1


  parent reply	other threads:[~2020-03-24 15:41 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-24 15:36 [RFC PATCH 00/21] Improve list integrity checking Will Deacon
2020-03-24 15:36 ` [RFC PATCH 01/21] list: Remove hlist_unhashed_lockless() Will Deacon
2020-03-24 16:27   ` Greg KH
2020-03-30 23:05   ` Paul E. McKenney
2020-03-24 15:36 ` [RFC PATCH 02/21] list: Remove hlist_nulls_unhashed_lockless() Will Deacon
2020-03-24 16:27   ` Greg KH
2020-03-30 23:07   ` Paul E. McKenney
2020-03-24 15:36 ` [RFC PATCH 03/21] list: Annotate lockless list primitives with data_race() Will Deacon
2020-03-24 16:20   ` Jann Horn
2020-03-24 16:26     ` Greg KH
2020-03-24 16:38       ` Jann Horn
2020-03-24 16:59         ` Greg KH
2020-03-24 18:22           ` Jann Horn
2020-03-24 16:23   ` Marco Elver
2020-03-24 21:33     ` Will Deacon
2020-03-31 13:10     ` Will Deacon
2020-04-01  6:34       ` Marco Elver
2020-04-01  8:40         ` Will Deacon
2020-05-08 13:46       ` [tip: locking/kcsan] kcsan: Change data_race() to no longer require marking racing accesses tip-bot2 for Marco Elver
2020-03-24 16:51   ` [RFC PATCH 03/21] list: Annotate lockless list primitives with data_race() Peter Zijlstra
2020-03-24 16:56     ` Jann Horn
2020-03-24 21:32       ` Will Deacon
2020-03-30 23:13         ` Paul E. McKenney
2020-04-24 17:39           ` Will Deacon
2020-04-27 19:24             ` Paul E. McKenney
2020-03-24 15:36 ` [RFC PATCH 04/21] timers: Use hlist_unhashed() instead of open-coding in timer_pending() Will Deacon
2020-03-24 16:30   ` Greg KH
2020-03-24 15:36 ` [RFC PATCH 05/21] list: Comment missing WRITE_ONCE() in __list_del() Will Deacon
2020-03-30 23:14   ` Paul E. McKenney
2020-03-24 15:36 ` [RFC PATCH 06/21] list: Remove superfluous WRITE_ONCE() from hlist_nulls implementation Will Deacon
2020-03-30 23:21   ` Paul E. McKenney
2020-03-24 15:36 ` [RFC PATCH 07/21] Revert "list: Use WRITE_ONCE() when adding to lists and hlists" Will Deacon
2020-03-30 23:19   ` Paul E. McKenney
2020-03-24 15:36 ` [RFC PATCH 08/21] Revert "list: Use WRITE_ONCE() when initializing list_head structures" Will Deacon
2020-03-30 23:25   ` Paul E. McKenney
2020-03-31 13:11     ` Will Deacon
2020-03-31 13:47       ` Paul E. McKenney
2020-03-24 15:36 ` [RFC PATCH 09/21] list: Remove unnecessary WRITE_ONCE() from hlist_bl_add_before() Will Deacon
2020-03-30 23:30   ` Paul E. McKenney
2020-03-31 12:37     ` Will Deacon
2020-03-24 15:36 ` [RFC PATCH 10/21] kernel-hacking: Make DEBUG_{LIST,PLIST,SG,NOTIFIERS} non-debug options Will Deacon
2020-03-24 16:42   ` Greg KH
2020-03-24 15:36 ` [RFC PATCH 11/21] list: Add integrity checking to hlist implementation Will Deacon
2020-03-24 15:36 ` [RFC PATCH 12/21] list: Poison ->next pointer for non-RCU deletion of 'hlist_nulls_node' Will Deacon
2020-03-30 23:32   ` Paul E. McKenney
2020-03-24 15:36 ` [RFC PATCH 13/21] list: Add integrity checking to hlist_nulls implementation Will Deacon
2020-03-24 15:36 ` [RFC PATCH 14/21] plist: Use CHECK_DATA_CORRUPTION instead of explicit {BUG,WARN}_ON() Will Deacon
2020-03-24 16:42   ` Greg KH
2020-03-24 15:36 ` Will Deacon [this message]
2020-03-24 15:36 ` [RFC PATCH 16/21] list_bl: Extend integrity checking in deletion routines Will Deacon
2020-03-24 15:36 ` [RFC PATCH 17/21] linux/bit_spinlock.h: Include linux/processor.h Will Deacon
2020-03-24 16:28   ` Greg KH
2020-03-24 21:08     ` Will Deacon
2020-03-24 15:36 ` [RFC PATCH 18/21] list_bl: Move integrity checking out of line Will Deacon
2020-03-24 15:36 ` [RFC PATCH 19/21] list_bl: Extend integrity checking to cover the same cases as 'hlist' Will Deacon
2020-03-24 15:36 ` [RFC PATCH 20/21] list: Format CHECK_DATA_CORRUPTION error messages consistently Will Deacon
2020-03-24 16:40   ` Greg KH
2020-03-24 15:36 ` [RFC PATCH 21/21] lkdtm: Extend list corruption checks Will Deacon

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=20200324153643.15527-16-will@kernel.org \
    --to=will@kernel.org \
    --cc=edumazet@google.com \
    --cc=elver@google.com \
    --cc=jannh@google.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maddiestone@google.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.