Re: efivarfs: fix error on write to new variable leaving remnants

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Wed, 2025-02-26 at 12:30 +0100, Ard Biesheuvel wrote:
> On Tue, 25 Feb 2025 at 13:59, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
[...]
> > Reading the code in the fix, it looks like you were creating a file
> > in EFI (which is naturally zero length), then closing it (because
> > glib gio specifically has an API for this), then clearing the
> > immutable bit and then writing to it to actually create a variable?
> > 
> > However, none of that dance is at all required.  A newly created
> > file naturally allows writing on the file descriptor you used to
> > create it. It's only if you open it again that the entry becomes
> > immutable.
> 
> Should we be doing this in the first place? It makes sense for
> efivarfs to mark variables that it doesn't know about as immutable if
> they are present at boot - the original rationale here was systems
> with buggy firmware, where an inadvertent 'rm -r
> /sys/firmware/efivars/*' would brick the device.
> 
> But the fact that a newly created file becomes immutable once you
> close and reopen it seems unnecessarily strict to me.
> 
> (I am aware that this is orthogonal to the issue at hand, but it is
> something we might fix nonetheless)

Well, I've always thought it was silly ever since it was introduced:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ed8b0de5a33d2a2557dce7f9429dca8cb5bc5879

The rationale it was because root could brick their system by doing rm
-fr never made any sense, since if root is going to do rm -fr they're
asking to become unbootable anyway.

All of us who have code that manipulates the variables simply added
remove immutable flag code like the above.  If it does get removed,
efivarfs will still need to respond to get/setfsxattr though to make
sure that code keeps working, so it can't be a clean revert.

Regards,

James