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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Feb 2018 14:01:51 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     "Luck, Tony" <tony.luck@...el.com>
Cc:     Joe Konno <joe.konno@...ux.intel.com>, linux-efi@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Matthew Garrett <matthew.garrett@...ula.com>,
        Jeremy Kerr <jk@...abs.org>, Andi Kleen <ak@...ux.intel.com>,
        Matthew Garrett <mjg59@...gle.com>,
        Peter Jones <pjones@...hat.com>,
        Andy Lutomirski <luto@...nel.org>,
        James Bottomley <james.bottomley@...senpartnership.com>
Subject: Re: [PATCH 1/2] fs/efivarfs: restrict inode permissions

On Tue, Feb 20, 2018 at 1:18 PM, Luck, Tony <tony.luck@...el.com> 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