RCU list updates for 4.9
Subsystems (3)
cover RCU list updates for 4.9
Hello!
This series provides updates to RCU-protected linked lists:
1. Expand list_first_entry_or_null() to allow better compiler
optimizations, courtesy of Chris Wilson.
2-6. Allow security-conscious builds to specify BUG rather than WARN
for linked-list corruption, courtesy of Kees Cook.
7. Fix a bogus CONFIG_-prefixed Kconfig-option reference in #5
above, courtesy of Valentin Rothberg.
Thanx, Paul
PATCH 1/7 list: Expand list_first_entry_or_null()
From: Chris Wilson <[email protected]>
Due to the use of READ_ONCE() in list_empty() the compiler cannot optimise !list_empty() ? list_first_entry() : NULL very well. By manually expanding list_first_entry_or_null() we can take advantage of the READ_ONCE() to avoid the list element changing under the test while the compiler can generate smaller code.
--- include/linux/list.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
include/linux/list.h +5 -2
@@ -381,8 +381,11 @@ static inline void list_splice_tail_init(struct list_head *list,* * Note that if the list is empty, it returns NULL. */ -#define list_first_entry_or_null (ptr, type, member) \ - (!list_empty(ptr) ? list_first_entry(ptr, type, member) : NULL) +#define list_first_entry_or_null (ptr, type, member) ({ \ + struct list_head *head__ = (ptr); \ + struct list_head *pos__ = READ_ONCE(head__->next); \ + pos__ != head__ ? list_entry(pos__, type, member) : NULL; \ +})/** * list_next_entry - get the next element in list
Signature · 1 line
2.5.2
PATCH 2/7 list: Split list_add() debug checking into separate function
In reply to Paul E. McKenneyFrom: Kees Cook <[email protected]>
Right now, __list_add() code is repeated either in list.h or in list_debug.c, but only the debug checks are the different part. This extracts the checking into a separate function and consolidates __list_add(). Additionally this __list_add_debug() will stop list manipulations if a corruption is detected, instead of allowing for further corruption that may lead to even worse conditions.
This is slight refactoring of the same hardening done in PaX and Grsecurity.
--- include/linux/list.h | 22 ++++++++++++++++------ lib/list_debug.c | 48 +++++++++++++++++++++++------------------------- 2 files changed, 39 insertions(+), 31 deletions(-)
include/linux/list.h +16 -6
@@ -28,27 +28,37 @@ static inline void INIT_LIST_HEAD(struct list_head *list) list->prev = list; } +#ifdef CONFIG_DEBUG_LIST +extern bool __list_add_valid (struct list_head *new, +struct list_head *prev, +struct list_head *next); +#else +static inline bool __list_add_valid (struct list_head *new, +struct list_head *prev, +struct list_head *next) +{ +return true; +} +#endif +/* * Insert a new entry between two known consecutive entries. * * This is only for internal list manipulation where we know * the prev/next entries already! */ -#ifndef CONFIG_DEBUG_LIST static inline void __list_add (struct list_head *new,struct list_head *prev,struct list_head *next) { +if (!__list_add_valid (new, prev, next)) +return ; + next->prev = new; new->next = next; new->prev = prev;WRITE_ONCE (prev->next, new); } -#else -extern void __list_add (struct list_head *new, -struct list_head *prev, -struct list_head *next); -#endif /** * list_add - add a new entry
lib/list_debug.c +23 -25
64 lines not on this page · raw message
Signature · 1 line
2.5.2
PATCH 3/7 rculist: Consolidate DEBUG_LIST for list_add_rcu()
In reply to Paul E. McKenneyFrom: Kees Cook <[email protected]>
Consolidates the debug checking for list_add_rcu() into the new single debug function. Notably, this fixes the sanity check that was added in
commit 17a801f4bfeb ("list_debug: WARN for adding something already in
the list"). Before, it wasn't being checked for RCU lists.
--- include/linux/rculist.h | 8 +++----- lib/list_debug.c | 19 ------------------- 2 files changed, 3 insertions(+), 24 deletions(-)
include/linux/rculist.h +3 -5
@@ -45,19 +45,17 @@ static inline void INIT_LIST_HEAD_RCU(struct list_head *list)* This is only for internal list manipulation where we know * the prev/next entries already! */ -#ifndef CONFIG_DEBUG_LIST static inline void __list_add_rcu (struct list_head *new,struct list_head *prev,struct list_head *next) { +if (!__list_add_valid (new, prev, next)) +return ; + new->next = next; new->prev = prev;rcu_assign_pointer (list_next_rcu (prev), new); next->prev = new; } -#else -void __list_add_rcu (struct list_head *new, -struct list_head *prev,struct list_head *next); -#endif /** * list_add_rcu - add a new entry to rcu-protected list
lib/list_debug.c +0 -19
@@ -77,22 +77,3 @@ void list_del(struct list_head *entry) entry->prev =LIST_POISON2 ; }EXPORT_SYMBOL (list_del); - -/* -* RCU variants. -*/ -void __list_add_rcu (struct list_head *new, -struct list_head *prev,struct list_head *next) -{ -WARN (next->prev != prev, -"list_add_rcu corruption. next->prev should be prev (%p), but was %p. (next=%p).\n" , - prev, next->prev, next); -WARN (prev->next != next, -"list_add_rcu corruption. prev->next should be next (%p), but was %p. (prev=%p).\n" , - next, prev->next, prev); - new->next = next; - new->prev = prev; -rcu_assign_pointer (list_next_rcu (prev), new); - next->prev = new; -} -EXPORT_SYMBOL (__list_add_rcu);
Signature · 1 line
2.5.2
PATCH 4/7 list: Split list_del() debug checking into separate function
In reply to Paul E. McKenneyFrom: Kees Cook <[email protected]>
Similar to the list_add() debug consolidation, this consolidates the debug checking performed during CONFIG_DEBUG_LIST, and stops list updates when corruption is found.
Refactored from same hardening in PaX and Grsecurity.
--- include/linux/list.h | 15 +++++++++------ lib/list_debug.c | 53 +++++++++++++++++++++++----------------------------- 2 files changed, 32 insertions(+), 36 deletions(-)
include/linux/list.h +9 -6
@@ -32,6 +32,7 @@ static inline void INIT_LIST_HEAD(struct list_head *list)extern bool __list_add_valid (struct list_head *new,struct list_head *prev,struct list_head *next); +extern bool __list_del_entry_valid (struct list_head *entry);#else static inline bool __list_add_valid(struct list_head *new, struct list_head *prev,
@@ -39,6 +40,10 @@ static inline bool __list_add_valid(struct list_head *new, {return true; } +static inline bool __list_del_entry_valid (struct list_head *entry) +{ +return true; +}#endif /*
@@ -106,22 +111,20 @@ static inline void __list_del(struct list_head * prev, struct list_head * next)* Note: list_empty() on entry does not return true after this, the entry is * in an undefined state. */ -#ifndef CONFIG_DEBUG_LIST static inline void __list_del_entry (struct list_head *entry) { +if (!__list_del_entry_valid (entry)) +return ; +__list_del (entry->prev, entry->next); }static inline void list_del (struct list_head *entry) { -__list_del (entry->prev, entry->next); +__list_del_entry (entry); entry->next =LIST_POISON1 ; entry->prev =LIST_POISON2 ; } -#else -extern void __list_del_entry (struct list_head *entry); -extern void list_del (struct list_head *entry); -#endif /** * list_replace - replace old entry by new one
lib/list_debug.c +23 -30
64 lines not on this page · raw message
Signature · 1 line
2.5.2
PATCH 5/7 bug: Provide toggle for BUG on data corruption
In reply to Paul E. McKenneyFrom: Kees Cook <[email protected]>
The kernel checks for cases of data structure corruption under some CONFIGs (e.g. CONFIG_DEBUG_LIST). When corruption is detected, some systems may want to BUG() immediately instead of letting the system run with known corruption. Usually these kinds of manipulation primitives can be used by security flaws to gain arbitrary memory write control. This provides a new config CONFIG_BUG_ON_DATA_CORRUPTION and a corresponding macro CHECK_DATA_CORRUPTION for handling these situations. Notably, even if not BUGing, the kernel should not continue processing the corrupted structure.
This is inspired by similar hardening by Syed Rameez Mustafa in MSM kernels, and in PaX and Grsecurity, which is likely in response to earlier removal of the BUG calls in commit 924d9addb9b1 ("list debugging: use WARN() instead of BUG()").
--- include/linux/bug.h | 17 ++++++++++++++++ lib/Kconfig.debug | 10 ++++++++++ lib/list_debug.c | 57 +++++++++++++++++++++-------------------------------- 3 files changed, 49 insertions(+), 35 deletions(-)
include/linux/bug.h +17 -0
@@ -118,4 +118,21 @@ static inline enum bug_trap_type report_bug(unsigned long bug_addr, }#endif /* CONFIG_GENERIC_BUG */ + +/* +* Since detected data corruption should stop operation on the affected +* structures, this returns false if the corruption condition is found. +*/ +#define CHECK_DATA_CORRUPTION (condition, fmt, ...) \ + do { \ + if (unlikely(condition)) { \ + if (IS_ENABLED(CONFIG_BUG_ON_DATA_CORRUPTION)) { \ + pr_err(fmt, ##__VA_ARGS__); \ + BUG(); \ + } else \ + WARN(1, fmt, ##__VA_ARGS__); \ + return false; \ + } \ + } while (0) +#endif /* _LINUX_BUG_H */
lib/Kconfig.debug +10 -0
@@ -1987,6 +1987,16 @@ config TEST_STATIC_KEYS If unsure, say N. +config BUG_ON_DATA_CORRUPTION + bool "Trigger a BUG when data corruption is detected" + select CONFIG_DEBUG_LIST + help + Select this option if the kernel should BUG when it encounters + data corruption in kernel memory structures when they get checked + for validity. + + If unsure, say N. + source "samples/Kconfig" source "lib/Kconfig.kgdb"
lib/list_debug.c +22 -35
69 lines not on this page · raw message
Signature · 1 line
2.5.2
PATCH 6/7 lkdtm: Add tests for struct list corruption
In reply to Paul E. McKenneyFrom: Kees Cook <[email protected]>
When building under CONFIG_DEBUG_LIST, list addition and removal will be sanity-checked. This validates that the check is working as expected by setting up classic corruption attacks against list manipulations, available with the new lkdtm tests CORRUPT_LIST_ADD and CORRUPT_LIST_DEL.
--- drivers/misc/lkdtm.h | 2 ++ drivers/misc/lkdtm_bugs.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++ drivers/misc/lkdtm_core.c | 2 ++ 3 files changed, 72 insertions(+)
drivers/misc/lkdtm.h +2 -0
@@ -21,6 +21,8 @@ void lkdtm_SPINLOCKUP(void);void lkdtm_HUNG_TASK (void );void lkdtm_ATOMIC_UNDERFLOW (void );void lkdtm_ATOMIC_OVERFLOW (void ); +void lkdtm_CORRUPT_LIST_ADD (void ); +void lkdtm_CORRUPT_LIST_DEL (void );/* lkdtm_heap.c */ void lkdtm_OVERWRITE_ALLOCATION (void );
drivers/misc/lkdtm_bugs.c +68 -0
79 lines not on this page · raw message
drivers/misc/lkdtm_core.c +2 -0
@@ -197,6 +197,8 @@ struct crashtype crashtypes[] = {CRASHTYPE (EXCEPTION ),CRASHTYPE (LOOP ),CRASHTYPE (OVERFLOW ), +CRASHTYPE (CORRUPT_LIST_ADD ), +CRASHTYPE (CORRUPT_LIST_DEL ),CRASHTYPE (CORRUPT_STACK ),CRASHTYPE (UNALIGNED_LOAD_STORE_WRITE ), CRASHTYPE(OVERWRITE_ALLOCATION),
Signature · 1 line
2.5.2
PATCH 7/7 lib/Kconfig.debug: Fix typo in select statement
In reply to Paul E. McKenneyFrom: Valentin Rothberg <[email protected]>
Commit 484f29c7430b3 ("bug: Provide toggle for BUG on data corruption") added a Kconfig select statement on CONFIG_DEBUG_LIST, but the CONFIG_ prefix is only used in Make and C(PP) syntax. Remove the CONFIG_ prefix to correctly select the Kconfig option DEBUG_LIST.
--- lib/Kconfig.debug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/Kconfig.debug +1 -1
@@ -1989,7 +1989,7 @@ config TEST_STATIC_KEYS config BUG_ON_DATA_CORRUPTION bool "Trigger a BUG when data corruption is detected" - select CONFIG_DEBUG_LIST + select DEBUG_LIST help Select this option if the kernel should BUG when it encounters data corruption in kernel memory structures when they get checked
Signature · 1 line
2.5.2