[RFC 0/6] ext4: Move out crypto ops to ext4_crypto.c

[RFC 0/6] ext4: Move out crypto ops to ext4_crypto.c

[Ritesh Harjani]

Hello,

This is 1st in the series to cleanup ext4/super.c, since it has grown quite large.
This moves out crypto related ops and few definitions to fs/ext4/ext4_crypto.c

Testing
=========
1. Tested "-g encrypt" with default configs.
2. Compiled tested on x86 & Power.


Ritesh Harjani (6):
  fscrypt: Provide definition of fscrypt_set_test_dummy_encryption
  ext4: Move ext4 crypto code to its own file ext4_crypto.c
  ext4: Directly opencode ext4_set_test_dummy_encryption
  ext4: Cleanup function defs from ext4.h into ext4_crypto.c
  ext4: Move all encryption related into a common #ifdef
  ext4: Use provided macro for checking dummy_enc_policy

 fs/ext4/Makefile        |   1 +
 fs/ext4/ext4.h          |  81 +++--------------
 fs/ext4/ext4_crypto.c   | 192 ++++++++++++++++++++++++++++++++++++++++
 fs/ext4/super.c         | 158 ++++-----------------------------
 include/linux/fscrypt.h |   7 ++
 5 files changed, 227 insertions(+), 212 deletions(-)
 create mode 100644 fs/ext4/ext4_crypto.c

--
2.31.1

[RFC 1/6] fscrypt: Provide definition of fscrypt_set_test_dummy_encryption

[Ritesh Harjani]

Provide definition of fscrypt_set_test_dummy_encryption() when
CONFIG_FS_ENCRYPTION is disabled. This then returns -EOPNOTSUPP.

Signed-off-by: Ritesh Harjani <ritesh.list@gmail.com>
---
 include/linux/fscrypt.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 91ea9477e9bd..18dd6048bab3 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -467,6 +467,13 @@ static inline int fscrypt_set_context(struct inode *inode, void *fs_data)
 struct fscrypt_dummy_policy {
 };
 
+static inline int fscrypt_set_test_dummy_encryption(struct super_block *sb,
+				const char *arg,
+				struct fscrypt_dummy_policy *dummy_policy)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline void fscrypt_show_test_dummy_encryption(struct seq_file *seq,
 						      char sep,
 						      struct super_block *sb)
-- 
2.31.1

[RFC 2/6] ext4: Move ext4 crypto code to its own file ext4_crypto.c

[Ritesh Harjani]

This is to cleanup super.c file which has grown quite large.
So, start moving ext4 crypto related code to where it should be in the first
place i.e. fs/ext4/ext4_crypto.c

Signed-off-by: Ritesh Harjani <ritesh.list@gmail.com>
---
 fs/ext4/Makefile      |   1 +
 fs/ext4/ext4.h        |   3 +
 fs/ext4/ext4_crypto.c | 127 ++++++++++++++++++++++++++++++++++++++++++
 fs/ext4/super.c       | 122 ----------------------------------------
 4 files changed, 131 insertions(+), 122 deletions(-)
 create mode 100644 fs/ext4/ext4_crypto.c

diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile
index 7d89142e1421..b340fe5f849c 100644
--- a/fs/ext4/Makefile
+++ b/fs/ext4/Makefile
@@ -17,3 +17,4 @@ ext4-$(CONFIG_EXT4_FS_SECURITY)		+= xattr_security.o
 ext4-inode-test-objs			+= inode-test.o
 obj-$(CONFIG_EXT4_KUNIT_TESTS)		+= ext4-inode-test.o
 ext4-$(CONFIG_FS_VERITY)		+= verity.o
+ext4-$(CONFIG_FS_ENCRYPTION)		+= ext4_crypto.o
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 1d79012c5a5b..8bac8af25ed8 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2727,6 +2727,9 @@ extern int ext4_fname_setup_ci_filename(struct inode *dir,
 					 struct ext4_filename *fname);
 #endif
 
+/* ext4 encryption related stuff goes here ext4_crypto.c */
+extern const struct fscrypt_operations ext4_cryptops;
+
 #ifdef CONFIG_FS_ENCRYPTION
 static inline void ext4_fname_from_fscrypt_name(struct ext4_filename *dst,
 						const struct fscrypt_name *src)
diff --git a/fs/ext4/ext4_crypto.c b/fs/ext4/ext4_crypto.c
new file mode 100644
index 000000000000..e5413c0970ee
--- /dev/null
+++ b/fs/ext4/ext4_crypto.c
@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/quotaops.h>
+
+#include "ext4.h"
+#include "xattr.h"
+#include "ext4_jbd2.h"
+
+static int ext4_get_context(struct inode *inode, void *ctx, size_t len)
+{
+	return ext4_xattr_get(inode, EXT4_XATTR_INDEX_ENCRYPTION,
+				 EXT4_XATTR_NAME_ENCRYPTION_CONTEXT, ctx, len);
+}
+
+static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
+							void *fs_data)
+{
+	handle_t *handle = fs_data;
+	int res, res2, credits, retries = 0;
+
+	/*
+	 * Encrypting the root directory is not allowed because e2fsck expects
+	 * lost+found to exist and be unencrypted, and encrypting the root
+	 * directory would imply encrypting the lost+found directory as well as
+	 * the filename "lost+found" itself.
+	 */
+	if (inode->i_ino == EXT4_ROOT_INO)
+		return -EPERM;
+
+	if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode)))
+		return -EINVAL;
+
+	if (ext4_test_inode_flag(inode, EXT4_INODE_DAX))
+		return -EOPNOTSUPP;
+
+	res = ext4_convert_inline_data(inode);
+	if (res)
+		return res;
+
+	/*
+	 * If a journal handle was specified, then the encryption context is
+	 * being set on a new inode via inheritance and is part of a larger
+	 * transaction to create the inode.  Otherwise the encryption context is
+	 * being set on an existing inode in its own transaction.  Only in the
+	 * latter case should the "retry on ENOSPC" logic be used.
+	 */
+
+	if (handle) {
+		res = ext4_xattr_set_handle(handle, inode,
+					    EXT4_XATTR_INDEX_ENCRYPTION,
+					    EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
+					    ctx, len, 0);
+		if (!res) {
+			ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
+			ext4_clear_inode_state(inode,
+					EXT4_STATE_MAY_INLINE_DATA);
+			/*
+			 * Update inode->i_flags - S_ENCRYPTED will be enabled,
+			 * S_DAX may be disabled
+			 */
+			ext4_set_inode_flags(inode, false);
+		}
+		return res;
+	}
+
+	res = dquot_initialize(inode);
+	if (res)
+		return res;
+retry:
+	res = ext4_xattr_set_credits(inode, len, false /* is_create */,
+				     &credits);
+	if (res)
+		return res;
+
+	handle = ext4_journal_start(inode, EXT4_HT_MISC, credits);
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+
+	res = ext4_xattr_set_handle(handle, inode, EXT4_XATTR_INDEX_ENCRYPTION,
+				    EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
+				    ctx, len, 0);
+	if (!res) {
+		ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
+		/*
+		 * Update inode->i_flags - S_ENCRYPTED will be enabled,
+		 * S_DAX may be disabled
+		 */
+		ext4_set_inode_flags(inode, false);
+		res = ext4_mark_inode_dirty(handle, inode);
+		if (res)
+			EXT4_ERROR_INODE(inode, "Failed to mark inode dirty");
+	}
+	res2 = ext4_journal_stop(handle);
+
+	if (res == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
+		goto retry;
+	if (!res)
+		res = res2;
+	return res;
+}
+
+static const union fscrypt_policy *ext4_get_dummy_policy(struct super_block *sb)
+{
+	return EXT4_SB(sb)->s_dummy_enc_policy.policy;
+}
+
+static bool ext4_has_stable_inodes(struct super_block *sb)
+{
+	return ext4_has_feature_stable_inodes(sb);
+}
+
+static void ext4_get_ino_and_lblk_bits(struct super_block *sb,
+				       int *ino_bits_ret, int *lblk_bits_ret)
+{
+	*ino_bits_ret = 8 * sizeof(EXT4_SB(sb)->s_es->s_inodes_count);
+	*lblk_bits_ret = 8 * sizeof(ext4_lblk_t);
+}
+
+const struct fscrypt_operations ext4_cryptops = {
+	.key_prefix		= "ext4:",
+	.get_context		= ext4_get_context,
+	.set_context		= ext4_set_context,
+	.get_dummy_policy	= ext4_get_dummy_policy,
+	.empty_dir		= ext4_empty_dir,
+	.has_stable_inodes	= ext4_has_stable_inodes,
+	.get_ino_and_lblk_bits	= ext4_get_ino_and_lblk_bits,
+};
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ae98b07285d2..8bb5fa15a013 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1488,128 +1488,6 @@ static int ext4_nfs_commit_metadata(struct inode *inode)
 	return ext4_write_inode(inode, &wbc);
 }
 
-#ifdef CONFIG_FS_ENCRYPTION
-static int ext4_get_context(struct inode *inode, void *ctx, size_t len)
-{
-	return ext4_xattr_get(inode, EXT4_XATTR_INDEX_ENCRYPTION,
-				 EXT4_XATTR_NAME_ENCRYPTION_CONTEXT, ctx, len);
-}
-
-static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
-							void *fs_data)
-{
-	handle_t *handle = fs_data;
-	int res, res2, credits, retries = 0;
-
-	/*
-	 * Encrypting the root directory is not allowed because e2fsck expects
-	 * lost+found to exist and be unencrypted, and encrypting the root
-	 * directory would imply encrypting the lost+found directory as well as
-	 * the filename "lost+found" itself.
-	 */
-	if (inode->i_ino == EXT4_ROOT_INO)
-		return -EPERM;
-
-	if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode)))
-		return -EINVAL;
-
-	if (ext4_test_inode_flag(inode, EXT4_INODE_DAX))
-		return -EOPNOTSUPP;
-
-	res = ext4_convert_inline_data(inode);
-	if (res)
-		return res;
-
-	/*
-	 * If a journal handle was specified, then the encryption context is
-	 * being set on a new inode via inheritance and is part of a larger
-	 * transaction to create the inode.  Otherwise the encryption context is
-	 * being set on an existing inode in its own transaction.  Only in the
-	 * latter case should the "retry on ENOSPC" logic be used.
-	 */
-
-	if (handle) {
-		res = ext4_xattr_set_handle(handle, inode,
-					    EXT4_XATTR_INDEX_ENCRYPTION,
-					    EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
-					    ctx, len, 0);
-		if (!res) {
-			ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
-			ext4_clear_inode_state(inode,
-					EXT4_STATE_MAY_INLINE_DATA);
-			/*
-			 * Update inode->i_flags - S_ENCRYPTED will be enabled,
-			 * S_DAX may be disabled
-			 */
-			ext4_set_inode_flags(inode, false);
-		}
-		return res;
-	}
-
-	res = dquot_initialize(inode);
-	if (res)
-		return res;
-retry:
-	res = ext4_xattr_set_credits(inode, len, false /* is_create */,
-				     &credits);
-	if (res)
-		return res;
-
-	handle = ext4_journal_start(inode, EXT4_HT_MISC, credits);
-	if (IS_ERR(handle))
-		return PTR_ERR(handle);
-
-	res = ext4_xattr_set_handle(handle, inode, EXT4_XATTR_INDEX_ENCRYPTION,
-				    EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
-				    ctx, len, 0);
-	if (!res) {
-		ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
-		/*
-		 * Update inode->i_flags - S_ENCRYPTED will be enabled,
-		 * S_DAX may be disabled
-		 */
-		ext4_set_inode_flags(inode, false);
-		res = ext4_mark_inode_dirty(handle, inode);
-		if (res)
-			EXT4_ERROR_INODE(inode, "Failed to mark inode dirty");
-	}
-	res2 = ext4_journal_stop(handle);
-
-	if (res == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
-		goto retry;
-	if (!res)
-		res = res2;
-	return res;
-}
-
-static const union fscrypt_policy *ext4_get_dummy_policy(struct super_block *sb)
-{
-	return EXT4_SB(sb)->s_dummy_enc_policy.policy;
-}
-
-static bool ext4_has_stable_inodes(struct super_block *sb)
-{
-	return ext4_has_feature_stable_inodes(sb);
-}
-
-static void ext4_get_ino_and_lblk_bits(struct super_block *sb,
-				       int *ino_bits_ret, int *lblk_bits_ret)
-{
-	*ino_bits_ret = 8 * sizeof(EXT4_SB(sb)->s_es->s_inodes_count);
-	*lblk_bits_ret = 8 * sizeof(ext4_lblk_t);
-}
-
-static const struct fscrypt_operations ext4_cryptops = {
-	.key_prefix		= "ext4:",
-	.get_context		= ext4_get_context,
-	.set_context		= ext4_set_context,
-	.get_dummy_policy	= ext4_get_dummy_policy,
-	.empty_dir		= ext4_empty_dir,
-	.has_stable_inodes	= ext4_has_stable_inodes,
-	.get_ino_and_lblk_bits	= ext4_get_ino_and_lblk_bits,
-};
-#endif
-
 #ifdef CONFIG_QUOTA
 static const char * const quotatypes[] = INITQFNAMES;
 #define QTYPE2NAME(t) (quotatypes[t])
-- 
2.31.1

[RFC 3/6] ext4: Directly opencode ext4_set_test_dummy_encryption

[Ritesh Harjani]

Simplify the code by opencoding ext4_set_test_dummy_encryption(), since
there is only one caller of it and ext4_** variant in itself does nothing
special other than calling fscrypt_set_test_dummy_encryption(). This can
be done directly by opencoding it in the caller.

Signed-off-by: Ritesh Harjani <ritesh.list@gmail.com>
---
 fs/ext4/super.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 8bb5fa15a013..e7e5c9c057d7 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1863,24 +1863,6 @@ ext4_sb_read_encoding(const struct ext4_super_block *es)
 }
 #endif
 
-static int ext4_set_test_dummy_encryption(struct super_block *sb, char *arg)
-{
-#ifdef CONFIG_FS_ENCRYPTION
-	struct ext4_sb_info *sbi = EXT4_SB(sb);
-	int err;
-
-	err = fscrypt_set_test_dummy_encryption(sb, arg,
-						&sbi->s_dummy_enc_policy);
-	if (err) {
-		ext4_msg(sb, KERN_WARNING,
-			 "Error while setting test dummy encryption [%d]", err);
-		return err;
-	}
-	ext4_msg(sb, KERN_WARNING, "Test dummy encryption mode enabled");
-#endif
-	return 0;
-}
-
 #define EXT4_SPEC_JQUOTA			(1 <<  0)
 #define EXT4_SPEC_JQFMT				(1 <<  1)
 #define EXT4_SPEC_DATAJ				(1 <<  2)
@@ -2795,8 +2777,20 @@ static int ext4_apply_options(struct fs_context *fc, struct super_block *sb)
 
 	ext4_apply_quota_options(fc, sb);
 
-	if (ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION)
-		ret = ext4_set_test_dummy_encryption(sb, ctx->test_dummy_enc_arg);
+	if (IS_ENABLED(CONFIG_FS_ENCRYPTION) &&
+			(ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION)) {
+
+		ret = fscrypt_set_test_dummy_encryption(sb,
+						ctx->test_dummy_enc_arg,
+						&sbi->s_dummy_enc_policy);
+		if (ret) {
+			ext4_msg(sb, KERN_WARNING, "Error while setting test dummy encryption [%d]",
+				 ret);
+			return ret;
+		}
+
+		ext4_msg(sb, KERN_WARNING, "Test dummy encryption mode enabled");
+	}
 
 	return ret;
 }
-- 
2.31.1

[RFC 4/6] ext4: Cleanup function defs from ext4.h into ext4_crypto.c

[Ritesh Harjani]

Some of these functions when CONFIG_FS_ENCRYPTION is enabled are not
really inline (let compiler be the best judge of it).
Remove inline and move them into ext4_crypto.c where they should be present.

Signed-off-by: Ritesh Harjani <ritesh.list@gmail.com>
---
 fs/ext4/ext4.h        | 70 ++++---------------------------------------
 fs/ext4/ext4_crypto.c | 65 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 64 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 8bac8af25ed8..caf154db4680 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2731,73 +2731,15 @@ extern int ext4_fname_setup_ci_filename(struct inode *dir,
 extern const struct fscrypt_operations ext4_cryptops;
 
 #ifdef CONFIG_FS_ENCRYPTION
-static inline void ext4_fname_from_fscrypt_name(struct ext4_filename *dst,
-						const struct fscrypt_name *src)
-{
-	memset(dst, 0, sizeof(*dst));
-
-	dst->usr_fname = src->usr_fname;
-	dst->disk_name = src->disk_name;
-	dst->hinfo.hash = src->hash;
-	dst->hinfo.minor_hash = src->minor_hash;
-	dst->crypto_buf = src->crypto_buf;
-}
-
-static inline int ext4_fname_setup_filename(struct inode *dir,
-					    const struct qstr *iname,
-					    int lookup,
-					    struct ext4_filename *fname)
-{
-	struct fscrypt_name name;
-	int err;
+int ext4_fname_setup_filename(struct inode *dir,
+			      const struct qstr *iname, int lookup,
+			      struct ext4_filename *fname);
 
-	err = fscrypt_setup_filename(dir, iname, lookup, &name);
-	if (err)
-		return err;
+int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry,
+			      struct ext4_filename *fname);
 
-	ext4_fname_from_fscrypt_name(fname, &name);
+void ext4_fname_free_filename(struct ext4_filename *fname);
 
-#if IS_ENABLED(CONFIG_UNICODE)
-	err = ext4_fname_setup_ci_filename(dir, iname, fname);
-#endif
-	return err;
-}
-
-static inline int ext4_fname_prepare_lookup(struct inode *dir,
-					    struct dentry *dentry,
-					    struct ext4_filename *fname)
-{
-	struct fscrypt_name name;
-	int err;
-
-	err = fscrypt_prepare_lookup(dir, dentry, &name);
-	if (err)
-		return err;
-
-	ext4_fname_from_fscrypt_name(fname, &name);
-
-#if IS_ENABLED(CONFIG_UNICODE)
-	err = ext4_fname_setup_ci_filename(dir, &dentry->d_name, fname);
-#endif
-	return err;
-}
-
-static inline void ext4_fname_free_filename(struct ext4_filename *fname)
-{
-	struct fscrypt_name name;
-
-	name.crypto_buf = fname->crypto_buf;
-	fscrypt_free_filename(&name);
-
-	fname->crypto_buf.name = NULL;
-	fname->usr_fname = NULL;
-	fname->disk_name.name = NULL;
-
-#if IS_ENABLED(CONFIG_UNICODE)
-	kfree(fname->cf_name.name);
-	fname->cf_name.name = NULL;
-#endif
-}
 #else /* !CONFIG_FS_ENCRYPTION */
 static inline int ext4_fname_setup_filename(struct inode *dir,
 					    const struct qstr *iname,
diff --git a/fs/ext4/ext4_crypto.c b/fs/ext4/ext4_crypto.c
index e5413c0970ee..7e89f86a4429 100644
--- a/fs/ext4/ext4_crypto.c
+++ b/fs/ext4/ext4_crypto.c
@@ -6,6 +6,71 @@
 #include "xattr.h"
 #include "ext4_jbd2.h"
 
+void ext4_fname_from_fscrypt_name(struct ext4_filename *dst,
+				  const struct fscrypt_name *src)
+{
+	memset(dst, 0, sizeof(*dst));
+
+	dst->usr_fname = src->usr_fname;
+	dst->disk_name = src->disk_name;
+	dst->hinfo.hash = src->hash;
+	dst->hinfo.minor_hash = src->minor_hash;
+	dst->crypto_buf = src->crypto_buf;
+}
+
+int ext4_fname_setup_filename(struct inode *dir, const struct qstr *iname,
+			      int lookup, struct ext4_filename *fname)
+{
+	struct fscrypt_name name;
+	int err;
+
+	err = fscrypt_setup_filename(dir, iname, lookup, &name);
+	if (err)
+		return err;
+
+	ext4_fname_from_fscrypt_name(fname, &name);
+
+#if IS_ENABLED(CONFIG_UNICODE)
+	err = ext4_fname_setup_ci_filename(dir, iname, fname);
+#endif
+	return err;
+}
+
+int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry,
+			      struct ext4_filename *fname)
+{
+	struct fscrypt_name name;
+	int err;
+
+	err = fscrypt_prepare_lookup(dir, dentry, &name);
+	if (err)
+		return err;
+
+	ext4_fname_from_fscrypt_name(fname, &name);
+
+#if IS_ENABLED(CONFIG_UNICODE)
+	err = ext4_fname_setup_ci_filename(dir, &dentry->d_name, fname);
+#endif
+	return err;
+}
+
+void ext4_fname_free_filename(struct ext4_filename *fname)
+{
+	struct fscrypt_name name;
+
+	name.crypto_buf = fname->crypto_buf;
+	fscrypt_free_filename(&name);
+
+	fname->crypto_buf.name = NULL;
+	fname->usr_fname = NULL;
+	fname->disk_name.name = NULL;
+
+#if IS_ENABLED(CONFIG_UNICODE)
+	kfree(fname->cf_name.name);
+	fname->cf_name.name = NULL;
+#endif
+}
+
 static int ext4_get_context(struct inode *inode, void *ctx, size_t len)
 {
 	return ext4_xattr_get(inode, EXT4_XATTR_INDEX_ENCRYPTION,
-- 
2.31.1

[RFC 5/6] ext4: Move all encryption related into a common #ifdef

[Ritesh Harjani]

This just moves left over usages of #ifdef CONFIG_FS_ENCRYPTION
into a common place for function/macro definitions.

Signed-off-by: Ritesh Harjani <ritesh.list@gmail.com>
---
 fs/ext4/ext4.h | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index caf154db4680..2fb69c500063 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1440,12 +1440,6 @@ struct ext4_super_block {

 #ifdef __KERNEL__

-#ifdef CONFIG_FS_ENCRYPTION
-#define DUMMY_ENCRYPTION_ENABLED(sbi) ((sbi)->s_dummy_enc_policy.policy != NULL)
-#else
-#define DUMMY_ENCRYPTION_ENABLED(sbi) (0)
-#endif
-
 /* Number of quota types we support */
 #define EXT4_MAXQUOTAS 3

@@ -2740,6 +2734,8 @@ int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry,

 void ext4_fname_free_filename(struct ext4_filename *fname);

+#define DUMMY_ENCRYPTION_ENABLED(sbi) ((sbi)->s_dummy_enc_policy.policy != NULL)
+
 #else /* !CONFIG_FS_ENCRYPTION */
 static inline int ext4_fname_setup_filename(struct inode *dir,
 					    const struct qstr *iname,
@@ -2772,6 +2768,8 @@ static inline void ext4_fname_free_filename(struct ext4_filename *fname)
 	fname->cf_name.name = NULL;
 #endif
 }
+
+#define DUMMY_ENCRYPTION_ENABLED(sbi) (0)
 #endif /* !CONFIG_FS_ENCRYPTION */

 /* dir.c */
--
2.31.1

[RFC 6/6] ext4: Use provided macro for checking dummy_enc_policy

[Ritesh Harjani]

We have a macro which test is dummy_enc_policy is enabled or not.
Use that instead.

Signed-off-by: Ritesh Harjani <ritesh.list@gmail.com>
---
 fs/ext4/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e7e5c9c057d7..73fb54c3efd3 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2685,7 +2685,7 @@ static int ext4_check_opt_consistency(struct fs_context *fc,
 	 * it to be specified during remount, but only if there is no change.
 	 */
 	if ((ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION) &&
-	    is_remount && !sbi->s_dummy_enc_policy.policy) {
+	    is_remount && !DUMMY_ENCRYPTION_ENABLED(sbi)) {
 		ext4_msg(NULL, KERN_WARNING,
 			 "Can't set test_dummy_encryption on remount");
 		return -1;
-- 
2.31.1

Re: [RFC 0/6] ext4: Move out crypto ops to ext4_crypto.c

[Eric Biggers]

On Thu, Apr 21, 2022 at 10:53:16AM +0530, Ritesh Harjani wrote:
> Hello,
> 
> This is 1st in the series to cleanup ext4/super.c, since it has grown quite large.
> This moves out crypto related ops and few definitions to fs/ext4/ext4_crypto.c
> 
> Testing
> =========
> 1. Tested "-g encrypt" with default configs.
> 2. Compiled tested on x86 & Power.
> 
> 
> Ritesh Harjani (6):
>   fscrypt: Provide definition of fscrypt_set_test_dummy_encryption
>   ext4: Move ext4 crypto code to its own file ext4_crypto.c
>   ext4: Directly opencode ext4_set_test_dummy_encryption
>   ext4: Cleanup function defs from ext4.h into ext4_crypto.c
>   ext4: Move all encryption related into a common #ifdef
>   ext4: Use provided macro for checking dummy_enc_policy
> 
>  fs/ext4/Makefile        |   1 +
>  fs/ext4/ext4.h          |  81 +++--------------
>  fs/ext4/ext4_crypto.c   | 192 ++++++++++++++++++++++++++++++++++++++++
>  fs/ext4/super.c         | 158 ++++-----------------------------
>  include/linux/fscrypt.h |   7 ++
>  5 files changed, 227 insertions(+), 212 deletions(-)
>  create mode 100644 fs/ext4/ext4_crypto.c

How about calling it crypto.c instead of ext4_crypto.c?  It is already in the
ext4 directory, so ext4 is implied.

Otherwise this patchset looks good to me.

Did you consider moving any of the other CONFIG_FS_ENCRYPTION code blocks into
the new file as well?  The implementation of FS_IOC_GET_ENCRYPTION_PWSALT might
be a good candidate too.

- Eric

Re: [RFC 0/6] ext4: Move out crypto ops to ext4_crypto.c

[Ritesh Harjani]

On 22/04/21 12:24AM, Eric Biggers wrote:
> On Thu, Apr 21, 2022 at 10:53:16AM +0530, Ritesh Harjani wrote:
> > Hello,
> >
> > This is 1st in the series to cleanup ext4/super.c, since it has grown quite large.
> > This moves out crypto related ops and few definitions to fs/ext4/ext4_crypto.c
> >
> > Testing
> > =========
> > 1. Tested "-g encrypt" with default configs.
> > 2. Compiled tested on x86 & Power.
> >
> >
> > Ritesh Harjani (6):
> >   fscrypt: Provide definition of fscrypt_set_test_dummy_encryption
> >   ext4: Move ext4 crypto code to its own file ext4_crypto.c
> >   ext4: Directly opencode ext4_set_test_dummy_encryption
> >   ext4: Cleanup function defs from ext4.h into ext4_crypto.c
> >   ext4: Move all encryption related into a common #ifdef
> >   ext4: Use provided macro for checking dummy_enc_policy
> >
> >  fs/ext4/Makefile        |   1 +
> >  fs/ext4/ext4.h          |  81 +++--------------
> >  fs/ext4/ext4_crypto.c   | 192 ++++++++++++++++++++++++++++++++++++++++
> >  fs/ext4/super.c         | 158 ++++-----------------------------
> >  include/linux/fscrypt.h |   7 ++
> >  5 files changed, 227 insertions(+), 212 deletions(-)
> >  create mode 100644 fs/ext4/ext4_crypto.c
>
> How about calling it crypto.c instead of ext4_crypto.c?  It is already in the
> ext4 directory, so ext4 is implied.

Sure will do so in next revision which can also cover any other comments from
others.

>
> Otherwise this patchset looks good to me.

Sure thanks. May I add your Reviewed-by in next version if I don't see any major
changes?

>
> Did you consider moving any of the other CONFIG_FS_ENCRYPTION code blocks into
> the new file as well?  The implementation of FS_IOC_GET_ENCRYPTION_PWSALT might
> be a good candidate too.

Sure, thanks for pointing out. I will take a look at it for next patch series or
so.

-ritesh

Re: [RFC 0/6] ext4: Move out crypto ops to ext4_crypto.c

[Eric Biggers]

On Thu, Apr 21, 2022 at 10:53:16AM +0530, Ritesh Harjani wrote:
> Hello,
> 
> This is 1st in the series to cleanup ext4/super.c, since it has grown quite large.
> This moves out crypto related ops and few definitions to fs/ext4/ext4_crypto.c
> 
> Testing
> =========
> 1. Tested "-g encrypt" with default configs.
> 2. Compiled tested on x86 & Power.
> 
> 
> Ritesh Harjani (6):
>   fscrypt: Provide definition of fscrypt_set_test_dummy_encryption
>   ext4: Move ext4 crypto code to its own file ext4_crypto.c
>   ext4: Directly opencode ext4_set_test_dummy_encryption
>   ext4: Cleanup function defs from ext4.h into ext4_crypto.c
>   ext4: Move all encryption related into a common #ifdef
>   ext4: Use provided macro for checking dummy_enc_policy

FYI, the patchset
https://lore.kernel.org/linux-ext4/20220501050857.538984-1-ebiggers@kernel.org
I just sent out cleans up how the test_dummy_encryption mount option is handled.
It would supersede patches 1, 3, 5, and 6 of this series (since those all only
deal with test_dummy_encryption-related code).

To avoid conflicting changes, maybe you should just focus on your patches 2 and
4 for now, along with possibly FS_IOC_GET_ENCRYPTION_PWSALT as I mentioned?
There shouldn't be any overlap that way.

- Eric

Re: [RFC 0/6] ext4: Move out crypto ops to ext4_crypto.c

[Ritesh Harjani]

On 22/05/01 12:18AM, Eric Biggers wrote:
> On Thu, Apr 21, 2022 at 10:53:16AM +0530, Ritesh Harjani wrote:
> > Hello,
> >
> > This is 1st in the series to cleanup ext4/super.c, since it has grown quite large.
> > This moves out crypto related ops and few definitions to fs/ext4/ext4_crypto.c
> >
> > Testing
> > =========
> > 1. Tested "-g encrypt" with default configs.
> > 2. Compiled tested on x86 & Power.
> >
> >
> > Ritesh Harjani (6):
> >   fscrypt: Provide definition of fscrypt_set_test_dummy_encryption
> >   ext4: Move ext4 crypto code to its own file ext4_crypto.c
> >   ext4: Directly opencode ext4_set_test_dummy_encryption
> >   ext4: Cleanup function defs from ext4.h into ext4_crypto.c
> >   ext4: Move all encryption related into a common #ifdef
> >   ext4: Use provided macro for checking dummy_enc_policy
>
> FYI, the patchset
> https://lore.kernel.org/linux-ext4/20220501050857.538984-1-ebiggers@kernel.org
> I just sent out cleans up how the test_dummy_encryption mount option is handled.
> It would supersede patches 1, 3, 5, and 6 of this series (since those all only
> deal with test_dummy_encryption-related code).

Sure got it.

>
> To avoid conflicting changes, maybe you should just focus on your patches 2 and
> 4 for now, along with possibly FS_IOC_GET_ENCRYPTION_PWSALT as I mentioned?
> There shouldn't be any overlap that way.

Yes, agreed with al of above points Eric. Will make the changes and send a new
revision.

-ritesh