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

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

[Sascha Hauer]

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.

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;
 
-- 
2.39.5

[PATCH 2/2] fs: unreference path in canonicalize_path()

[Sascha Hauer]

canonicalize_path() missed to unreference the path after usage, so the
filesystem containing *pathname can't be unmounted anymore. This happens
for example on a 'findmnt /mnt/mmc2.1' after which /mnt/mmc2.1 cannot be
unmounted.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/fs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/fs.c b/fs/fs.c
index 18343552a7..8689e14e68 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -2996,6 +2996,8 @@ char *canonicalize_path(int dirfd, const char *pathname)
 		goto out;
 
 	res = dpath(path.dentry, d_root);
+
+	path_put(&path);
 out:
 	errno_set(ret);
 	return res;
-- 
2.39.5

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

[Sascha Hauer]

On Wed, 11 Jun 2025 13:56:34 +0200, 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
> 
> [...]

Applied, thanks!

[1/2] fs: unreference path on file close
      https://git.pengutronix.de/cgit/barebox/commit/?id=a3ef153fdf26 (link may not be stable)
[2/2] fs: unreference path in canonicalize_path()
      https://git.pengutronix.de/cgit/barebox/commit/?id=aef3b0134519 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

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

[Ahmad Fatoum]

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 |