linux-kernel - Re: [syzbot] [overlayfs?] WARNING in shmem

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: <CAOQ4uxh_yrq76Rq9RoykGdANZNBWc16UgbSBRjDtXKeLdA7-3Q@mail.gmail.com>
Date: Wed, 27 Aug 2025 20:23:58 +0200
From: Amir Goldstein <amir73il@...il.com>
To: NeilBrown <neil@...wn.name>, André Almeida <andrealmeid@...lia.com>
Cc: syzbot <syzbot+ec9fab8b7f0386b98a17@...kaller.appspotmail.com>, 
	linux-kernel@...r.kernel.org, linux-unionfs@...r.kernel.org, 
	miklos@...redi.hu, syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [overlayfs?] WARNING in shmem_unlink

On Mon, Aug 18, 2025 at 11:35 PM NeilBrown <neil@...wn.name> wrote:
>
> On Mon, 18 Aug 2025, Amir Goldstein wrote:
> > On Mon, Aug 18, 2025 at 2:34 AM NeilBrown <neil@...wn.name> wrote:
> > >
> > > On Mon, 18 Aug 2025, Amir Goldstein wrote:
> > > > Neil,
> > > >
> > > > I will have a look tomorrow.
> > > > If you have ideas I am open to hear them.
> > > > The repro is mounting overlayfs all over each other in concurrent threads
> > > > and one of the rmdir of "work" dir triggers this assertion
> > >
> > > My guess is that by dropping and retaking the lock, we open the
> > > possibility of a race so that by the time vfs_unlink() is called the
> > > dentry has already been unlinked.  In that case it would be unhashed.
> > > So after retaking the lock we need to check d_unhashed() as well as
> > > ->d_parent.
> > >
> > > So something like
> > > --- a/fs/overlayfs/util.c
> > > +++ b/fs/overlayfs/util.c
> > > @@ -1552,7 +1552,8 @@ void ovl_copyattr(struct inode *inode)
> > >  int ovl_parent_lock(struct dentry *parent, struct dentry *child)
> > >  {
> > >         inode_lock_nested(parent->d_inode, I_MUTEX_PARENT);
> > > -       if (!child || child->d_parent == parent)
> > > +       if (!child ||
> > > +           (!d_unhashed(child) && child->d_parent == parent))
> > >                 return 0;
> > >
> > >         inode_unlock(parent->d_inode);
> > >
> > >
> > > NeilBrown
> > >
> >
> > Nice!
> > I pushed this commit to ovl-fixes:
> >
> > commit c56976d86e11afcd6b23633395a7f2e6e920e42d (HEAD -> ovl-fixes)
> > Author: Amir Goldstein <amir73il@...il.com>
> > Date:   Mon Aug 18 11:23:55 2025 +0200
> >
> >     ovl: fix possible double unlink
> >
> >     commit 9d23967b18c6 ("ovl: simplify an error path in
> >     ovl_copy_up_workdir()") introduced the helper ovl_cleanup_unlocked(),
> >     which is later used in several following patches to re-acquire the parent
> >     inode lock and unlink a dentry that was earlier found using lookup.
> >     This helper was eventually renamed to ovl_cleanup().
> >
> >     The helper ovl_parent_lock() is used to re-acquire the parent inode lock.
> >     After acquiring the parent inode lock, the helper verifies that the
> >     dentry has not since been moved to another parent, but it failed to
> >     verify that the dentry wasn't unlinked from the parent.
> >
> >     This means that now every call to ovl_cleanup() could potentially
> >     race with another thread, unlinking the dentry to be cleaned up
> >     underneath overlayfs and trigger a vfs assertion.
> >
> >     Reported-by: syzbot+ec9fab8b7f0386b98a17@...kaller.appspotmail.com
> >     Tested-by: syzbot+ec9fab8b7f0386b98a17@...kaller.appspotmail.com
> >     Fixes: 9d23967b18c6 ("ovl: simplify an error path in ovl_copy_up_workdir()")
> >     Suggested-by: NeilBrown <neil@...wn.name>
> >     Signed-off-by: Amir Goldstein <amir73il@...il.com>
> >
> > Neil,
> >
> > Please review my commit message.
> > If you want me to assign you ownership please sign off on this commit message.
>
> Looks good to me.  No changes needed.

We are having some problems with this fix colliding with a new ovl feature [1].

Let's try to test this revised fix:

#syz test: https://github.com/amir73il/linux ovl_casefold

Thanks,
Amir.

[1] https://lore.kernel.org/linux-unionfs/CAOQ4uxj551a7cvjpcYEyTLtsEXw9OxHtTc-VSm170J5pWtwoUQ@mail.gmail.com/