From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753715AbcHPV0X (ORCPT ); Tue, 16 Aug 2016 17:26:23 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:52966 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753368AbcHPV0V (ORCPT ); Tue, 16 Aug 2016 17:26:21 -0400 X-IBM-Helo: d03dlp02.boulder.ibm.com X-IBM-MailFrom: paulmck@linux.vnet.ibm.com Date: Tue, 16 Aug 2016 14:26:05 -0700 From: "Paul E. McKenney" To: Kees Cook Cc: Stephen Boyd , Daniel Micay , Arnd Bergmann , Greg Kroah-Hartman , Josh Triplett , Steven Rostedt , Mathieu Desnoyers , Lai Jiangshan , Peter Zijlstra , Ingo Molnar , Tejun Heo , Michael Ellerman , "Aneesh Kumar K.V" , "Kirill A. Shutemov" , Andrew Morton , Dan Williams , Jan Kara , Josef Bacik , Thomas Gleixner , Andrey Ryabinin , Nikolay Aleksandrov , Dmitry Vyukov , linux-kernel@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: Re: [PATCH 4/5] bug: Provide toggle for BUG on data corruption Reply-To: paulmck@linux.vnet.ibm.com References: <1471381865-25724-1-git-send-email-keescook@chromium.org> <1471381865-25724-5-git-send-email-keescook@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1471381865-25724-5-git-send-email-keescook@chromium.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16081621-0024-0000-0000-00001456F8B4 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00005605; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000181; SDB=6.00745602; UDB=6.00351418; IPR=6.00518151; BA=6.00004662; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00012353; XFM=3.00000011; UTC=2016-08-16 21:26:03 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16081621-0025-0000-0000-0000439CF003 Message-Id: <20160816212605.GY3482@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-08-16_12:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1608160242 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Aug 16, 2016 at 02:11:04PM -0700, Kees Cook wrote: > The kernel checks for several cases of data structure corruption under > either normal runtime, or under various CONFIG_DEBUG_* settings. When > corruption is detected, some systems may want to BUG() immediately instead > of letting the corruption continue. Many of these manipulation primitives > can be used by security flaws to gain arbitrary memory write control. This > provides CONFIG_BUG_ON_CORRUPTION to control newly added BUG() locations. > > This is inspired by similar hardening in PaX and Grsecurity, and by > Stephen Boyd in MSM kernels. > > Signed-off-by: Kees Cook OK, I will bite... Why both the WARN() and the BUG_ON()? Thanx, Paul > --- > include/linux/bug.h | 7 +++++++ > kernel/locking/spinlock_debug.c | 1 + > kernel/workqueue.c | 2 ++ > lib/Kconfig.debug | 10 ++++++++++ > lib/list_debug.c | 7 +++++++ > 5 files changed, 27 insertions(+) > > diff --git a/include/linux/bug.h b/include/linux/bug.h > index e51b0709e78d..7e69758dd798 100644 > --- a/include/linux/bug.h > +++ b/include/linux/bug.h > @@ -118,4 +118,11 @@ static inline enum bug_trap_type report_bug(unsigned long bug_addr, > } > > #endif /* CONFIG_GENERIC_BUG */ > + > +#ifdef CONFIG_BUG_ON_CORRUPTION > +# define CORRUPTED_DATA_STRUCTURE true > +#else > +# define CORRUPTED_DATA_STRUCTURE false > +#endif > + > #endif /* _LINUX_BUG_H */ > diff --git a/kernel/locking/spinlock_debug.c b/kernel/locking/spinlock_debug.c > index 0374a596cffa..d5f833769feb 100644 > --- a/kernel/locking/spinlock_debug.c > +++ b/kernel/locking/spinlock_debug.c > @@ -64,6 +64,7 @@ static void spin_dump(raw_spinlock_t *lock, const char *msg) > owner ? owner->comm : "", > owner ? task_pid_nr(owner) : -1, > lock->owner_cpu); > + BUG_ON(CORRUPTED_DATA_STRUCTURE); > dump_stack(); > } > > diff --git a/kernel/workqueue.c b/kernel/workqueue.c > index ef071ca73fc3..ea0132b55eca 100644 > --- a/kernel/workqueue.c > +++ b/kernel/workqueue.c > @@ -48,6 +48,7 @@ > #include > #include > #include > +#include > > #include "workqueue_internal.h" > > @@ -2108,6 +2109,7 @@ __acquires(&pool->lock) > current->comm, preempt_count(), task_pid_nr(current), > worker->current_func); > debug_show_held_locks(current); > + BUG_ON(CORRUPTED_DATA_STRUCTURE); > dump_stack(); > } > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 2307d7c89dac..d64bd6b6fd45 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -1987,6 +1987,16 @@ config TEST_STATIC_KEYS > > If unsure, say N. > > +config BUG_ON_CORRUPTION > + bool "Trigger a BUG when data corruption is detected" > + help > + Select this option if the kernel should BUG when it encounters > + data corruption in various kernel memory structures during checks > + added by options like CONFIG_DEBUG_LIST, CONFIG_DEBUG_SPINLOCK, > + etc. > + > + If unsure, say N. > + > source "samples/Kconfig" > > source "lib/Kconfig.kgdb" > diff --git a/lib/list_debug.c b/lib/list_debug.c > index 80e2e40a6a4e..161c7e7d3478 100644 > --- a/lib/list_debug.c > +++ b/lib/list_debug.c > @@ -26,16 +26,19 @@ bool __list_add_debug(struct list_head *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); > + BUG_ON(CORRUPTED_DATA_STRUCTURE); > 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); > + BUG_ON(CORRUPTED_DATA_STRUCTURE); > return false; > } > if (unlikely(new == prev || new == next)) { > WARN(1, "list_add double add: new=%p, prev=%p, next=%p.\n", > new, prev, next); > + BUG_ON(CORRUPTED_DATA_STRUCTURE); > return false; > } > return true; > @@ -52,21 +55,25 @@ bool __list_del_entry_debug(struct list_head *entry) > if (unlikely(next == LIST_POISON1)) { > WARN(1, "list_del corruption, %p->next is LIST_POISON1 (%p)\n", > entry, LIST_POISON1); > + BUG_ON(CORRUPTED_DATA_STRUCTURE); > return false; > } > if (unlikely(prev == LIST_POISON2)) { > WARN(1, "list_del corruption, %p->prev is LIST_POISON2 (%p)\n", > entry, LIST_POISON2); > + BUG_ON(CORRUPTED_DATA_STRUCTURE); > return false; > } > if (unlikely(prev->next != entry)) { > WARN(1, "list_del corruption. prev->next should be %p, but was %p\n", > entry, prev->next); > + BUG_ON(CORRUPTED_DATA_STRUCTURE); > return false; > } > if (unlikely(next->prev != entry)) { > WARN(1, "list_del corruption. next->prev should be %p, but was %p\n", > entry, next->prev); > + BUG_ON(CORRUPTED_DATA_STRUCTURE); > return false; > } > return true; > -- > 2.7.4 >