diff mbox series

[f2fs-dev,8/8] fscrypt: Move d_revalidate configuration back into fscrypt

Message ID 20231213234031.1081-9-krisman@suse.de (mailing list archive)
State Superseded
Headers show
Series Revert setting casefolding dentry operations through s_d_op | expand

Commit Message

Gabriel Krisman Bertazi Dec. 13, 2023, 11:40 p.m. UTC
This partially reverts commit bb9cd9106b22 ("fscrypt: Have filesystems
handle their d_ops"), which moved this handler out of fscrypt and into
the filesystems, in preparation to support casefold and fscrypt
combinations.  Now that we set casefolding operations through
->s_d_op, move this back into fscrypt, where it belongs, but take care
to handle filesystems that set their own sb->s_d_op.

Signed-off-by: Gabriel Krisman Bertazi <krisman@suse.de>
---
 fs/crypto/hooks.c       |  8 ++++++++
 fs/ext4/namei.c         |  5 -----
 fs/f2fs/namei.c         |  5 -----
 fs/libfs.c              | 19 -------------------
 fs/ubifs/dir.c          |  1 -
 include/linux/fs.h      |  1 -
 include/linux/fscrypt.h | 10 +++++-----
 7 files changed, 13 insertions(+), 36 deletions(-)

Comments

kernel test robot Dec. 14, 2023, 2:51 p.m. UTC | #1
Hi Gabriel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jaegeuk-f2fs/dev-test]
[also build test WARNING on jaegeuk-f2fs/dev tytso-ext4/dev linus/master v6.7-rc5 next-20231214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Gabriel-Krisman-Bertazi/dcache-Add-helper-to-disable-d_revalidate-for-a-specific-dentry/20231214-074322
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test
patch link:    https://lore.kernel.org/r/20231213234031.1081-9-krisman%40suse.de
patch subject: [PATCH 8/8] fscrypt: Move d_revalidate configuration back into fscrypt
config: x86_64-randconfig-123-20231214 (https://download.01.org/0day-ci/archive/20231214/202312142213.uyrNJniX-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231214/202312142213.uyrNJniX-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312142213.uyrNJniX-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/libfs.c:1778:39: warning: unused variable 'generic_encrypted_dentry_ops' [-Wunused-const-variable]
   static const struct dentry_operations generic_encrypted_dentry_ops = {
                                         ^
   1 warning generated.


vim +/generic_encrypted_dentry_ops +1778 fs/libfs.c

608af703519a58 Daniel Rosenberg 2020-11-19  1776  
608af703519a58 Daniel Rosenberg 2020-11-19  1777  #ifdef CONFIG_FS_ENCRYPTION
608af703519a58 Daniel Rosenberg 2020-11-19 @1778  static const struct dentry_operations generic_encrypted_dentry_ops = {
608af703519a58 Daniel Rosenberg 2020-11-19  1779  	.d_revalidate = fscrypt_d_revalidate,
608af703519a58 Daniel Rosenberg 2020-11-19  1780  };
608af703519a58 Daniel Rosenberg 2020-11-19  1781  #endif
608af703519a58 Daniel Rosenberg 2020-11-19  1782
diff mbox series

Patch

diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index 52504dd478d3..166837d5af29 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -94,6 +94,10 @@  int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry,
 }
 EXPORT_SYMBOL_GPL(__fscrypt_prepare_rename);
 
+static const struct dentry_operations fscrypt_dentry_ops = {
+	.d_revalidate = fscrypt_d_revalidate,
+};
+
 int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
 			     struct fscrypt_name *fname)
 {
@@ -106,6 +110,10 @@  int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
 		spin_lock(&dentry->d_lock);
 		dentry->d_flags |= DCACHE_NOKEY_NAME;
 		spin_unlock(&dentry->d_lock);
+
+		/* Give preference to the filesystem hooks, if any. */
+		if (!dentry->d_op)
+			d_set_d_op(dentry, &fscrypt_dentry_ops);
 	}
 	return err;
 }
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 3c1208d5d85b..3f0b853a371e 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1762,11 +1762,6 @@  static struct buffer_head *ext4_lookup_entry(struct inode *dir,
 	struct buffer_head *bh;
 
 	err = ext4_fname_prepare_lookup(dir, dentry, &fname);
-
-	/* Case-insensitive volumes set dentry ops through sb->s_d_op. */
-	if (!dir->i_sb->s_encoding)
-		generic_set_encrypted_ci_d_ops(dentry);
-
 	if (err == -ENOENT)
 		return NULL;
 	if (err)
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 4053846e2cd3..b40c6c393bd6 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -532,11 +532,6 @@  static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
 	}
 
 	err = f2fs_prepare_lookup(dir, dentry, &fname);
-
-	/* Case-insensitive volumes set dentry ops through sb->s_d_op. */
-	if (!dir->i_sb->s_encoding)
-		generic_set_encrypted_ci_d_ops(dentry);
-
 	if (err == -ENOENT)
 		goto out_splice;
 	if (err)
diff --git a/fs/libfs.c b/fs/libfs.c
index 41c02c003265..4fed170dfe49 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1780,25 +1780,6 @@  static const struct dentry_operations generic_encrypted_dentry_ops = {
 };
 #endif
 
-/**
- * generic_set_encrypted_ci_d_ops - helper for setting d_ops for given dentry
- * @dentry:	dentry to set ops on
- *
- * Encryption works differently in that the only dentry operation it needs is
- * d_revalidate, which it only needs on dentries that have the no-key name flag.
- * The no-key flag can't be set "later", so we don't have to worry about that.
- */
-void generic_set_encrypted_ci_d_ops(struct dentry *dentry)
-{
-#ifdef CONFIG_FS_ENCRYPTION
-	if (dentry->d_flags & DCACHE_NOKEY_NAME) {
-		d_set_d_op(dentry, &generic_encrypted_dentry_ops);
-		return;
-	}
-#endif
-}
-EXPORT_SYMBOL(generic_set_encrypted_ci_d_ops);
-
 /**
  * inode_maybe_inc_iversion - increments i_version
  * @inode: inode with the i_version that should be updated
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 3b13c648d490..51b9a10a9851 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -205,7 +205,6 @@  static struct dentry *ubifs_lookup(struct inode *dir, struct dentry *dentry,
 	dbg_gen("'%pd' in dir ino %lu", dentry, dir->i_ino);
 
 	err = fscrypt_prepare_lookup(dir, dentry, &nm);
-	generic_set_encrypted_ci_d_ops(dentry);
 	if (err == -ENOENT)
 		return d_splice_alias(NULL, dentry);
 	if (err)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 887a27d07f96..e5ae21f9f637 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3201,7 +3201,6 @@  extern int generic_file_fsync(struct file *, loff_t, loff_t, int);
 
 extern int generic_check_addressable(unsigned, u64);
 
-extern void generic_set_encrypted_ci_d_ops(struct dentry *dentry);
 extern const struct dentry_operations generic_ci_dentry_ops;
 
 int may_setattr(struct mnt_idmap *idmap, struct inode *inode,
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 12f9e455d569..97a11280c2bd 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -961,11 +961,11 @@  static inline int fscrypt_prepare_rename(struct inode *old_dir,
  * key is available, then the lookup is assumed to be by plaintext name;
  * otherwise, it is assumed to be by no-key name.
  *
- * This will set DCACHE_NOKEY_NAME on the dentry if the lookup is by no-key
- * name.  In this case the filesystem must assign the dentry a dentry_operations
- * which contains fscrypt_d_revalidate (or contains a d_revalidate method that
- * calls fscrypt_d_revalidate), so that the dentry will be invalidated if the
- * directory's encryption key is later added.
+ * This also optionally installs a custom ->d_revalidate() method which will
+ * invalidate the dentry if it was created without the key and the key is later
+ * added.  If the filesystem provides its own ->d_op hooks, they will be used
+ * instead, but then the filesystem must make sure to call fscrypt_d_revalidate
+ * in its d_revalidate hook, to check if fscrypt considers the dentry stale.
  *
  * Return: 0 on success; -ENOENT if the directory's key is unavailable but the
  * filename isn't a valid no-key name, so a negative dentry should be created;