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

Al Viro
2025-01-16T18:59:52Z

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...