Hello,
Please find the v3 of this cleanup series. Thanks to Eric for his quick
review of the patch series.
Description
=============
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 fs encryption related
definitions to fs/ext4/crypto.c
I have tested "-g encrypt" xfstests group and don't see any surprises there.
The changes are relatively straight forward since it is just moving/refactoring.
Currently these patches apply cleanly on ext4 tree's dev branch.
Since in these series test_dummy_encryption related changes are dropped, hence
I don't think that this should give any major conflict with Eric's series.
NOTE: I noticed we could move both ext4 & f2fs to use uuid_t lib API from
include/linux/uuid.h for managing sb->s_encrypt_pw_salt.
That should kill custom implementations of uuid_is_zero()/uuid_is_nonzero().
But since I noticed it while writing this cover letter, so I would like to
do those changes in a seperate patch series if that is ok. That way maybe we
could cleanup tree wide changes in fs/ (if there are others too).
v2 -> v3
=========
1. Addressed review comments from Eric.
RFC -> v2
==========
1. Dropped all test_dummy_encryption related changes
(Eric has separately submitted a v3 for fixing more general problems with
that mount option).
2. Addressed Eric comments to:-
1. rename ext4_crypto.c -> crypto.c
2. Refactor out ext4_ioc_get_encryption_pwsalt() into crypto.c
3. Made ext4_fname_from_fscrypt_name() static since it is only being called
from within crypto.c functions.
[v2] - https://lore.kernel.org/linux-ext4/[email protected]/T/#t
[RFC] - https://lore.kernel.org/linux-ext4/[email protected]/
Ritesh Harjani (3):
ext4: Move ext4 crypto code to its own file crypto.c
ext4: Cleanup function defs from ext4.h into crypto.c
ext4: Refactor and move ext4_ioctl_get_encryption_pwsalt()
fs/ext4/Makefile | 1 +
fs/ext4/crypto.c | 246 +++++++++++++++++++++++++++++++++++++++++++++++
fs/ext4/ext4.h | 76 +++------------
fs/ext4/ioctl.c | 59 +-----------
fs/ext4/super.c | 122 -----------------------
5 files changed, 263 insertions(+), 241 deletions(-)
create mode 100644 fs/ext4/crypto.c
--
2.31.1
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 crypto.c where they should be present.
Reviewed-by: Eric Biggers <[email protected]>
Signed-off-by: Ritesh Harjani <[email protected]>
---
fs/ext4/crypto.c | 65 +++++++++++++++++++++++++++++++++++++++++++++
fs/ext4/ext4.h | 69 ++++--------------------------------------------
2 files changed, 70 insertions(+), 64 deletions(-)
diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index e5413c0970ee..f8333927f0f6 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c
@@ -6,6 +6,71 @@
#include "xattr.h"
#include "ext4_jbd2.h"
+static 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,
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 95d87641ad87..3c474c9623af 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2735,73 +2735,14 @@ extern int ext4_fname_setup_ci_filename(struct inode *dir,
#ifdef CONFIG_FS_ENCRYPTION
extern const struct fscrypt_operations ext4_cryptops;
-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,
--
2.31.1
This patch move code for FS_IOC_GET_ENCRYPTION_PWSALT case into
ext4's crypto.c file, i.e. ext4_ioctl_get_encryption_pwsalt()
and uuid_is_zero(). This is mostly refactoring logic and should
not affect any functionality change.
Suggested-by: Eric Biggers <[email protected]>
Reviewed-by: Eric Biggers <[email protected]>
Signed-off-by: Ritesh Harjani <[email protected]>
---
fs/ext4/crypto.c | 54 ++++++++++++++++++++++++++++++++++++++++++++
fs/ext4/ext4.h | 8 +++++++
fs/ext4/ioctl.c | 59 ++----------------------------------------------
3 files changed, 64 insertions(+), 57 deletions(-)
diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index f8333927f0f6..e20ac0654b3f 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/quotaops.h>
+#include <linux/uuid.h>
#include "ext4.h"
#include "xattr.h"
@@ -71,6 +72,59 @@ void ext4_fname_free_filename(struct ext4_filename *fname)
#endif
}
+static bool uuid_is_zero(__u8 u[16])
+{
+ int i;
+
+ for (i = 0; i < 16; i++)
+ if (u[i])
+ return false;
+ return true;
+}
+
+int ext4_ioctl_get_encryption_pwsalt(struct file *filp, void __user *arg)
+{
+ struct super_block *sb = file_inode(filp)->i_sb;
+ struct ext4_sb_info *sbi = EXT4_SB(sb);
+ int err, err2;
+ handle_t *handle;
+
+ if (!ext4_has_feature_encrypt(sb))
+ return -EOPNOTSUPP;
+
+ if (uuid_is_zero(sbi->s_es->s_encrypt_pw_salt)) {
+ err = mnt_want_write_file(filp);
+ if (err)
+ return err;
+ handle = ext4_journal_start_sb(sb, EXT4_HT_MISC, 1);
+ if (IS_ERR(handle)) {
+ err = PTR_ERR(handle);
+ goto pwsalt_err_exit;
+ }
+ err = ext4_journal_get_write_access(handle, sb, sbi->s_sbh,
+ EXT4_JTR_NONE);
+ if (err)
+ goto pwsalt_err_journal;
+ lock_buffer(sbi->s_sbh);
+ generate_random_uuid(sbi->s_es->s_encrypt_pw_salt);
+ ext4_superblock_csum_set(sb);
+ unlock_buffer(sbi->s_sbh);
+ err = ext4_handle_dirty_metadata(handle, NULL, sbi->s_sbh);
+pwsalt_err_journal:
+ err2 = ext4_journal_stop(handle);
+ if (err2 && !err)
+ err = err2;
+pwsalt_err_exit:
+ mnt_drop_write_file(filp);
+ if (err)
+ return err;
+ }
+
+ if (copy_to_user(arg, sbi->s_es->s_encrypt_pw_salt, 16))
+ return -EFAULT;
+ return 0;
+}
+
static int ext4_get_context(struct inode *inode, void *ctx, size_t len)
{
return ext4_xattr_get(inode, EXT4_XATTR_INDEX_ENCRYPTION,
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 3c474c9623af..ec859b42dafd 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2743,6 +2743,8 @@ int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry,
void ext4_fname_free_filename(struct ext4_filename *fname);
+int ext4_ioctl_get_encryption_pwsalt(struct file *filp, void __user *arg);
+
#else /* !CONFIG_FS_ENCRYPTION */
static inline int ext4_fname_setup_filename(struct inode *dir,
const struct qstr *iname,
@@ -2775,6 +2777,12 @@ static inline void ext4_fname_free_filename(struct ext4_filename *fname)
fname->cf_name.name = NULL;
#endif
}
+
+static inline int ext4_ioctl_get_encryption_pwsalt(struct file *filp,
+ void __user *arg)
+{
+ return -EOPNOTSUPP;
+}
#endif /* !CONFIG_FS_ENCRYPTION */
/* dir.c */
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index ba44fa1be70a..d8639aaed3f6 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -16,7 +16,6 @@
#include <linux/file.h>
#include <linux/quotaops.h>
#include <linux/random.h>
-#include <linux/uuid.h>
#include <linux/uaccess.h>
#include <linux/delay.h>
#include <linux/iversion.h>
@@ -504,18 +503,6 @@ static long swap_inode_boot_loader(struct super_block *sb,
return err;
}
-#ifdef CONFIG_FS_ENCRYPTION
-static int uuid_is_zero(__u8 u[16])
-{
- int i;
-
- for (i = 0; i < 16; i++)
- if (u[i])
- return 0;
- return 1;
-}
-#endif
-
/*
* If immutable is set and we are not clearing it, we're not allowed to change
* anything else in the inode. Don't error out if we're only trying to set
@@ -1432,51 +1419,9 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
return -EOPNOTSUPP;
return fscrypt_ioctl_set_policy(filp, (const void __user *)arg);
- case FS_IOC_GET_ENCRYPTION_PWSALT: {
-#ifdef CONFIG_FS_ENCRYPTION
- int err, err2;
- struct ext4_sb_info *sbi = EXT4_SB(sb);
- handle_t *handle;
+ case FS_IOC_GET_ENCRYPTION_PWSALT:
+ return ext4_ioctl_get_encryption_pwsalt(filp, (void __user *)arg);
- if (!ext4_has_feature_encrypt(sb))
- return -EOPNOTSUPP;
- if (uuid_is_zero(sbi->s_es->s_encrypt_pw_salt)) {
- err = mnt_want_write_file(filp);
- if (err)
- return err;
- handle = ext4_journal_start_sb(sb, EXT4_HT_MISC, 1);
- if (IS_ERR(handle)) {
- err = PTR_ERR(handle);
- goto pwsalt_err_exit;
- }
- err = ext4_journal_get_write_access(handle, sb,
- sbi->s_sbh,
- EXT4_JTR_NONE);
- if (err)
- goto pwsalt_err_journal;
- lock_buffer(sbi->s_sbh);
- generate_random_uuid(sbi->s_es->s_encrypt_pw_salt);
- ext4_superblock_csum_set(sb);
- unlock_buffer(sbi->s_sbh);
- err = ext4_handle_dirty_metadata(handle, NULL,
- sbi->s_sbh);
- pwsalt_err_journal:
- err2 = ext4_journal_stop(handle);
- if (err2 && !err)
- err = err2;
- pwsalt_err_exit:
- mnt_drop_write_file(filp);
- if (err)
- return err;
- }
- if (copy_to_user((void __user *) arg,
- sbi->s_es->s_encrypt_pw_salt, 16))
- return -EFAULT;
- return 0;
-#else
- return -EOPNOTSUPP;
-#endif
- }
case FS_IOC_GET_ENCRYPTION_POLICY:
if (!ext4_has_feature_encrypt(sb))
return -EOPNOTSUPP;
--
2.31.1
On Sun, 15 May 2022 12:07:45 +0530, Ritesh Harjani wrote:
> Please find the v3 of this cleanup series. Thanks to Eric for his quick
> review of the patch series.
>
> Description
> =============
> 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 fs encryption related
> definitions to fs/ext4/crypto.c
>
> [...]
Applied, thanks!
[1/3] ext4: Move ext4 crypto code to its own file crypto.c
commit: ebe541bdc293d4b2511bc4abb640dcddd454e54c
[2/3] ext4: Cleanup function defs from ext4.h into crypto.c
commit: df56bae5a36f891021ea868657ab85f501d85176
[3/3] ext4: Refactor and move ext4_ioctl_get_encryption_pwsalt()
commit: a137c5b48cb48b6c2885eeeec398433a435cf078
Best regards,
--
Theodore Ts'o <[email protected]>