From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752835AbcE2Qmq (ORCPT ); Sun, 29 May 2016 12:42:46 -0400 Received: from mail-oi0-f45.google.com ([209.85.218.45]:36726 "EHLO mail-oi0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752771AbcE2Qmp (ORCPT ); Sun, 29 May 2016 12:42:45 -0400 MIME-Version: 1.0 In-Reply-To: <20160525094906.GA4420@pd.tnic> References: <5bcff14f574b1a9852d1334a9fa8886c70c928c4.1464129798.git.luto@kernel.org> <20160525094906.GA4420@pd.tnic> From: Andy Lutomirski Date: Sun, 29 May 2016 09:42:24 -0700 Message-ID: Subject: Re: [PATCH 5/7] x86/uaccess: Warn on uaccess faults other than #PF To: Borislav Petkov Cc: Andy Lutomirski , X86 ML , "linux-kernel@vger.kernel.org" , Kees Cook , Brian Gerst Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, May 25, 2016 at 2:49 AM, Borislav Petkov wrote: > On Tue, May 24, 2016 at 03:48:42PM -0700, Andy Lutomirski wrote: >> If a uaccess instruction fails due to an8 error other than #PF, >> warn. If the fault is #GP, it most likely indicates access to a >> non-canonical address, which means that an access_ok check is >> missing, and that's bad. If the fault is something else (#UD?), >> then something is very wrong and we should diagnose it rather >> than ignoring it. >> >> Signed-off-by: Andy Lutomirski >> --- >> arch/x86/mm/extable.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c >> index 658292fdee5e..c1933471fce7 100644 >> --- a/arch/x86/mm/extable.c >> +++ b/arch/x86/mm/extable.c >> @@ -29,6 +29,19 @@ EXPORT_SYMBOL(ex_handler_default); >> static bool uaccess_fault_okay(int trapnr, unsigned long error_code, >> unsigned long extra) >> { >> + /* >> + * For uaccess, only page faults should be fixed up. I can't see >> + * any exploit mitigation value in OOPSing on other types of faults, >> + * so just warn and continue if that happens. This means that >> + * uaccess faults to non-canonical addresses will warn. That's okay >> + * -- this will only happen if an access_ok is missing, and we want to >> + * detect that error if it happens. >> + */ >> + if (WARN_ONCE(trapnr != X86_TRAP_PF, >> + "unexpected uaccess trap %d (may indicate a missing access_ok on a non-canonical address)\n", >> + trapnr)) > > Perhaps dump also regs->ip and make the warn message more helpful... > The stack trace will show it, and I'm not convinced that regs->ip by itself will be all that helpful -- depending on what gets inlined, it could just point to __copy_from_user or similar. >> + return true; /* no good reason to OOPS. */ > > You love those side comments, don'tcha? :-) ECO tip #102: avoid silly newlines :-p --Andy