Re: [PATCH v2 6/6] efivarfs: fix error on write to new variable leaving remnants

[Al Viro <viro@zeniv.linux.org.uk>]

On Thu, Jan 16, 2025 at 01:54:44PM -0500, James Bottomley wrote:
> On Thu, 2025-01-16 at 18:45 +0000, Al Viro wrote:
> > On Mon, Jan 06, 2025 at 06:35:25PM -0800, James Bottomley wrote:
> > 
> > > +       inode_lock(inode);
> > > +       if (d_unhashed(file->f_path.dentry)) {
> > > +               /*
> > > +                * file got removed; don't allow a set.  Caused by
> > > an
> > > +                * unsuccessful create or successful delete write
> > > +                * racing with us.
> > > +                */
> > > +               bytes = -EIO;
> > > +               goto out;
> > > +       }
> > 
> > Wouldn't the check for zero ->i_size work here?  Would be easier to
> > follow...
> 
> Unfortunately not.  The pathway for creating a variable involves a call
> to efivarfs_create() (create inode op) first, which would in itself
> create a zero length file, then a call to efivarfs_file_write(), so if
> we key here on zero length we'd never be able to create new variables.
> 
> The idea behind the check is that delete could race with write and if
> so, we can't resurrect the variable once it's been unhashed from the
> directory, so we need to error out at that point.

D'oh...  Point, but it still feels as if you are misplacing the object
state here ;-/

OK, so we have
	* created, open but yet to be written into
	* live
	* removed

Might be better off with explicit state in efivar_entry...