On Wed, 2024-12-11 at 12:16 +0100, Christian Brauner wrote:
> On Tue, Dec 10, 2024 at 12:02:24PM -0500, James Bottomley wrote:
[...]
> > +static int efivarfs_file_release(struct inode *inode, struct file
> > *file)
> > +{
> > + if (i_size_read(inode) == 0) {
> > + drop_nlink(inode);
> > + d_delete(file->f_path.dentry);
> > + dput(file->f_path.dentry);
> > + }
>
> Without wider context the dput() looks UAF-y as __fput() will do:
>
> struct dentry *dentry = file->f_path.dentry;
> if (file->f_op->release)
> file->f_op->release(inode, file);
> dput(dentry);
>
> Is there an extra reference on file->f_path.dentry taken somewhere?
Heh, well, this is why I cc'd fsdevel to make sure I got all the fs
bits I used to be familiar with, but knowledge of which has atrophied,
correct.
I think it's paired with the extra dget() just after d_instantiate() in
fs/efivarfs/inode.c:efivarfs_create(). The reason being this is a
pseudo-filesystem so all the dentries representing objects have to be
born with a positive reference count to prevent them being reclaimed
under memory pressure.
Regards,
James
