[RFC PATCH 15/21] list_bl: Use CHECK_DATA_CORRUPTION instead of custom BUG_ON() wrapper

From: Will Deacon
Date: Tue Mar 24 2020 - 11:38:51 EST


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@xxxxxxxxxxxx>
Cc: Paul E. McKenney <paulmck@xxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Signed-off-by: Will Deacon <will@xxxxxxxxxx>
---
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