diff mbox series

[v6,3/8] fscrypt: split setup_per_mode_enc_key()

Message ID cd8fffce4ec37af89c5f92fd8077e7f055fccc0e.1691505830.git.sweettea-kernel@dorminy.me (mailing list archive)
State Superseded
Headers show
Series fscrypt: preliminary rearrangmeents of key setup | expand

Commit Message

Sweet Tea Dorminy Aug. 8, 2023, 5:08 p.m. UTC 
At present, setup_per_mode_enc_key() tries to find, within an array of
mode keys in the master key, an already prepared key, and if it doesn't
find a pre-prepared key, sets up a new one. This caching is not super
clear, at least to me, and splitting this function makes it clearer.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/keysetup.c | 61 +++++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 24 deletions(-)

Comments

Josef Bacik Aug. 9, 2023, 5:22 p.m. UTC | #1 
On Tue, Aug 08, 2023 at 01:08:03PM -0400, Sweet Tea Dorminy wrote:
> At present, setup_per_mode_enc_key() tries to find, within an array of
> mode keys in the master key, an already prepared key, and if it doesn't
> find a pre-prepared key, sets up a new one. This caching is not super
> clear, at least to me, and splitting this function makes it clearer.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef
Eric Biggers Aug. 10, 2023, 6:37 a.m. UTC | #2 
On Tue, Aug 08, 2023 at 01:08:03PM -0400, Sweet Tea Dorminy wrote:
> +static int find_mode_prepared_key(struct fscrypt_info *ci,
> +				  struct fscrypt_master_key *mk,
> +				  struct fscrypt_prepared_key *keys,
> +				  u8 hkdf_context, bool include_fs_uuid)
> +{
> +	struct fscrypt_mode *mode = ci->ci_mode;
> +	const u8 mode_num = mode - fscrypt_modes;
> +	struct fscrypt_prepared_key *prep_key;
> +	int err;
> +
> +	if (WARN_ON_ONCE(mode_num > FSCRYPT_MODE_MAX))
> +		return -EINVAL;
> +
> +	prep_key = &keys[mode_num];
> +	if (fscrypt_is_key_prepared(prep_key, ci)) {
> +		ci->ci_enc_key = *prep_key;
> +		return 0;
> +	}
> +	err = setup_new_mode_prepared_key(mk, prep_key, ci, hkdf_context,
> +					  include_fs_uuid);
> +	if (err)
> +		return err;
> +
> +	ci->ci_enc_key = *prep_key;
> +	return 0;
> +}

Any thoughts about the feedback I gave about this on v2
(https://lore.kernel.org/linux-fscrypt/20230411032935.GC47625@sol.localdomain/)?
The new naming with "find" seems wrong.

- Eric
diff mbox series

Patch

diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 727d473b6b03..06deac6f4487 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -184,34 +184,24 @@  int fscrypt_set_per_file_enc_key(struct fscrypt_info *ci, const u8 *raw_key)
 	return fscrypt_prepare_key(&ci->ci_enc_key, raw_key, ci);
 }
 
-static int setup_per_mode_enc_key(struct fscrypt_info *ci,
-				  struct fscrypt_master_key *mk,
-				  struct fscrypt_prepared_key *keys,
-				  u8 hkdf_context, bool include_fs_uuid)
+static int setup_new_mode_prepared_key(struct fscrypt_master_key *mk,
+				       struct fscrypt_prepared_key *prep_key,
+				       const struct fscrypt_info *ci,
+				       u8 hkdf_context, bool include_fs_uuid)
 {
 	const struct inode *inode = ci->ci_inode;
 	const struct super_block *sb = inode->i_sb;
 	struct fscrypt_mode *mode = ci->ci_mode;
 	const u8 mode_num = mode - fscrypt_modes;
-	struct fscrypt_prepared_key *prep_key;
 	u8 mode_key[FSCRYPT_MAX_KEY_SIZE];
 	u8 hkdf_info[sizeof(mode_num) + sizeof(sb->s_uuid)];
 	unsigned int hkdf_infolen = 0;
-	int err;
-
-	if (WARN_ON_ONCE(mode_num > FSCRYPT_MODE_MAX))
-		return -EINVAL;
-
-	prep_key = &keys[mode_num];
-	if (fscrypt_is_key_prepared(prep_key, ci)) {
-		ci->ci_enc_key = *prep_key;
-		return 0;
-	}
+	int err = 0;
 
 	mutex_lock(&fscrypt_mode_key_setup_mutex);
 
 	if (fscrypt_is_key_prepared(prep_key, ci))
-		goto done_unlock;
+		goto out_unlock;
 
 	BUILD_BUG_ON(sizeof(mode_num) != 1);
 	BUILD_BUG_ON(sizeof(sb->s_uuid) != 16);
@@ -229,16 +219,39 @@  static int setup_per_mode_enc_key(struct fscrypt_info *ci,
 		goto out_unlock;
 	err = fscrypt_prepare_key(prep_key, mode_key, ci);
 	memzero_explicit(mode_key, mode->keysize);
-	if (err)
-		goto out_unlock;
-done_unlock:
-	ci->ci_enc_key = *prep_key;
-	err = 0;
+
 out_unlock:
 	mutex_unlock(&fscrypt_mode_key_setup_mutex);
 	return err;
 }
 
+static int find_mode_prepared_key(struct fscrypt_info *ci,
+				  struct fscrypt_master_key *mk,
+				  struct fscrypt_prepared_key *keys,
+				  u8 hkdf_context, bool include_fs_uuid)
+{
+	struct fscrypt_mode *mode = ci->ci_mode;
+	const u8 mode_num = mode - fscrypt_modes;
+	struct fscrypt_prepared_key *prep_key;
+	int err;
+
+	if (WARN_ON_ONCE(mode_num > FSCRYPT_MODE_MAX))
+		return -EINVAL;
+
+	prep_key = &keys[mode_num];
+	if (fscrypt_is_key_prepared(prep_key, ci)) {
+		ci->ci_enc_key = *prep_key;
+		return 0;
+	}
+	err = setup_new_mode_prepared_key(mk, prep_key, ci, hkdf_context,
+					  include_fs_uuid);
+	if (err)
+		return err;
+
+	ci->ci_enc_key = *prep_key;
+	return 0;
+}
+
 /*
  * Derive a SipHash key from the given fscrypt master key and the given
  * application-specific information string.
@@ -294,7 +307,7 @@  static int fscrypt_setup_iv_ino_lblk_32_key(struct fscrypt_info *ci,
 {
 	int err;
 
-	err = setup_per_mode_enc_key(ci, mk, mk->mk_iv_ino_lblk_32_keys,
+	err = find_mode_prepared_key(ci, mk, mk->mk_iv_ino_lblk_32_keys,
 				     HKDF_CONTEXT_IV_INO_LBLK_32_KEY, true);
 	if (err)
 		return err;
@@ -344,7 +357,7 @@  static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
 		 * encryption key.  This ensures that the master key is
 		 * consistently used only for HKDF, avoiding key reuse issues.
 		 */
-		err = setup_per_mode_enc_key(ci, mk, mk->mk_direct_keys,
+		err = find_mode_prepared_key(ci, mk, mk->mk_direct_keys,
 					     HKDF_CONTEXT_DIRECT_KEY, false);
 	} else if (ci->ci_policy.v2.flags &
 		   FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64) {
@@ -354,7 +367,7 @@  static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
 		 * the IVs.  This format is optimized for use with inline
 		 * encryption hardware compliant with the UFS standard.
 		 */
-		err = setup_per_mode_enc_key(ci, mk, mk->mk_iv_ino_lblk_64_keys,
+		err = find_mode_prepared_key(ci, mk, mk->mk_iv_ino_lblk_64_keys,
 					     HKDF_CONTEXT_IV_INO_LBLK_64_KEY,
 					     true);
 	} else if (ci->ci_policy.v2.flags &