From 65bf000962a33ec9d2488e4e1797b6cf630abcbb Mon Sep 17 00:00:00 2001 From: Chris Mason Date: Wed, 7 Oct 2026 10:34:08 +0000 Subject: [PATCH] ovl: take a snapshot of the name for copy up The copy up of a directory reads a freed dentry name when a lookup moves the directory at the same time. A pointer to the name of a dentry is only valid while something stops a move of the dentry. ovl_do_copy_up() keeps a plain copy of c->dentry->d_name in c->destname, and ovl_copy_up_start() only keeps ovl_rename() away. When a lookup finds the same directory under another name after a change to the lower layer, d_splice_alias() moves the dentry and frees its external name. With a delay in ovl_do_copy_up(), a chmod of a lower-only directory with a 60 byte name raced with a rename of that directory in the lower layer and a stat of the new name, and KASAN reported a use-after-free in lookup_noperm_common(). Fix this by using take_dentry_name_snapshot() for c->destname. Fixes: 829c28be9bb9 ("ovl: use d_splice_alias() in place of d_add() in lookup") Cc: stable@vger.kernel.org Signed-off-by: Chris Mason Signed-off-by: Josef Bacik Signed-off-by: Amir Goldstein --- fs/overlayfs/copy_up.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 92b3d08a0234..12000e45fefc 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -920,6 +920,7 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c) struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb); struct dentry *origin = c->lowerpath.dentry; struct ovl_fh *fh = NULL; + struct name_snapshot name; bool to_index = false; /* @@ -957,12 +958,15 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c) goto out_free_fh; } else { /* - * c->dentry->d_name is stabilzed by ovl_copy_up_start(), - * because if we got here, it means that c->dentry has no upper - * alias and changing ->d_name means going through ovl_rename() - * that will call ovl_copy_up() on source and target dentry. + * ovl_copy_up_start() keeps ovl_rename() away from c->dentry, + * because ovl_rename() calls ovl_copy_up() on source and target + * dentry. It does not stop d_splice_alias() from moving a + * directory that a lookup found under another name after a + * change to the lower layer. That move frees an external name, + * so take a snapshot. */ - c->destname = c->dentry->d_name; + take_dentry_name_snapshot(&name, c->dentry); + c->destname = name.name; /* * Mark parent "impure" because it may now contain non-pure * upper @@ -971,7 +975,7 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c) err = ovl_set_impure(c->parent, c->destdir); ovl_end_write(c->dentry); if (err) - goto out_free_fh; + goto out; } /* Should we copyup with O_TMPFILE or with workdir? */ @@ -1005,6 +1009,8 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c) out: if (to_index) kfree(c->destname.name); + else + release_dentry_name_snapshot(&name); out_free_fh: kfree(fh); return err;