[PATCH] ovl: fail ovl_lock_rename_workdir() if either target is unhashed

NeilBrown posted 1 patch 3 days, 19 hours ago
fs/overlayfs/util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] ovl: fail ovl_lock_rename_workdir() if either target is unhashed
Posted by NeilBrown 3 days, 19 hours ago

From: NeilBrown <neil@brown.name>

As well as checking that the parent hasn't changed after getting the
lock we need to check that the dentry hasn't been unhashed.
Otherwise we might try to rename something that has been removed.

Reported-by: syzbot+bfc9a0ccf0de47d04e8c@syzkaller.appspotmail.com
Fixes: d2c995581c7c ("ovl: Call ovl_create_temp() without lock held.")
Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/overlayfs/util.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index f76672f2e686..82373dd1ce6e 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1234,9 +1234,9 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work,
 		goto err;
 	if (trap)
 		goto err_unlock;
-	if (work && work->d_parent != workdir)
+	if (work && (work->d_parent != workdir || d_unhashed(work)))
 		goto err_unlock;
-	if (upper && upper->d_parent != upperdir)
+	if (upper && (upper->d_parent != upperdir || d_unhashed(upper)))
 		goto err_unlock;
 
 	return 0;
-- 
2.50.0.107.gf914562f5916.dirty
Re: [PATCH] ovl: fail ovl_lock_rename_workdir() if either target is unhashed
Posted by Amir Goldstein 3 days, 11 hours ago
On Fri, Nov 28, 2025 at 2:22 AM NeilBrown <neilb@ownmail.net> wrote:
>
>
> From: NeilBrown <neil@brown.name>
>
> As well as checking that the parent hasn't changed after getting the
> lock we need to check that the dentry hasn't been unhashed.
> Otherwise we might try to rename something that has been removed.
>
> Reported-by: syzbot+bfc9a0ccf0de47d04e8c@syzkaller.appspotmail.com
> Fixes: d2c995581c7c ("ovl: Call ovl_create_temp() without lock held.")
> Signed-off-by: NeilBrown <neil@brown.name>

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  fs/overlayfs/util.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index f76672f2e686..82373dd1ce6e 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -1234,9 +1234,9 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work,
>                 goto err;
>         if (trap)
>                 goto err_unlock;
> -       if (work && work->d_parent != workdir)
> +       if (work && (work->d_parent != workdir || d_unhashed(work)))
>                 goto err_unlock;
> -       if (upper && upper->d_parent != upperdir)
> +       if (upper && (upper->d_parent != upperdir || d_unhashed(upper)))
>                 goto err_unlock;
>
>         return 0;
> --
> 2.50.0.107.gf914562f5916.dirty
>
Re: [PATCH] ovl: fail ovl_lock_rename_workdir() if either target is unhashed
Posted by Christian Brauner 3 days, 11 hours ago
On Fri, 28 Nov 2025 12:22:35 +1100, NeilBrown wrote:
> As well as checking that the parent hasn't changed after getting the
> lock we need to check that the dentry hasn't been unhashed.
> Otherwise we might try to rename something that has been removed.
> 
> 

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] ovl: fail ovl_lock_rename_workdir() if either target is unhashed
      https://git.kernel.org/vfs/vfs/c/4ef470912f91
Re: [PATCH] ovl: fail ovl_lock_rename_workdir() if either target is unhashed
Posted by Hillf Danton 3 days, 14 hours ago
#syz test upstream master

From: NeilBrown <neil@brown.name>

As well as checking that the parent hasn't changed after getting the
lock we need to check that the dentry hasn't been unhashed.
Otherwise we might try to rename something that has been removed.

Reported-by: syzbot+bfc9a0ccf0de47d04e8c@syzkaller.appspotmail.com
Fixes: d2c995581c7c ("ovl: Call ovl_create_temp() without lock held.")
Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/overlayfs/util.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index f76672f2e686..82373dd1ce6e 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1234,9 +1234,9 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work,
 		goto err;
 	if (trap)
 		goto err_unlock;
-	if (work && work->d_parent != workdir)
+	if (work && (work->d_parent != workdir || d_unhashed(work)))
 		goto err_unlock;
-	if (upper && upper->d_parent != upperdir)
+	if (upper && (upper->d_parent != upperdir || d_unhashed(upper)))
 		goto err_unlock;

 	return 0;
--
2.50.0.107.gf914562f5916.dirty
Re: [syzbot] [overlayfs?] WARNING in shmem_unlink (2)
Posted by syzbot 3 days, 13 hours ago
Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+bfc9a0ccf0de47d04e8c@syzkaller.appspotmail.com
Tested-by: syzbot+bfc9a0ccf0de47d04e8c@syzkaller.appspotmail.com

Tested on:

commit:         e538109a Merge tag 'drm-fixes-2025-11-28' of https://g..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=17bc7e12580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=38a0c4cddc846161
dashboard link: https://syzkaller.appspot.com/bug?extid=bfc9a0ccf0de47d04e8c
compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
patch:          https://syzkaller.appspot.com/x/patch.diff?x=17af2e12580000

Note: testing is done by a robot and is best-effort only.