Re: [PATCH] ovl: use linked upper dentry in copy-up tmpfile
Amir Goldstein <[email protected]> Sun, 3 May 2026 22:37:27 +0200
| Newsgroups | org.kernel.vger.linux-unionfs,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <CAOQ4uxh6YUJEh75sASqi1gOaQYKpWhftz4to1j8s2Y9Jef-XQg@mail.gmail.com> |
On Sat, May 2, 2026 at 1:27 AM Souvik Banerjee <[email protected]> wrote: > > ovl_copy_up_tmpfile() stores the disconnected O_TMPFILE dentry as the > overlay's upper dentry reference via ovl_inode_update(). vfs_tmpfile() > allocated this dentry via d_alloc(parentpath->dentry, &slash_name), so > d_name is "/" and d_parent is c->workdir. Local upper filesystems > (ext4, btrfs, xfs, ...) immediately rename it to "#<inum>" via > d_mark_tmpfile() inside their ->tmpfile() op; FUSE and virtiofs do > not, so both fields stay that way. Neither identifies the destination > directory and filename where ovl_do_link() actually linked the file. > > When the upper filesystem implements ->d_revalidate() (e.g. FUSE or > virtiofs), ovl_revalidate_real() calls it with the dentry's parent > inode and a snapshot of d_name. The server tries to look up "/" inside > c->workdir, fails, and overlayfs reports -ESTALE. > > This causes persistent ESTALE errors for any file that was copied up via > the tmpfile path, breaking dpkg, apt, and other tools that do > rename-over-existing on overlayfs with a FUSE/virtiofs upper. > > Before commit 6b52243f633e ("ovl: fold copy-up helpers into callers"), > the tmpfile copy-up path used a dedicated helper ovl_link_tmpfile() > that captured the linked destination dentry returned by ovl_do_link(): > > err = ovl_do_link(temp, udir, upper); > ... > if (!err) > *newdentry = dget(upper); > > and published it via ovl_inode_update(d_inode(c->dentry), newdentry). > The fold inlined ovl_do_link() into ovl_copy_up_tmpfile() but dropped > the dget(upper) capture, and rewrote the publish line as > ovl_inode_update(d_inode(c->dentry), dget(temp)) — where temp is the > disconnected O_TMPFILE dentry. > > Fix by keeping a reference to the linked destination dentry after > ovl_do_link() succeeds, and publishing that dentry at the existing > ovl_inode_update() call site. The non-tmpfile/workdir path continues to > publish the renamed temporary dentry. > > Reproducer: > - Mount overlayfs with virtiofs (or a FUSE fs whose server advertises > FUSE_TMPFILE) as upper > - Run: dpkg -i <any .deb> > - Observe: "error installing new file '...': Stale file handle" > > Fixes: 6b52243f633e ("ovl: fold copy-up helpers into callers") > Cc: [email protected] # v4.20+ > Signed-off-by: Souvik Banerjee <[email protected]> > --- > fs/overlayfs/copy_up.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c > index 13cb60b52bd6..e963701b4c87 100644 > --- a/fs/overlayfs/copy_up.c > +++ b/fs/overlayfs/copy_up.c > @@ -853,7 +853,7 @@ static int ovl_copy_up_tmpfile(struct ovl_copy_up_ctx *c) > { > struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb); > struct inode *udir = d_inode(c->destdir); > - struct dentry *temp, *upper; > + struct dentry *temp, *upper, *newdentry = NULL; > struct file *tmpfile; > int err; > > @@ -889,6 +889,14 @@ static int ovl_copy_up_tmpfile(struct ovl_copy_up_ctx *c) > err = PTR_ERR(upper); > if (!IS_ERR(upper)) { > err = ovl_do_link(ofs, temp, udir, upper); > + if (!err) { > + /* > + * Record the linked dentry -- not the disconnected > + * O_TMPFILE dentry -- so that ->d_revalidate() on > + * the upper fs sees the real parent/name. > + */ > + newdentry = dget(upper); > + } > end_creating(upper); > } > > @@ -903,7 +911,7 @@ static int ovl_copy_up_tmpfile(struct ovl_copy_up_ctx *c) > > if (!c->metacopy) > ovl_set_upperdata(d_inode(c->dentry)); > - ovl_inode_update(d_inode(c->dentry), dget(temp)); > + ovl_inode_update(d_inode(c->dentry), newdentry); > > out: > ovl_end_write(c->dentry); > -- > 2.51.1 > Hi Souvik, Thank you for the analysis and the fix. Looks correct to me. Reviewed-by: Amir Goldstein <[email protected]> Christian, Could you pick this up for vfs-fixes? I do not have any other ovl fixes queued up. Thanks, Amir.