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

[Luck, Tony]

On Tue, Feb 20, 2018 at 11:18:57AM -0800, Andy Lutomirski wrote:
> On 02/15/2018 10:22 AM, Joe Konno wrote:
> > From: Joe Konno <joe.konno@xxxxxxxxx>
> > 
> > Efivarfs nodes are created with group and world readable permissions.
> > Reading certain EFI variables trigger SMIs. So, this is a potential DoS
> > surface.
> > 
> > Make permissions more restrictive-- only the owner may read or write to
> > created inodes.
...
> The discussion in this thread has gone on too long, so:
> 
> Acked-by: Andy Lutomirski <luto@xxxxxxxxxx>
> 
> And yes, this patch will break a couple of minor usecases, but IMO those
> usecases deserve to break.
...
> > -	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?

What breaks:
User can't run efibootmgr(8) to see things like BootOrder. Also
"fwupdate", "dbxtool", "mokutil", and "tpmtotp" have some modes
where ordinary users need read access to some EFI variables.

We looked at some other options.

1) When mounting efivarfs have it read all the variables and
   cache the values. Then user can read without making an EFI
   call because we just copyout the cached copy.
   Rejected as there can be a lot of variables (70 on Peter Jones
   system) and EFI dropped the 1KB per variable limit. So this
   pins a bunch of memory for a few obscure use cases.

2) Rate limit EFI calls for non-root
   This solution still has some cheer-leaders. Obviously a bit
   more code than just changing the permissions. But would also
   preemptively fix any other places where an ordinary user can
   trigger an EFI call.

-Tony