From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751501AbeBPThE (ORCPT ); Fri, 16 Feb 2018 14:37:04 -0500 Received: from mail-io0-f195.google.com ([209.85.223.195]:36635 "EHLO mail-io0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750830AbeBPThA (ORCPT ); Fri, 16 Feb 2018 14:37:00 -0500 X-Google-Smtp-Source: AH8x226mv6wVUvzJOZ72lhglw+U6u3dEGO+EemF9iPjq3q9ZO7LC2h74n0wZ/W1XLXHPNCZFZo1K3g2xmWFCV3hDra8= MIME-Version: 1.0 In-Reply-To: <20180216192220.wljl23g533sc3oxg@redhat.com> References: <20180215182208.35003-1-joe.konno@linux.intel.com> <20180216105548.GA29042@pd.tnic> <20180216110821.GB29042@pd.tnic> <20180216184832.sqreq5zhar3jqdae@jbkonno-saint14> <20180216192220.wljl23g533sc3oxg@redhat.com> From: Ard Biesheuvel Date: Fri, 16 Feb 2018 19:31:50 +0000 Message-ID: Subject: Re: [PATCH 0/2] efivars: reading variables can generate SMIs To: Peter Jones Cc: Joe Konno , Borislav Petkov , Matthew Garrett , Ingo Molnar , Andy Lutomirski , linux-efi@vger.kernel.org, Linux Kernel Mailing List , Jeremy Kerr , Andi Kleen , Tony Luck , Benjamin Drung 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 16 February 2018 at 19:22, Peter Jones wrote: > On Fri, Feb 16, 2018 at 10:48:32AM -0800, Joe Konno wrote: >> On Fri, Feb 16, 2018 at 11:18:12AM +0000, Ard Biesheuvel wrote: >> > On 16 February 2018 at 11:08, Borislav Petkov wrote: >> > > On Fri, Feb 16, 2018 at 10:58:47AM +0000, Ard Biesheuvel wrote: >> > >> By your own reasoning above, that's a no-no as well. >> > > >> > > I'm sure we can come up with some emulation - the same way we did the >> > > BIOS emulation. >> > > >> > >> But thanks for your input. Anyone else got something constructive to contribute? >> > > >> > > The not-breaking userspace is constructive contribution. The last >> > > paragraph is my usual rant. >> > > >> > >> > Fair enough. And I am not disagreeing with you either. >> > >> > So question to Joe: is it well defined which variables may exhibit >> > this behavior? >> >> For brevity's sake, "not yet." Folks-- e.g. firmware writers and >> equipment makers-- may use SMIs more (or less) than others. So, how many >> SMIs generated by the reference shell script can, and will, vary >> platform to platform. I and my colleagues are digging into this. > > As a first guess: anything generated during boot is probably not an > SMI. Everything else is probably an SMI. In fact, I would expect that > for most systems, the entire list of things that *don't* generate an SMI > (aside from a few IBV specific variables) is all the variables in Table > 10 of the UEFI spec that don't have the NV flag. > >> > Given that UEFI variables are GUID scoped, would whitelisting certain >> > GUIDs (the ones userland currently relies on to be readable my >> > non-privileged users) and making everything else user-only solve this >> > problem as well? >> >> Perhaps. I don't want us chasing white rabbits just yet, but the >> whitelist is but one approach under consideration. We may see some other >> patches or RFCs about caching and/or shadowing variable values in >> efivarfs to reduce the number of direct EFI reads, with the goal of >> reducing how many SMIs are generated. >> >> Any obvious EFI variables that userspace tools have come to depend on-- >> those which normal, unprivileged users need to read-- are helpful inputs >> to this discussion. > > So, our big userland consumers are efibootmgr, fwupdate, and > "systemctl reboot --firmware-setup". efibootmgr and fwupdate can do the > "show the current status" half of their job as a user right now, but > they rely on root for the other half anyway. I don't think we normally > use libfwup as non-root even for reading status. So basically, the use > case from userland that this will effect looks like: > > efibootmgr -v > *scratch head* > sudo su - > efibootmgr -b 0000 -B > efibootmgr -b 0000 -c -L "fixed entry" ... > exit > > I don't feel really bad about people having to move the third line up to > the top of that. It's also not a use case you can have very much of: it > means you manually booted without any valid boot entries. fallback.efi > would have created a valid boot entry if you didn't do it manually. > > "systemctl reboot --firmware-setup" effectively runs as root in all > cases. > > The only thing we really must ensure to preserve the other workflows > is making sure creat() and open() with 0644 doesn't return an error (it > obviously won't be preserved across a reboot), because that would break > the existing tools. But I don't see anything in this patchset which > will cause that. > > tl;dr: I think changing everything to 0600 is probably completely fine, > and whitelisting is probably pointless. This is why I was leaning towards applying these patches: not breaking userland is an important rule, but it does not imply every aspect of behavior observable by userland is set in stone. In other words, I agree with Peter that making this change does not *break* userland in a way anyone is likely to care deeply about. Adding a caching layer to efivarfs for this purpose is really overkill IMHO. Also, we need this only on x86, so I'd like to see it proposed in a way that allows it to be compiled out for architectures that have no need for it.