On Tue, 2024-12-24 at 04:44 +0000, Al Viro wrote:
> On Mon, Dec 23, 2024 at 11:04:58PM -0500, James Bottomley wrote:
>
> > +static int efivarfs_file_release(struct inode *inode, struct file
> > *file)
> > +{
> > + if (i_size_read(inode) == 0)
> > + simple_recursive_removal(file->f_path.dentry,
> > NULL);
> > +
> > + return 0;
> > +}
>
> What happens if you have
>
> fd = creat(name, 0700);
> fd2 = open(name, O_RDONLY);
> close(fd2);
> write(fd, "barf", 4);
>
> or, better yet, if open()/close() pair happens in an unrelated thread
> poking around?
According to my tests you get -EIO to the last write. I could be glib
and point out that a write of "barf" would return EINVAL anyway, but
assuming it's correctly formatted, you'll get -EIO from the d_unhashed
check before the variable gets created. If you want to check this
yourself, useful writes that will create a variable are:
echo 0700000054|xxd -r -p > name
And to delete it:
echo 07000000|xxd -r -p > name
You can check your above scenario from a shell with
{ sleep 10; echo 0700000054|xxd -r -p; } > name &
cat name
> I mean, having that logics in ->release() feels very awkward...
>
> For that matter, what about
> fd = creat(name, 0700);
> fd2 = open(name, O_RDWR);
> close(fd);
> write(fd2, "barf", 4);
Same thing: -EIO to last write.
> I'm not asking about the implementation; what behaviour do you want
> to see in userland?
Given the fact that very few things actually manipulate efi variables
and when they do they perform open/write/close in quick succession to
set or remove variables, I think the above behaviour is consistent with
the use and gets rid of the ghost files problem and won't cause any
additional issues. On the other hand the most intuitive thing would be
to remove zero length files on last close, not first, so if you have a
thought on how to do that easily, I'm all ears.
Regards,
James
