From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D700EC433DB for ; Thu, 11 Feb 2021 08:49:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7E75E64E38 for ; Thu, 11 Feb 2021 08:49:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229702AbhBKIra (ORCPT ); Thu, 11 Feb 2021 03:47:30 -0500 Received: from mail.kernel.org ([198.145.29.99]:36444 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229478AbhBKIkp (ORCPT ); Thu, 11 Feb 2021 03:40:45 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id 8D45464E7D; Thu, 11 Feb 2021 08:39:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1613032750; bh=bqZJ/IZSzLA4VIQ7PM3Z5GOEZ9mZPLQ6ZRDuR1URnf8=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=hNgP4oKHV61iOtHE0Sw3R09LyLE1/MMtGx0Bv3YViJSWRJ4+7bRP6DUveKMc6feq8 CAUR/5nAdTJqNqpDKPmZp1lNtRDzdw3rqm7doEZH/GbOzzs9pUiL4aV0jwMm7wLhq5 SvKoTVTcfT18N8nlcFfwCgcVMxp//6H+oFnIG5e16/vKd6mwKUYoBhj230n0YOjNfo qis/fKxR15kXUo2WdCUc5CfZXs9XzpURs/wN3jyFdZfAgdyaPqqNi2f5+h0g2lPPtY SK9luynXVEoZmCxInv5nVccsT+tBe4U6Qq0n0HhIkpvTqU3wU0WM63qE9c3ngeUrWT cmSbwdGRZZySg== Received: by mail-ot1-f42.google.com with SMTP id i20so4471838otl.7; Thu, 11 Feb 2021 00:39:10 -0800 (PST) X-Gm-Message-State: AOAM533/C4XEfb/77h8KtbE2ctVspOyKA5BLgI5Uw6VZ6TWgPDk8XhaK iEZCZUZQaBnzi0lR32bHX45hObMjVTk7K+msH/E= X-Google-Smtp-Source: ABdhPJySlkSf5kNF2I/QffAamswNjgwWlw5DvpDxObLmvFfQUsdJVEQUwB5QlCa4rK/oleLEVvdMU/NxCtXKVFoB8I4= X-Received: by 2002:a05:6830:1285:: with SMTP id z5mr5020865otp.90.1613032749813; Thu, 11 Feb 2021 00:39:09 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Ard Biesheuvel Date: Thu, 11 Feb 2021 09:38:58 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 14/14] x86/fault, x86/efi: Fix and rename efi_recover_from_page_fault() To: Andy Lutomirski Cc: X86 ML , LKML , Dave Hansen , Alexei Starovoitov , Daniel Borkmann , Yonghong Song , Masami Hiramatsu , Peter Zijlstra , linux-efi Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 10 Feb 2021 at 03:34, Andy Lutomirski wrote: > > efi_recover_from_page_fault() doesn't recover -- it does a special EFI > mini-oops. Rename it to make it clear that it crashes. > > While renaming it, I noticed a blatant bug: a page fault oops in a > different thread happening concurrently with an EFI runtime service call > would be misinterpreted as an EFI page fault. Fix that. > > This isn't quite exact. We could do better by using a special CS for > calls into EFI. > > Cc: Dave Hansen > Cc: Peter Zijlstra > Cc: Ard Biesheuvel > Cc: linux-efi@vger.kernel.org > Signed-off-by: Andy Lutomirski Acked-by: Ard Biesheuvel > --- > arch/x86/include/asm/efi.h | 2 +- > arch/x86/mm/fault.c | 11 ++++++----- > arch/x86/platform/efi/quirks.c | 16 ++++++++++++---- > 3 files changed, 19 insertions(+), 10 deletions(-) > > diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h > index c98f78330b09..4b7706ddd8b6 100644 > --- a/arch/x86/include/asm/efi.h > +++ b/arch/x86/include/asm/efi.h > @@ -150,7 +150,7 @@ extern void __init efi_apply_memmap_quirks(void); > extern int __init efi_reuse_config(u64 tables, int nr_tables); > extern void efi_delete_dummy_variable(void); > extern void efi_switch_mm(struct mm_struct *mm); > -extern void efi_recover_from_page_fault(unsigned long phys_addr); > +extern void efi_crash_gracefully_on_page_fault(unsigned long phys_addr); > extern void efi_free_boot_services(void); > > /* kexec external ABI */ > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c > index eed217d4a877..dfdd56d9c020 100644 > --- a/arch/x86/mm/fault.c > +++ b/arch/x86/mm/fault.c > @@ -16,7 +16,7 @@ > #include /* prefetchw */ > #include /* exception_enter(), ... */ > #include /* faulthandler_disabled() */ > -#include /* efi_recover_from_page_fault()*/ > +#include /* efi_crash_gracefully_on_page_fault()*/ > #include > > #include /* boot_cpu_has, ... */ > @@ -25,7 +25,7 @@ > #include /* emulate_vsyscall */ > #include /* struct vm86 */ > #include /* vma_pkey() */ > -#include /* efi_recover_from_page_fault()*/ > +#include /* efi_crash_gracefully_on_page_fault()*/ > #include /* store_idt(), ... */ > #include /* exception stack */ > #include /* VMALLOC_START, ... */ > @@ -700,11 +700,12 @@ page_fault_oops(struct pt_regs *regs, unsigned long error_code, > #endif > > /* > - * Buggy firmware could access regions which might page fault, try to > - * recover from such faults. > + * Buggy firmware could access regions which might page fault. If > + * this happens, EFI has a special OOPS path that will try to > + * avoid hanging the system. > */ > if (IS_ENABLED(CONFIG_EFI)) > - efi_recover_from_page_fault(address); > + efi_crash_gracefully_on_page_fault(address); > > oops: > /* > diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c > index 5a40fe411ebd..0463ef9cddd6 100644 > --- a/arch/x86/platform/efi/quirks.c > +++ b/arch/x86/platform/efi/quirks.c > @@ -687,15 +687,25 @@ int efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff, > * @return: Returns, if the page fault is not handled. This function > * will never return if the page fault is handled successfully. > */ > -void efi_recover_from_page_fault(unsigned long phys_addr) > +void efi_crash_gracefully_on_page_fault(unsigned long phys_addr) > { > if (!IS_ENABLED(CONFIG_X86_64)) > return; > > + /* > + * If we are in an interrupt nested inside an EFI runtime service, > + * then this is a regular OOPS, not an EFI failure. > + */ > + if (in_interrupt() || in_nmi() || in_softirq()) > + return; > + > /* > * Make sure that an efi runtime service caused the page fault. > + * READ_ONCE() because we might be OOPSing in a different thread, > + * and we don't want to trip KTSAN while trying to OOPS. > */ > - if (efi_rts_work.efi_rts_id == EFI_NONE) > + if (READ_ONCE(efi_rts_work.efi_rts_id) == EFI_NONE || > + current_work() != &efi_rts_work.work) > return; > > /* > @@ -747,6 +757,4 @@ void efi_recover_from_page_fault(unsigned long phys_addr) > set_current_state(TASK_IDLE); > schedule(); > } > - > - return; > } > -- > 2.29.2 >