Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753723AbcDCNWd (ORCPT ); Sun, 3 Apr 2016 09:22:33 -0400 Received: from mail-ob0-f170.google.com ([209.85.214.170]:34523 "EHLO mail-ob0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751724AbcDCNWb (ORCPT ); Sun, 3 Apr 2016 09:22:31 -0400 MIME-Version: 1.0 In-Reply-To: <20160403080737.GA19007@pd.tnic> References: <20fc047d926150cb08cb9b9f2923519b07ec1a15.1459605520.git.luto@kernel.org> <20160402185227.GB2538@pd.tnic> <20160402205248.GD2538@pd.tnic> <20160403080737.GA19007@pd.tnic> From: Andy Lutomirski Date: Sun, 3 Apr 2016 06:22:10 -0700 Message-ID: Subject: Re: [PATCH v5 4/9] x86/traps: Enable all exception handler callbacks early To: Borislav Petkov Cc: "H. Peter Anvin" , Andy Lutomirski , X86 ML , Paolo Bonzini , Peter Zijlstra , KVM list , Arjan van de Ven , xen-devel , "linux-kernel@vger.kernel.org" , Linus Torvalds , Andrew Morton Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2755 Lines: 62 On Sun, Apr 3, 2016 at 1:07 AM, Borislav Petkov wrote: > On Sat, Apr 02, 2016 at 10:52:48PM +0200, Borislav Petkov wrote: >> On Sat, Apr 02, 2016 at 01:16:07PM -0700, Andy Lutomirski wrote: >> > I have no idea why it was explicitly unsupported, but I'm guessing it >> > was just to avoid duplicating the code. Early "ext" uaccess failures >> > are certainly not going to work, but I don't think this is a problem >> > -- there's no userspace before trap_init runs, so how exactly is an >> > "ext" uaccess going to happen in the first place? >> > >> > In any event, if it did happen in older kernels, it would have >> > immediately panicked due to that code. At least with my code it just >> > might manage to EFAULT correctly. >> >> Yeah, I was wondering what that early thing meant. >> >> Linus or tip guys probably remember what this whole deal with early >> uaccess was about. I'll try to do some git archeology tomorrow. > > Yep, just as I suspected: > > 6a1ea279c210 ("x86, extable: Add early_fixup_exception()") > > Apparently, thread_info might not have been setup yet. I'm guessing the > intention behind this no-uaccess-fixup-early is to not even attempt any > fixup due to stuff *probably* not initialized yet and so the safer thing > would be to panic instead. > > I'm wondering whether making it try to EFAULT correctly is the right > thing to do... We're certainly more conservative if we panic and not > allow some silently failed attempt at recovery which looks successful, > to continue. > > hpa, thoughts? I don't think this matters much. There aren't many users of this mechanism in the tree: ./arch/x86/kernel/signal.c: get_user_try { ./arch/x86/kernel/signal.c: put_user_try { ./arch/x86/kernel/signal.c: put_user_try { ./arch/x86/kernel/signal.c: put_user_try { ./arch/x86/kernel/signal.c: put_user_try { ./arch/x86/kernel/signal_compat.c: put_user_try { ./arch/x86/kernel/signal_compat.c: get_user_try { ./arch/x86/kernel/vm86_32.c: put_user_try { ./arch/x86/kernel/vm86_32.c: get_user_try { ./arch/x86/ia32/ia32_signal.c: get_user_try { ./arch/x86/ia32/ia32_signal.c: put_user_try { ./arch/x86/ia32/ia32_signal.c: put_user_try { ./arch/x86/ia32/ia32_signal.c: put_user_try { ./arch/x86/include/asm/uaccess.h: * {get|put}_user_try and catch ./arch/x86/include/asm/uaccess.h: * get_user_try { ./arch/x86/include/asm/uaccess.h:#define get_user_try uaccess_try ./arch/x86/include/asm/uaccess.h:#define put_user_try uaccess_try I don't see how we could get to that code in the first place without current_thread_info() working. If we can ever convince gcc to do jump labels properly for uaccess, it would probably be better to just delete all that code. --Andy