linux-kernel - Re: [linux-next:master] [VFS/nfsd/cachefiles/ovl] 7ab96df840: WARNING:at_fs/dcache.c:#umount

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] [day] [month] [year] [list]

Message-id: <176424141191.634289.800675023484871934@noble.neil.brown.name>
Date: Thu, 27 Nov 2025 22:03:31 +1100
From: NeilBrown <neilb@...mail.net>
To: "Christian Brauner" <brauner@...nel.org>
Cc: "Amir Goldstein" <amir73il@...il.com>, "Jeff Layton" <jlayton@...nel.org>,
 "kernel test robot" <oliver.sang@...el.com>, oe-lkp@...ts.linux.dev,
 lkp@...el.com, netfs@...ts.linux.dev, linux-fsdevel@...r.kernel.org,
 linux-nfs@...r.kernel.org, linux-unionfs@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [linux-next:master] [VFS/nfsd/cachefiles/ovl] 7ab96df840:
 WARNING:at_fs/dcache.c:#umount_check

On Thu, 27 Nov 2025, Christian Brauner wrote:
> On Thu, Nov 27, 2025 at 07:51:18AM +1100, NeilBrown wrote:
> > On Wed, 26 Nov 2025, Amir Goldstein wrote:
> > > On Wed, Nov 26, 2025 at 11:42 AM Christian Brauner <brauner@...nel.org> wrote:
> > > >
> > > > On Tue, Nov 25, 2025 at 09:48:18PM +0800, kernel test robot wrote:
> > > > >
> > > > > Hello,
> > > > >
> > > > > kernel test robot noticed "WARNING:at_fs/dcache.c:#umount_check" on:
> > > > >
> > > > > commit: 7ab96df840e60eb933abfe65fc5fe44e72f16dc0 ("VFS/nfsd/cachefiles/ovl: add start_creating() and end_creating()")
> > > > > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
> > > > >
> > > > > [test failed on linux-next/master d724c6f85e80a23ed46b7ebc6e38b527c09d64f5]
> > > >
> > > > Neil, can you please take a look at this soon?
> > > > I plan on sending the batch of PRs for this cycle on Friday.
> > > >
> > > > >
> > > > > in testcase: filebench
> > > > > version: filebench-x86_64-22620e6-1_20251009
> > > > > with following parameters:
> > > > >
> > > > >       disk: 1SSD
> > > > >       fs: ext4
> > > > >       fs2: nfsv4
> > > > >       test: ratelimcopyfiles.f
> > > > >       cpufreq_governor: performance
> > > > >
> > > 
> > > Test is copying to nfsv4 so that's the immediate suspect.
> > > WARN_ON is in unmount of ext4, but I suspect that nfs
> > > was loop mounted for the test.
> > > 
> > > FWIW, nfsd_proc_create() looks very suspicious.
> > > 
> > > nfsd_create_locked() does end_creating() internally (internal API change)
> > > but nfsd_create_locked() still does end_creating() regardless.
> > 
> > Thanks for looking at this Amir.  That omission in nfsproc.c is
> > certainly part of the problem but not all of it.
> > By skipping the end_creating() there, we avoid a duplicate unlock, but
> > also lose a dput() which we need.  Both callers of nfsd_create_locked()
> > have the same problem.
> > I think this should fix it.  The resulting code is a bit ugly but I can
> > fix that with the nfsd team once this gets upstream.
> > 
> > (FYI nfsd_proc_create() is only used for NFSv2 and as it was an nfsv4 test,
> >  that could wouldn't have been run)
> > 
> > Thanks,
> > NeilBrown
> > 
> > diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
> > index 28f03a6a3cc3..481e789a7697 100644
> > --- a/fs/nfsd/nfsproc.c
> > +++ b/fs/nfsd/nfsproc.c
> > @@ -407,6 +407,9 @@ nfsd_proc_create(struct svc_rqst *rqstp)
> >  		/* File doesn't exist. Create it and set attrs */
> >  		resp->status = nfsd_create_locked(rqstp, dirfhp, &attrs, type,
> >  						  rdev, newfhp);
> > +		/* nfsd_create_locked() unlocked the parent */
> > +		dput(dchild);
> > +		goto out_write;
> >  	} else if (type == S_IFREG) {
> >  		dprintk("nfsd:   existing %s, valid=%x, size=%ld\n",
> >  			argp->name, attr->ia_valid, (long) attr->ia_size);
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 145f1c8d124d..4688f3fd59e2 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -1633,16 +1633,14 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >  		return nfserrno(host_err);
> >  
> >  	err = fh_compose(resfhp, fhp->fh_export, dchild, fhp);
> > -	/*
> > -	 * We unconditionally drop our ref to dchild as fh_compose will have
> > -	 * already grabbed its own ref for it.
> > -	 */
> >  	if (err)
> >  		goto out_unlock;
> >  	err = fh_fill_pre_attrs(fhp);
> >  	if (err != nfs_ok)
> >  		goto out_unlock;
> >  	err = nfsd_create_locked(rqstp, fhp, attrs, type, rdev, resfhp);
> > +	/* nfsd_create_locked() unlocked the parent */
> > +	dput(dchild);
> >  	return err;
> >  
> >  out_unlock:
> 
> Thanks for the quick fix. I've added a patch to
> vfs-6.19.directory.unlocking which I attributed to you.
> It'd be easier if you just shoot something I can apply directly next
> time. :)
> 

Thanks looks good ( on vfs-6.19.directory.locking of course not
unlocking :-)

Though I prefer
Signed-off-by: NeilBrown <neil@...wn.name>

(I received mail at that address but cannot send it because of SPF
 silliness at brown.name).

I'll make sure it is a properly formatted patch if there is a next
time.

Thanks,
NeilBrown