Re: [PATCH 0/2] efivars: reading variables can generate SMIs

[Peter Jones]

On Fri, Feb 16, 2018 at 07:32:17PM +0000, Luck, Tony wrote:
> > tl;dr: I think changing everything to 0600 is probably completely fine,
> > and whitelisting is probably pointless.  
> 
> But do you speak for all users?

No, I just write their tools :)

> It will just take one person complaining that efibootmgr no longer
> shows them what it used to show to bring down the wrath of Linus on
> our (specifically Joe's) head for breaking user space.

The userland use case is gazing idly at the values without intending to
do anything about them.  And most of this is firmware config and
firmware/OS interaction.  Honestly it should never have been user
readable to begin with.

But also, we had a bug for quite some time where efibootmgr created
everything as 0600, and as a result non-root users couldn't do e.g.
"efibootmgr -v" and see the paths of new entries until a reboot
occurred.  Nobody ever reported it in bugzilla.redhat.com or efibootmgr
or efivar's github issues pages.  One person noticed it while commenting
about another issue, but didn't see it as related to his actual issue or
being a bug so much as "weird" that listing worked as non-root before
changing something but not after.

Another user asked about getting permission denied while setting the
boot order on askubuntu here:
https://askubuntu.com/questions/688317/getting-permission-denied-errors-from-efibootmgr
The response was exactly that you have to run it as root.  I think it's
telling that nobody said anything about reading vs writing.

> I've got someone about to start looking at making efivarfs read and save
> the values during mount, so all the read-only options can continue to work
> without making EFI calls.
> 
> This will cost some memory (say 20-30 variables at up to 1K each).

71 variables on my laptop, and the 1K restriction went away a *loooong*
time ago.  It was fully excised from the userland tools in 2013.  On my
laptop, 4 of those 71 variables are >5000 bytes.  The total storage of
all of the data in the variables is 38kB.

I still think changing it to 0600 and calling this done is the right
thing to do.

-- 
  Peter