Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756917AbcC2Num (ORCPT ); Tue, 29 Mar 2016 09:50:42 -0400 Received: from mail-io0-f175.google.com ([209.85.223.175]:35574 "EHLO mail-io0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753890AbcC2Nul (ORCPT ); Tue, 29 Mar 2016 09:50:41 -0400 MIME-Version: 1.0 In-Reply-To: <20160329122658.GC3625@codeblueprint.co.uk> References: <1458219431-24741-1-git-send-email-matt@codeblueprint.co.uk> <1458219431-24741-3-git-send-email-matt@codeblueprint.co.uk> <20160321203159.GF11676@codeblueprint.co.uk> <20160329122658.GC3625@codeblueprint.co.uk> Date: Tue, 29 Mar 2016 15:50:39 +0200 Message-ID: Subject: Re: [PATCH 2/4] efi: Capsule update support From: Ard Biesheuvel To: Matt Fleming Cc: "linux-efi@vger.kernel.org" , "linux-kernel@vger.kernel.org" , joeyli , Kweh Hock Leong , Borislav Petkov , Mark Salter , Peter Jones , "Bryan O'Donoghue" 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: 1870 Lines: 50 On 29 March 2016 at 14:26, Matt Fleming wrote: > On Mon, 21 Mar, at 08:31:59PM, Matt Fleming wrote: >> >> Good question. They're not handled in any special way with this patch >> series, so the firmware will just initiate its own reset inside of >> UpdateCapsule(). >> >> That's probably not what we want, because things like on-disk >> consistency are not guaranteed if the machine spontaneously reboots >> without assistance from the kernel. >> >> The simplest thing to do is to refuse to pass such capsules to the >> firmware, since it's likely not going to be a common use case. But >> maybe that's overly restrictive. >> >> Let me have a think about that one. > > OK, I did think about this, and until someone actually requests the > ability to handle CAPSULE_FLAGS_INITIATE_RESET, I'm happy to just punt > on the problem. Anyone got any objections? > Nope > --- > > diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c > index dac25208ad5e..84450e9cdf41 100644 > --- a/drivers/firmware/efi/capsule.c > +++ b/drivers/firmware/efi/capsule.c > @@ -84,6 +84,14 @@ int efi_capsule_supported(efi_guid_t guid, u32 flags, size_t size, int *reset) > u64 max_size; > int rv = 0; > > + /* > + * We do not handle firmware-initiated reset because that > + * would require us to prepare the kernel for reboot. Refuse > + * to load any capsules with that flag. > + */ > + if (flags & EFI_CAPSULE_INITIATE_RESET) > + return -EINVAL; > + Should we perhaps whitelist rather than blacklist these flags? If a 'EFI_CAPSULE_INITIATE_RESET_TOO' surfaces at some point, or flags that do other nasty things, at least we won't be caught off guard. > capsule = kmalloc(sizeof(*capsule), GFP_KERNEL); > if (!capsule) > return -ENOMEM;