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]
Date:   Mon, 31 Jan 2022 15:14:00 +0000
From:   David Howells <dhowells@...hat.com>
To:     Amir Goldstein <amir73il@...il.com>
Cc:     Miklos Szeredi <miklos@...redi.hu>, linux-cachefs@...hat.com,
        linux-unionfs@...r.kernel.org, dhowells@...hat.com,
        Christoph Hellwig <hch@...radead.org>,
        Miklos Szeredi <miklos@...redi.hu>,
        Jeff Layton <jlayton@...nel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        torvalds@...ux-foundation.org, linux-unionfs@...r.kernel.org,
        linux-cachefs@...hat.com, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH 4/5] cachefiles: Use I_EXCL_INUSE instead of S_KERNEL_FILE

Get rid of S_KERNEL_FILE and use I_EXCL_INUSE instead, thereby sharing that
flag with overlayfs.  This is used by cachefiles for two purposes: firstly,
to prevent simultaneous access to a backing file, which could cause data
corruption, and secondly, to allow cachefilesd to find out if it's allowed
to cull a backing file without having to have duplicate lookup
infrastructure (the VFS already has all the infrastructure that is
necessary to do the lookup; cachefiles just needs a single bit flag).

Signed-off-by: David Howells <dhowells@...hat.com>
cc: Amir Goldstein <amir73il@...il.com>
cc: Miklos Szeredi <miklos@...redi.hu>
cc: linux-cachefs@...hat.com
cc: linux-unionfs@...r.kernel.org
Link: https://lore.kernel.org/r/CAOQ4uxhRS3MGEnCUDcsB1RL0d1Oy0g0Rzm75hVFAJw2dJ7uKSA@mail.gmail.com/ [1]
---

 fs/cachefiles/namei.c |   46 ++++++++++++++++++++--------------------------
 include/linux/fs.h    |    2 +-
 2 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 8930c767d93a..0c88c82c188f 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -18,22 +18,19 @@ static bool __cachefiles_mark_inode_in_use(struct cachefiles_object *object,
 					   struct dentry *dentry)
 {
 	struct inode *inode = d_backing_inode(dentry);
-	bool can_use = false;
+	bool locked;
 
-	spin_lock(&inode->i_lock);
-	if (!(inode->i_flags & S_KERNEL_FILE)) {
-		inode->i_flags |= S_KERNEL_FILE;
+	locked = inode_excl_inuse_trylock(dentry, object ? object->debug_id : 0,
+					  inode_excl_inuse_by_cachefiles);
+	if (locked) {
+		spin_lock(&inode->i_lock);
 		inode->i_state |= I_NO_REMOVE;
-		trace_cachefiles_mark_active(object, inode);
-		can_use = true;
+		spin_unlock(&inode->i_lock);
 	} else {
-		trace_cachefiles_mark_failed(object, inode);
 		pr_notice("cachefiles: Inode already in use: %pd (B=%lx)\n",
 			  dentry, inode->i_ino);
 	}
-	spin_unlock(&inode->i_lock);
-
-	return can_use;
+	return locked;
 }
 
 static bool cachefiles_mark_inode_in_use(struct cachefiles_object *object,
@@ -57,10 +54,9 @@ static void __cachefiles_unmark_inode_in_use(struct cachefiles_object *object,
 	struct inode *inode = d_backing_inode(dentry);
 
 	spin_lock(&inode->i_lock);
-	inode->i_flags &= ~S_KERNEL_FILE;
-	inode->i_state &= ~I_NO_REMOVE;
+	inode->i_state |= I_NO_REMOVE;
 	spin_unlock(&inode->i_lock);
-	trace_cachefiles_mark_inactive(object, inode);
+	inode_excl_inuse_unlock(dentry, object ? object->debug_id : 0);
 }
 
 /*
@@ -754,7 +750,7 @@ static struct dentry *cachefiles_lookup_for_cull(struct cachefiles_cache *cache,
 		goto lookup_error;
 	if (d_is_negative(victim))
 		goto lookup_put;
-	if (d_inode(victim)->i_flags & S_KERNEL_FILE)
+	if (inode_is_excl_inuse(victim))
 		goto lookup_busy;
 	return victim;
 
@@ -790,6 +786,7 @@ int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir,
 {
 	struct dentry *victim;
 	struct inode *inode;
+	bool locked;
 	int ret;
 
 	_enter(",%pd/,%s", dir, filename);
@@ -798,19 +795,16 @@ int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir,
 	if (IS_ERR(victim))
 		return PTR_ERR(victim);
 
-	/* check to see if someone is using this object */
+	/* Check to see if someone is using this object and, if not, stop the
+	 * cache from picking it back up.
+	 */
 	inode = d_inode(victim);
 	inode_lock(inode);
-	if (inode->i_flags & S_KERNEL_FILE) {
-		ret = -EBUSY;
-	} else {
-		/* Stop the cache from picking it back up */
-		inode->i_flags |= S_KERNEL_FILE;
-		ret = 0;
-	}
+	locked = inode_excl_inuse_trylock(victim, 0,
+					  inode_excl_inuse_by_cachefiles);
 	inode_unlock(inode);
-	if (ret < 0)
-		goto error_unlock;
+	if (!locked)
+		goto busy;
 
 	ret = cachefiles_bury_object(cache, NULL, dir, victim,
 				     FSCACHE_OBJECT_WAS_CULLED);
@@ -822,8 +816,8 @@ int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir,
 	_leave(" = 0");
 	return 0;
 
-error_unlock:
-	inode_unlock(d_inode(dir));
+busy:
+	ret = -EBUSY;
 error:
 	dput(victim);
 	if (ret == -ENOENT)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a273d5cde731..009ca9f783bd 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2161,7 +2161,6 @@ struct super_operations {
 #define S_ENCRYPTED	(1 << 14) /* Encrypted file (using fs/crypto/) */
 #define S_CASEFOLD	(1 << 15) /* Casefolded file */
 #define S_VERITY	(1 << 16) /* Verity file (using fs/verity/) */
-#define S_KERNEL_FILE	(1 << 17) /* File is in use by the kernel (eg. fs/cachefiles) */
 
 /*
  * Note that nosuid etc flags are inode-specific: setting some file-system
@@ -2394,6 +2393,7 @@ static inline bool inode_is_dirtytime_only(struct inode *inode)
 }
 
 enum inode_excl_inuse_by {
+	inode_excl_inuse_by_cachefiles,
 	inode_excl_inuse_by_overlayfs,
 };
 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ