From 68e051d4a782767639205e447c083d08c51bc028 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 17 Aug 2016 14:42:08 -0700 Subject: BACKPORT: list: Split list_add() debug checking into separate function (cherry-picked from d7c816733d501b59dbdc2483f2cc8e4431fd9160) Right now, __list_add() code is repeated either in list.h or in list_debug.c, but the only differences between the two versions are the debug checks. This commit therefore extracts these debug checks into a separate __list_add_valid() function and consolidates __list_add(). Additionally this new __list_add_valid() function 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. Change-Id: I9a9c9a58857cf837bec7abdb2ee4970cd1242a5e Signed-off-by: Kees Cook Acked-by: Steven Rostedt Signed-off-by: Paul E. McKenney Acked-by: Rik van Riel Signed-off-by: Satya Tangirala --- include/linux/list.h | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) (limited to 'include/linux') diff --git a/include/linux/list.h b/include/linux/list.h index 993395a2e55c..eb783a0192bd 100644 --- a/include/linux/list.h +++ b/include/linux/list.h @@ -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; 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 -- cgit v1.2.3 From f0b68d746af4aa4988463b24c6f5d0f97f0aaa82 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 17 Aug 2016 14:42:09 -0700 Subject: UPSTREAM: rculist: Consolidate DEBUG_LIST for list_add_rcu() (cherry-picked from 54acd4397d7e7a725c94101180cd9f38ef701acc) This commit consolidates the debug checking for list_add_rcu() into the new single __list_add_valid() debug function. Notably, this commit fixes the sanity check that was added in commit 17a801f4bfeb ("list_debug: WARN for adding something already in the list"), which wasn't checking RCU-protected lists. Change-Id: I1f7e169d4dc45bbc9938087a171c5df747344414 Signed-off-by: Kees Cook Acked-by: Steven Rostedt Signed-off-by: Paul E. McKenney Acked-by: Rik van Riel Signed-off-by: Satya Tangirala --- include/linux/rculist.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'include/linux') diff --git a/include/linux/rculist.h b/include/linux/rculist.h index 5ed540986019..0c94d17a4642 100644 --- a/include/linux/rculist.h +++ b/include/linux/rculist.h @@ -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 -- cgit v1.2.3 From 10c8c8a765f1335c46817d6dbf3e288dd45884df Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 17 Aug 2016 14:42:10 -0700 Subject: UPSTREAM: list: Split list_del() debug checking into separate function (cherry-picked from 0cd340dcb05c4a43742fe156f36737bb2a321bfd) Similar to the list_add() debug consolidation, this commit consolidates the debug checking performed during CONFIG_DEBUG_LIST into a new __list_del_entry_valid() function, and stops list updates when corruption is found. Refactored from same hardening in PaX and Grsecurity. Change-Id: I9e3b8654ab25f3a196e3336fc4882b73010873e7 Signed-off-by: Kees Cook Acked-by: Steven Rostedt Signed-off-by: Paul E. McKenney Acked-by: Rik van Riel Signed-off-by: Satya Tangirala --- include/linux/list.h | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'include/linux') diff --git a/include/linux/list.h b/include/linux/list.h index eb783a0192bd..d5750f2f1c36 100644 --- a/include/linux/list.h +++ b/include/linux/list.h @@ -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 -- cgit v1.2.3 From 7514f97280fdf4373f99ca28e573925cfb828aa7 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 17 Aug 2016 14:42:11 -0700 Subject: UPSTREAM: bug: Provide toggle for BUG on data corruption (cherry-picked from de54ebbe26bb371a6f1fbc0593372232f04e3107) 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()"). Change-Id: I4cdfa9fbebe32a990a111d051e4ec4e421f77a09 Signed-off-by: Kees Cook Acked-by: Steven Rostedt Signed-off-by: Paul E. McKenney Acked-by: Rik van Riel Signed-off-by: Satya Tangirala --- include/linux/bug.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'include/linux') diff --git a/include/linux/bug.h b/include/linux/bug.h index 7f4818673c41..2bafb1d6ee89 100644 --- a/include/linux/bug.h +++ b/include/linux/bug.h @@ -109,4 +109,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 */ -- cgit v1.2.3 From a3772a806a04164f2fc9a956bb997c4b8c8179e9 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 24 Feb 2017 15:00:38 -0800 Subject: UPSTREAM: bug: switch data corruption check to __must_check (cherry-picked from 85caa95b9f19bb3a26d7e025d1134760b69e0c40) The CHECK_DATA_CORRUPTION() macro was designed to have callers do something meaningful/protective on failure. However, using "return false" in the macro too strictly limits the design patterns of callers. Instead, let callers handle the logic test directly, but make sure that the result IS checked by forcing __must_check (which appears to not be able to be used directly on macro expressions). Change-Id: I635dc2f39959104ea8b475d2d5018af3502f33ba Link: http://lkml.kernel.org/r/20170206204547.GA125312@beast Signed-off-by: Kees Cook Suggested-by: Arnd Bergmann Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds Signed-off-by: Satya Tangirala --- include/linux/bug.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'include/linux') diff --git a/include/linux/bug.h b/include/linux/bug.h index 2bafb1d6ee89..833746d361cf 100644 --- a/include/linux/bug.h +++ b/include/linux/bug.h @@ -112,18 +112,20 @@ static inline enum bug_trap_type report_bug(unsigned long bug_addr, /* * Since detected data corruption should stop operation on the affected - * structures, this returns false if the corruption condition is found. + * structures. Return value must be checked and sanely acted on by caller. */ +static inline __must_check bool check_data_corruption(bool v) { return v; } #define CHECK_DATA_CORRUPTION(condition, fmt, ...) \ - do { \ - if (unlikely(condition)) { \ + check_data_corruption(({ \ + bool corruption = unlikely(condition); \ + if (corruption) { \ if (IS_ENABLED(CONFIG_BUG_ON_DATA_CORRUPTION)) { \ pr_err(fmt, ##__VA_ARGS__); \ BUG(); \ } else \ WARN(1, fmt, ##__VA_ARGS__); \ - return false; \ } \ - } while (0) + corruption; \ + })) #endif /* _LINUX_BUG_H */ -- cgit v1.2.3 From 2191e07a73b26fa1f56b9e64f983f9da3f173883 Mon Sep 17 00:00:00 2001 From: Thomas Garnier Date: Wed, 14 Jun 2017 18:12:01 -0700 Subject: BACKPORT: x86/syscalls: Check address limit on user-mode return (cherry-picked from 5ea0727b163cb5575e36397a12eade68a1f35f24) Ensure the address limit is a user-mode segment before returning to user-mode. Otherwise a process can corrupt kernel-mode memory and elevate privileges [1]. The set_fs function sets the TIF_SETFS flag to force a slow path on return. In the slow path, the address limit is checked to be USER_DS if needed. The addr_limit_user_check function is added as a cross-architecture function to check the address limit. [1] https://bugs.chromium.org/p/project-zero/issues/detail?id=990 Change-Id: I604d85b262cc5b439b2665852865ca5a9ea6c5a3 Signed-off-by: Thomas Garnier Signed-off-by: Thomas Gleixner Cc: Mark Rutland Cc: kernel-hardening@lists.openwall.com Cc: Catalin Marinas Cc: Will Deacon Cc: David Howells Cc: Dave Hansen Cc: Miroslav Benes Cc: Chris Metcalf Cc: Pratyush Anand Cc: Russell King Cc: Petr Mladek Cc: Rik van Riel Cc: Kees Cook Cc: Arnd Bergmann Cc: Al Viro Cc: Andy Lutomirski Cc: Josh Poimboeuf Cc: linux-arm-kernel@lists.infradead.org Cc: Will Drewry Cc: linux-api@vger.kernel.org Cc: Oleg Nesterov Cc: Andy Lutomirski Cc: Paolo Bonzini Link: http://lkml.kernel.org/r/20170615011203.144108-1-thgarnie@google.com Signed-off-by: Satya Tangirala --- include/linux/syscalls.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'include/linux') diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index c2b66a277e98..a95cb2589765 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -205,6 +205,22 @@ extern struct trace_event_functions exit_syscall_print_funcs; } \ static inline long SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__)) +#ifdef TIF_FSCHECK +/* + * Called before coming back to user-mode. Returning to user-mode with an + * address limit different than USER_DS can allow to overwrite kernel memory. + */ +static inline void addr_limit_user_check(void) +{ + + if (!test_thread_flag(TIF_FSCHECK)) + return; + + BUG_ON(!segment_eq(get_fs(), USER_DS)); + clear_thread_flag(TIF_FSCHECK); +} +#endif + asmlinkage long sys32_quotactl(unsigned int cmd, const char __user *special, qid_t id, void __user *addr); asmlinkage long sys_time(time_t __user *tloc); -- cgit v1.2.3 From 1a0df285e47623996c2879ee2507a70730c5e0dc Mon Sep 17 00:00:00 2001 From: Thomas Garnier Date: Thu, 7 Sep 2017 08:30:44 -0700 Subject: UPSTREAM: syscalls: Use CHECK_DATA_CORRUPTION for addr_limit_user_check (cherry-picked from bf29ed1567b67854dc13504f685c45a2ea9b2081) Use CHECK_DATA_CORRUPTION instead of BUG_ON to provide more flexibility on address limit failures. By default, send a SIGKILL signal to kill the current process preventing exploitation of a bad address limit. Make the TIF_FSCHECK flag optional so ARM can use this function. Change-Id: I02b39760aaa794db77de7b0c0b1b0ec66abe1cb1 Signed-off-by: Thomas Garnier Signed-off-by: Kees Cook Signed-off-by: Thomas Gleixner Cc: Pratyush Anand Cc: Dave Martin Cc: Will Drewry Cc: Arnd Bergmann Cc: Catalin Marinas Cc: Will Deacon Cc: Russell King Cc: Andy Lutomirski Cc: David Howells Cc: Dave Hansen Cc: Al Viro Cc: linux-api@vger.kernel.org Cc: Yonghong Song Cc: linux-arm-kernel@lists.infradead.org Link: http://lkml.kernel.org/r/1504798247-48833-2-git-send-email-keescook@chromium.org Signed-off-by: Satya Tangirala --- include/linux/syscalls.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'include/linux') diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index a95cb2589765..5d2779aa4bbe 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -205,21 +205,25 @@ extern struct trace_event_functions exit_syscall_print_funcs; } \ static inline long SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__)) -#ifdef TIF_FSCHECK /* * Called before coming back to user-mode. Returning to user-mode with an * address limit different than USER_DS can allow to overwrite kernel memory. */ static inline void addr_limit_user_check(void) { - +#ifdef TIF_FSCHECK if (!test_thread_flag(TIF_FSCHECK)) return; +#endif - BUG_ON(!segment_eq(get_fs(), USER_DS)); + if (CHECK_DATA_CORRUPTION(!segment_eq(get_fs(), USER_DS), + "Invalid address limit on user-mode return")) + force_sig(SIGKILL, current); + +#ifdef TIF_FSCHECK clear_thread_flag(TIF_FSCHECK); -} #endif +} asmlinkage long sys32_quotactl(unsigned int cmd, const char __user *special, qid_t id, void __user *addr); -- cgit v1.2.3