From: Kees Cook <keescook@chromium.org> To: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com> Cc: Kees Cook <keescook@chromium.org>, Laura Abbott <labbott@redhat.com>, Steven Rostedt <rostedt@goodmis.org>, Stephen Boyd <sboyd@codeaurora.org>, Daniel Micay <danielmicay@gmail.com>, Joe Perches <joe@perches.com>, Arnd Bergmann <arnd@arndb.de>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Josh Triplett <josh@joshtriplett.org>, Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, Lai Jiangshan <jiangshanlai@gmail.com>, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, Michael Ellerman <mpe@ellerman.id.au>, Dan Williams <dan.j.williams@intel.com>, Andrew Morton <akpm@linux-foundation.org>, Ingo Molnar <mingo@kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Josef Bacik <jbacik@fb.com>, Andrey Ryabinin <aryabinin@virtuozzo.com>, Tejun Heo <tj@kernel.org>, Nikolay Aleksandrov <nikolay@cumulusnetworks.com>, Dmitry Vyukov <dvyukov@google.com>, linux-kernel@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: [PATCH v2 1/5] list: Split list_add() debug checking into separate function Date: Tue, 16 Aug 2016 17:20:25 -0700 [thread overview] Message-ID: <1471393229-27182-2-git-send-email-keescook@chromium.org> (raw) In-Reply-To: <1471393229-27182-1-git-send-email-keescook@chromium.org> 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. Signed-off-by: Kees Cook <keescook@chromium.org> --- include/linux/list.h | 22 ++++++++++++++++------ lib/list_debug.c | 48 +++++++++++++++++++++++------------------------- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/include/linux/list.h b/include/linux/list.h index 5183138aa932..0ed58591538e 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; 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 diff --git a/lib/list_debug.c b/lib/list_debug.c index 3859bf63561c..149dd57b583b 100644 --- a/lib/list_debug.c +++ b/lib/list_debug.c @@ -2,8 +2,7 @@ * Copyright 2006, Red Hat, Inc., Dave Jones * Released under the General Public License (GPL). * - * This file contains the linked list implementations for - * DEBUG_LIST. + * This file contains the linked list validation for DEBUG_LIST. */ #include <linux/export.h> @@ -13,33 +12,32 @@ #include <linux/rculist.h> /* - * Insert a new entry between two known consecutive entries. - * - * This is only for internal list manipulation where we know - * the prev/next entries already! + * Check that the data structures for the list manipulations are reasonably + * valid. Failures here indicate memory corruption (and possibly an exploit + * attempt). */ -void __list_add(struct list_head *new, - struct list_head *prev, - struct list_head *next) +bool __list_add_valid(struct list_head *new, struct list_head *prev, + struct list_head *next) { - WARN(next->prev != prev, - "list_add corruption. next->prev should be " - "prev (%p), but was %p. (next=%p).\n", - prev, next->prev, next); - WARN(prev->next != next, - "list_add corruption. prev->next should be " - "next (%p), but was %p. (prev=%p).\n", - next, prev->next, prev); - WARN(new == prev || new == next, - "list_add double add: new=%p, prev=%p, next=%p.\n", - new, prev, next); - next->prev = new; - new->next = next; - new->prev = prev; - WRITE_ONCE(prev->next, new); + if (unlikely(next->prev != prev)) { + WARN(1, "list_add corruption. next->prev should be prev (%p), but was %p. (next=%p).\n", + prev, next->prev, next); + return false; + } + if (unlikely(prev->next != next)) { + WARN(1, "list_add corruption. prev->next should be next (%p), but was %p. (prev=%p).\n", + next, prev->next, prev); + return false; + } + if (unlikely(new == prev || new == next)) { + WARN(1, "list_add double add: new=%p, prev=%p, next=%p.\n", + new, prev, next); + return false; + } + return true; } -EXPORT_SYMBOL(__list_add); +EXPORT_SYMBOL(__list_add_valid); void __list_del_entry(struct list_head *entry) { -- 2.7.4
WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org> To: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com> Cc: Kees Cook <keescook@chromium.org>, Laura Abbott <labbott@redhat.com>, Steven Rostedt <rostedt@goodmis.org>, Stephen Boyd <sboyd@codeaurora.org>, Daniel Micay <danielmicay@gmail.com>, Joe Perches <joe@perches.com>, Arnd Bergmann <arnd@arndb.de>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Josh Triplett <josh@joshtriplett.org>, Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, Lai Jiangshan <jiangshanlai@gmail.com>, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, Michael Ellerman <mpe@ellerman.id.au>, Dan Williams <dan.j.williams@intel.com>, Andrew Morton <akpm@linux-foundation.org>, Ingo Molnar <mingo@kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Josef Bacik <jbacik@fb.com>, Andrey Ryabinin <aryabinin@virtuozzo.com>, Tejun Heo <tj@kernel.org>, Nikolay Aleksandrov <nikolay@cumulusnetworks.com>, Dmitry Vyukov <dvyukov@google.com>, linux-kernel@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: [kernel-hardening] [PATCH v2 1/5] list: Split list_add() debug checking into separate function Date: Tue, 16 Aug 2016 17:20:25 -0700 [thread overview] Message-ID: <1471393229-27182-2-git-send-email-keescook@chromium.org> (raw) In-Reply-To: <1471393229-27182-1-git-send-email-keescook@chromium.org> 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. Signed-off-by: Kees Cook <keescook@chromium.org> --- include/linux/list.h | 22 ++++++++++++++++------ lib/list_debug.c | 48 +++++++++++++++++++++++------------------------- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/include/linux/list.h b/include/linux/list.h index 5183138aa932..0ed58591538e 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; 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 diff --git a/lib/list_debug.c b/lib/list_debug.c index 3859bf63561c..149dd57b583b 100644 --- a/lib/list_debug.c +++ b/lib/list_debug.c @@ -2,8 +2,7 @@ * Copyright 2006, Red Hat, Inc., Dave Jones * Released under the General Public License (GPL). * - * This file contains the linked list implementations for - * DEBUG_LIST. + * This file contains the linked list validation for DEBUG_LIST. */ #include <linux/export.h> @@ -13,33 +12,32 @@ #include <linux/rculist.h> /* - * Insert a new entry between two known consecutive entries. - * - * This is only for internal list manipulation where we know - * the prev/next entries already! + * Check that the data structures for the list manipulations are reasonably + * valid. Failures here indicate memory corruption (and possibly an exploit + * attempt). */ -void __list_add(struct list_head *new, - struct list_head *prev, - struct list_head *next) +bool __list_add_valid(struct list_head *new, struct list_head *prev, + struct list_head *next) { - WARN(next->prev != prev, - "list_add corruption. next->prev should be " - "prev (%p), but was %p. (next=%p).\n", - prev, next->prev, next); - WARN(prev->next != next, - "list_add corruption. prev->next should be " - "next (%p), but was %p. (prev=%p).\n", - next, prev->next, prev); - WARN(new == prev || new == next, - "list_add double add: new=%p, prev=%p, next=%p.\n", - new, prev, next); - next->prev = new; - new->next = next; - new->prev = prev; - WRITE_ONCE(prev->next, new); + if (unlikely(next->prev != prev)) { + WARN(1, "list_add corruption. next->prev should be prev (%p), but was %p. (next=%p).\n", + prev, next->prev, next); + return false; + } + if (unlikely(prev->next != next)) { + WARN(1, "list_add corruption. prev->next should be next (%p), but was %p. (prev=%p).\n", + next, prev->next, prev); + return false; + } + if (unlikely(new == prev || new == next)) { + WARN(1, "list_add double add: new=%p, prev=%p, next=%p.\n", + new, prev, next); + return false; + } + return true; } -EXPORT_SYMBOL(__list_add); +EXPORT_SYMBOL(__list_add_valid); void __list_del_entry(struct list_head *entry) { -- 2.7.4
next prev parent reply other threads:[~2016-08-17 0:25 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-08-17 0:20 [PATCH v2 0/5] bug: Provide toggle for BUG on data corruption Kees Cook 2016-08-17 0:20 ` [kernel-hardening] " Kees Cook 2016-08-17 0:20 ` Kees Cook [this message] 2016-08-17 0:20 ` [kernel-hardening] [PATCH v2 1/5] list: Split list_add() debug checking into separate function Kees Cook 2016-08-17 13:16 ` Steven Rostedt 2016-08-17 13:16 ` [kernel-hardening] " Steven Rostedt 2016-08-17 0:20 ` [PATCH v2 2/5] rculist: Consolidate DEBUG_LIST for list_add_rcu() Kees Cook 2016-08-17 0:20 ` [kernel-hardening] " Kees Cook 2016-08-17 0:20 ` [PATCH v2 3/5] list: Split list_del() debug checking into separate function Kees Cook 2016-08-17 0:20 ` [kernel-hardening] " Kees Cook 2016-08-17 0:20 ` [PATCH v2 4/5] bug: Provide toggle for BUG on data corruption Kees Cook 2016-08-17 0:20 ` [kernel-hardening] " Kees Cook 2016-08-17 0:26 ` Joe Perches 2016-08-17 0:26 ` [kernel-hardening] " Joe Perches 2016-08-17 3:39 ` Kees Cook 2016-08-17 3:39 ` [kernel-hardening] " Kees Cook 2016-08-17 13:20 ` Steven Rostedt 2016-08-17 13:20 ` [kernel-hardening] " Steven Rostedt 2016-08-17 0:20 ` [PATCH v2 5/5] lkdtm: Add tests for struct list corruption Kees Cook 2016-08-17 0:20 ` [kernel-hardening] " Kees Cook 2016-08-17 0:55 ` [PATCH v2 0/5] bug: Provide toggle for BUG on data corruption Henrique de Moraes Holschuh 2016-08-17 0:55 ` [kernel-hardening] " Henrique de Moraes Holschuh 2016-08-17 3:37 ` Kees Cook 2016-08-17 3:37 ` [kernel-hardening] " Kees Cook 2016-08-17 20:17 ` Stephen Boyd 2016-08-17 20:17 ` [kernel-hardening] " Stephen Boyd 2016-08-17 21:11 ` Kees Cook 2016-08-17 21:11 ` [kernel-hardening] " 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=1471393229-27182-2-git-send-email-keescook@chromium.org \ --to=keescook@chromium.org \ --cc=akpm@linux-foundation.org \ --cc=aneesh.kumar@linux.vnet.ibm.com \ --cc=arnd@arndb.de \ --cc=aryabinin@virtuozzo.com \ --cc=dan.j.williams@intel.com \ --cc=danielmicay@gmail.com \ --cc=dvyukov@google.com \ --cc=gregkh@linuxfoundation.org \ --cc=jbacik@fb.com \ --cc=jiangshanlai@gmail.com \ --cc=joe@perches.com \ --cc=josh@joshtriplett.org \ --cc=kernel-hardening@lists.openwall.com \ --cc=kirill.shutemov@linux.intel.com \ --cc=labbott@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=mathieu.desnoyers@efficios.com \ --cc=mingo@kernel.org \ --cc=mpe@ellerman.id.au \ --cc=nikolay@cumulusnetworks.com \ --cc=paulmck@linux.vnet.ibm.com \ --cc=rostedt@goodmis.org \ --cc=sboyd@codeaurora.org \ --cc=tglx@linutronix.de \ --cc=tj@kernel.org \ /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: linkBe 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.