Re: [PATCH 1/2] fs/efivarfs: restrict inode permissions

[Linus Torvalds]

On Tue, Feb 20, 2018 at 1:18 PM, Luck, Tony <tony.luck@xxxxxxxxx> wrote:
> ...
>> > -   inode = efivarfs_get_inode(sb, d_inode(root), S_IFREG | 0644, 0,
>> > +   inode = efivarfs_get_inode(sb, d_inode(root), S_IFREG | 0600, 0,
>> >                                is_removable);
>
> Linus,
>
> Does this rate an exception to the "don't break userspace" for a security issue?

I have a hard time judging, since I don't know what the breakage is.

_Theoretical_ breakage doesn't matter.

But yes, if it's actually part of some regular use flow, then it
matters, and we should not do this change. It's not a real security
issue, afaik.

I do think that we might need to just extend on the whitelist for efi
variables we _already_ have for other reasons. I'm talking about that
whole variable_validate[] array.

Which variables are actually used by user space tools? It sounds like
it may be exactly those boot order things that we already have flags
and special behavior for.

And no, I don't believe in the "SMI can trigger a DoS" argument. Those
garbage efivars that *do* trigger SMI's should me marked and shamed,
and maybe _they_ need to be added to the list of things that you
should look out for. Root or not.

And just on general principlies, I don't want to see weasel-wordy
commit messages like

 "Reading certain EFI variables trigger SMIs"

I understand *writing* them causing SMI's due to some flash protection
scheme. What's the reading thing? And why aren't we calling that
garbage out?

So even if we do need to limit reading, I want out comments (in core
or commits) to actually explain *Why*., instead of this kind of
non-specific fear-mongering that nobody can really say yay or nay
about.

               Linus