Re: [PATCH 1/2] fs: unreference path on file close

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi,

On 6/11/25 13:56, Sascha Hauer wrote:
> Since commit 83ffe89840e8 ("fs: use filename_create/filename_lookup
> instead of open-coding") we no longer open-code the file lookup during
> openat(). Unlike the open-coded variant filename_lookup() keeps a
> reference to the file which is now leaked and the filesystem cannot be
> unmounted anymore:
> 
> umount: Device or resource busy
> 
> Since commit a1da0079d162 ("fs: implement opendir in terms of fdopendir")
> this also happens when just a directory is opened.
> 
> The original code was not quite correct: After calling terminate_walk()
> we no longer held a reference to the path for an opened file. Transfer
> the path to the already existing f_path in struct file and unreference
> it a close() time.

Unfortunately, this introduced a regression to v2025.06.0:

Before:

  barebox@TI AM335x BeagleBone black:/ rm /mnt/mmc0.0/barebox.env
  barebox@TI AM335x BeagleBone black:/ saveenv
  saving environment to /mnt/mmc0.0/barebox.env
  barebox@TI AM335x BeagleBone black:/ saveenv
  saving environment to /mnt/mmc0.0/barebox.env

After:

  barebox@TI AM335x BeagleBone black:/ rm /mnt/mmc0.0/barebox.env
  barebox@TI AM335x BeagleBone black:/ saveenv
  saving environment to /mnt/mmc0.0/barebox.env
  could not open /mnt/mmc0.0/barebox.env: Invalid argument
  saveenv: Invalid argument
  barebox@TI AM335x BeagleBone black:/ saveenv
  saving environment to /mnt/mmc0.0/barebox.env

The -EINVAL comes from fat_open being called with / as filename.
I am not yet sure what the proper fix is. At least we have a test
now to avoid the old regression from reocurring...

Cheers,
Ahmad

> 
> Fixes: 83ffe89840e8 ("fs: use filename_create/filename_lookup instead of open-coding")
> Fixes: a1da0079d162 ("fs: implement opendir in terms of fdopendir")
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  fs/fs.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fs.c b/fs/fs.c
> index d401268c1e..18343552a7 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -330,7 +330,7 @@ static void put_file(struct file *f)
>  	f->path = NULL;
>  	f->fsdev = NULL;
>  	iput(f->f_inode);
> -	dput(f->f_dentry);
> +	path_put(&f->f_path);
>  }
>  
>  static struct file *fd_to_file(int fd, bool o_path_ok)
> @@ -2550,7 +2550,7 @@ int openat(int dirfd, const char *pathname, int flags)
>  	int error = 0;
>  	struct inode *inode = NULL;
>  	struct dentry *dentry = NULL;
> -	struct path path;
> +	struct path path = {};
>  
>  	if (flags & O_TMPFILE) {
>  		error = filename_lookup(dirfd, getname(pathname), LOOKUP_DIRECTORY, &path);
> @@ -2628,11 +2628,12 @@ int openat(int dirfd, const char *pathname, int flags)
>  	f = get_file(fsdev);
>  	if (!f) {
>  		error = -EMFILE;
> +		path_put(&path);
>  		goto out1;
>  	}
>  
>  	f->path = dpath(dentry, d_root);
> -	f->f_dentry = dentry;
> +	f->f_path = path;
>  	f->f_inode = iget(inode);
>  	f->f_flags = flags;
>  

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |