On Mon, 2024-12-23 at 20:05 +0000, Al Viro wrote:
> On Mon, Dec 23, 2024 at 02:52:12PM -0500, James Bottomley wrote:
> >
> > +static int efivarfs_file_release(struct inode *inode, struct file
> > *file)
> > +{
> > + inode_lock(inode);
> > + if (i_size_read(inode) == 0 && !d_unhashed(file-
> > >f_path.dentry)) {
> > + drop_nlink(inode);
> > + d_delete(file->f_path.dentry);
> > + dput(file->f_path.dentry);
> > + }
> > + inode_unlock(inode);
> > + return 0;
> > +}
>
> This is wrong; so's existing logics for removal from write(). Think
> what happens if you open the sucker, have something bound on top of
> it and do that deleting write().
>
> Let me look into that area...
I thought about this some more. I could see a twisted container use
case where something like this might happen (expose some but not all
efi variables to the container).
So, help me understand the subtleties here. If it's the target of a
bind mount, that's all OK, because you are allowed to delete the
target. If something is bind mounted on to an efivarfs object, the
is_local_mountpoint() check in vfs_unlink would usually trip and
prevent deletion (so the subtree doesn't become unreachable). If I
were to duplicate that, I think the best way would be simply to do a
d_put() in the file->release function and implement drop_nlink() in
d_prune (since last put will always call __dentry_kill)?
Regards,
James
