linux-kernel - Re: [PATCH v4 07/14] VFS: introduce start_removing

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: <176186656376.1793333.1075264554692169239@noble.neil.brown.name>
Date: Fri, 31 Oct 2025 10:22:43 +1100
From: NeilBrown <neilb@...mail.net>
To: "Al Viro" <viro@...iv.linux.org.uk>
Cc: "Christian Brauner" <brauner@...nel.org>,
 "Amir Goldstein" <amir73il@...il.com>, "Jan Kara" <jack@...e.cz>,
 linux-fsdevel@...r.kernel.org, "Jeff Layton" <jlayton@...nel.org>,
 "Chris Mason" <clm@...com>, "David Sterba" <dsterba@...e.com>,
 "David Howells" <dhowells@...hat.com>,
 "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>,
 "Danilo Krummrich" <dakr@...nel.org>, "Tyler Hicks" <code@...icks.com>,
 "Miklos Szeredi" <miklos@...redi.hu>,
 "Chuck Lever" <chuck.lever@...cle.com>,
 "Olga Kornievskaia" <okorniev@...hat.com>,
 "Dai Ngo" <Dai.Ngo@...cle.com>, "Namjae Jeon" <linkinjeon@...nel.org>,
 "Steve French" <smfrench@...il.com>,
 "Sergey Senozhatsky" <senozhatsky@...omium.org>,
 "Carlos Maiolino" <cem@...nel.org>,
 "John Johansen" <john.johansen@...onical.com>,
 "Paul Moore" <paul@...l-moore.com>, "James Morris" <jmorris@...ei.org>,
 "Serge E. Hallyn" <serge@...lyn.com>,
 "Stephen Smalley" <stephen.smalley.work@...il.com>,
 "Ondrej Mosnacek" <omosnace@...hat.com>,
 "Mateusz Guzik" <mjguzik@...il.com>,
 "Lorenzo Stoakes" <lorenzo.stoakes@...cle.com>,
 "Stefan Berger" <stefanb@...ux.ibm.com>,
 "Darrick J. Wong" <djwong@...nel.org>, linux-kernel@...r.kernel.org,
 netfs@...ts.linux.dev, ecryptfs@...r.kernel.org,
 linux-nfs@...r.kernel.org, linux-unionfs@...r.kernel.org,
 linux-cifs@...r.kernel.org, linux-xfs@...r.kernel.org,
 apparmor@...ts.ubuntu.com, linux-security-module@...r.kernel.org,
 selinux@...r.kernel.org
Subject: Re: [PATCH v4 07/14] VFS: introduce start_removing_dentry()

On Thu, 30 Oct 2025, Al Viro wrote:
> On Thu, Oct 30, 2025 at 10:31:07AM +1100, NeilBrown wrote:
> 
> > @@ -428,11 +429,14 @@ static bool cachefiles_invalidate_cookie(struct fscache_cookie *cookie)
> >  		if (!old_tmpfile) {
> >  			struct cachefiles_volume *volume = object->volume;
> >  			struct dentry *fan = volume->fanout[(u8)cookie->key_hash];
> > -
> > -			inode_lock_nested(d_inode(fan), I_MUTEX_PARENT);
> > -			cachefiles_bury_object(volume->cache, object, fan,
> > -					       old_file->f_path.dentry,
> > -					       FSCACHE_OBJECT_INVALIDATED);
> > +			struct dentry *obj;
> > +
> > +			obj = start_removing_dentry(fan, old_file->f_path.dentry);
> > +			if (!IS_ERR(obj))
> > +				cachefiles_bury_object(volume->cache, object,
> > +						       fan, obj,
> > +						       FSCACHE_OBJECT_INVALIDATED);
> > +			end_removing(obj);
> 
> Huh?  Where did you change cachefiles_bury_object to *not* unlock the parent?
> Not in this commit, AFAICS, and that means at least a bisection hazard around
> here...
> 
> Confused...
> 

Thanks for the review and for catching that error.
This incremental patch should fix it.

Thanks,
NeilBrown

diff --git a/fs/cachefiles/interface.c b/fs/cachefiles/interface.c
index 3f8a6f1a8fc3..a08250d244ea 100644
--- a/fs/cachefiles/interface.c
+++ b/fs/cachefiles/interface.c
@@ -436,7 +436,6 @@ static bool cachefiles_invalidate_cookie(struct fscache_cookie *cookie)
 				cachefiles_bury_object(volume->cache, object,
 						       fan, obj,
 						       FSCACHE_OBJECT_INVALIDATED);
-			end_removing(obj);
 		}
 		fput(old_file);
 	}
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index b97a40917a32..0104ac00485d 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -261,6 +261,7 @@ static int cachefiles_unlink(struct cachefiles_cache *cache,
  * - Directory backed objects are stuffed into the graveyard for userspace to
  *   delete
  * On entry dir must be locked.  It will be unlocked on exit.
+ * On entry there must be at least 2 refs on rep, one will be dropped on exit.
  */
 int cachefiles_bury_object(struct cachefiles_cache *cache,
 			   struct cachefiles_object *object,
@@ -275,12 +276,6 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,

 	_enter(",'%pd','%pd'", dir, rep);

-	/* end_removing() will dput() @rep but we need to keep
-	 * a ref, so take one now.  This also stops the dentry
-	 * being negated when unlinked which we need.
-	 */
-	dget(rep);
-
 	if (rep->d_parent != dir) {
 		end_removing(rep);
 		_leave(" = -ESTALE");
@@ -650,7 +645,6 @@ bool cachefiles_look_up_object(struct cachefiles_object *object)
 			ret = cachefiles_bury_object(volume->cache, object,
 						     fan, de,
 						     FSCACHE_OBJECT_IS_WEIRD);
-		end_removing(de);
 		dput(dentry);
 		if (ret < 0)
 			return false;
diff --git a/fs/cachefiles/volume.c b/fs/cachefiles/volume.c
index ddf95ff5daf0..90ba926f488e 100644
--- a/fs/cachefiles/volume.c
+++ b/fs/cachefiles/volume.c
@@ -64,7 +64,6 @@ void cachefiles_acquire_volume(struct fscache_volume *vcookie)
 				cachefiles_bury_object(cache, NULL, cache->store,
 						       vdentry,
 						       FSCACHE_VOLUME_IS_WEIRD);
-			end_removing(vdentry);
 			cachefiles_put_directory(volume->dentry);
 			cond_resched();
 			goto retry;