linux-kernel - Re: [PATCH] ovl: fix UAF in ovl_dentry_update_reval by moving dput() in ovl_link

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOQ4uxhuN8Bs7yRDKtrdwixzU-T4-fN-SQSRvqT4hEbu0iDF8A@mail.gmail.com>
Date: Sat, 15 Feb 2025 10:30:55 +0100
From: Amir Goldstein <amir73il@...il.com>
To: Vasiliy Kovalev <kovalev@...linux.org>, Christian Brauner <brauner@...nel.org>
Cc: Miklos Szeredi <miklos@...redi.hu>, Gao Xiang <xiang@...nel.org>, linux-unionfs@...r.kernel.org, 
	linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org, 
	syzbot+316db8a1191938280eb6@...kaller.appspotmail.com
Subject: Re: [PATCH] ovl: fix UAF in ovl_dentry_update_reval by moving dput()
 in ovl_link_up

On Fri, Feb 14, 2025 at 10:51 PM Vasiliy Kovalev <kovalev@...linux.org> wrote:
>
> The issue was caused by dput(upper) being called before
> ovl_dentry_update_reval(), while upper->d_flags was still
> accessed in ovl_dentry_remote().
>
> Move dput(upper) after its last use to prevent use-after-free.
>
> BUG: KASAN: slab-use-after-free in ovl_dentry_remote fs/overlayfs/util.c:162 [inline]
> BUG: KASAN: slab-use-after-free in ovl_dentry_update_reval+0xd2/0xf0 fs/overlayfs/util.c:167
>
> Call Trace:
>  <TASK>
>  __dump_stack lib/dump_stack.c:88 [inline]
>  dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:114
>  print_address_description mm/kasan/report.c:377 [inline]
>  print_report+0xc3/0x620 mm/kasan/report.c:488
>  kasan_report+0xd9/0x110 mm/kasan/report.c:601
>  ovl_dentry_remote fs/overlayfs/util.c:162 [inline]
>  ovl_dentry_update_reval+0xd2/0xf0 fs/overlayfs/util.c:167
>  ovl_link_up fs/overlayfs/copy_up.c:610 [inline]
>  ovl_copy_up_one+0x2105/0x3490 fs/overlayfs/copy_up.c:1170
>  ovl_copy_up_flags+0x18d/0x200 fs/overlayfs/copy_up.c:1223
>  ovl_rename+0x39e/0x18c0 fs/overlayfs/dir.c:1136
>  vfs_rename+0xf84/0x20a0 fs/namei.c:4893
> ...
>  </TASK>
>
> Fixes: b07d5cc93e1b ("ovl: update of dentry revalidate flags after copy up")
> Reported-by: syzbot+316db8a1191938280eb6@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=316db8a1191938280eb6
> Signed-off-by: Vasiliy Kovalev <kovalev@...linux.org>

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

Christian,

Could you pick this up via vfs.fixes?

Thanks,
Amir.

> ---
>  fs/overlayfs/copy_up.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 0c28e5fa34077..d7310fcf38881 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -618,7 +618,6 @@ static int ovl_link_up(struct ovl_copy_up_ctx *c)
>         err = PTR_ERR(upper);
>         if (!IS_ERR(upper)) {
>                 err = ovl_do_link(ofs, ovl_dentry_upper(c->dentry), udir, upper);
> -               dput(upper);
>
>                 if (!err) {
>                         /* Restore timestamps on parent (best effort) */
> @@ -626,6 +625,7 @@ static int ovl_link_up(struct ovl_copy_up_ctx *c)
>                         ovl_dentry_set_upper_alias(c->dentry);
>                         ovl_dentry_update_reval(c->dentry, upper);
>                 }
> +               dput(upper);
>         }
>         inode_unlock(udir);
>         if (err)
> --
> 2.42.2
>