diff mbox series

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

Message ID 0ba2cb228aa367aea1442b8f1433f229040fe8dd.1687988119.git.sweettea-kernel@dorminy.me (mailing list archive)
State Superseded
Headers show
Series fscrypt: some rearrangements of key setup | expand

Commit Message

Sweet Tea Dorminy June 29, 2023, 12:28 a.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 | 59 +++++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 23 deletions(-)

Comments

kernel test robot June 29, 2023, 2:44 a.m. UTC | #1 
Hi Sweet,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 00bc86ea26ac88043f48916c273afc9fbb40c73f]

url:    https://github.com/intel-lab-lkp/linux/commits/Sweet-Tea-Dorminy/fscrypt-move-inline-crypt-decision-to-info-setup/20230629-083929
base:   00bc86ea26ac88043f48916c273afc9fbb40c73f
patch link:    https://lore.kernel.org/r/0ba2cb228aa367aea1442b8f1433f229040fe8dd.1687988119.git.sweettea-kernel%40dorminy.me
patch subject: [PATCH v4 3/8] fscrypt: split setup_per_mode_enc_key()
config: um-randconfig-r014-20230628 (https://download.01.org/0day-ci/archive/20230629/202306291013.yc7R9Rb7-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230629/202306291013.yc7R9Rb7-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/202306291013.yc7R9Rb7-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from fs/crypto/keysetup.c:14:
   In file included from fs/crypto/fscrypt_private.h:17:
   In file included from include/linux/blk-crypto.h:72:
   In file included from include/linux/blk_types.h:10:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from fs/crypto/keysetup.c:14:
   In file included from fs/crypto/fscrypt_private.h:17:
   In file included from include/linux/blk-crypto.h:72:
   In file included from include/linux/blk_types.h:10:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from fs/crypto/keysetup.c:14:
   In file included from fs/crypto/fscrypt_private.h:17:
   In file included from include/linux/blk-crypto.h:72:
   In file included from include/linux/blk_types.h:10:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> fs/crypto/keysetup.c:203:2: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (fscrypt_is_key_prepared(prep_key, ci))
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:55:28: note: expanded from macro 'if'
   #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:57:30: note: expanded from macro '__trace_if_var'
   #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/crypto/keysetup.c:225:9: note: uninitialized use occurs here
           return err;
                  ^~~
   fs/crypto/keysetup.c:203:2: note: remove the 'if' if its condition is always false
           if (fscrypt_is_key_prepared(prep_key, ci))
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:55:23: note: expanded from macro 'if'
   #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                         ^
   fs/crypto/keysetup.c:199:9: note: initialize the variable 'err' to silence this warning
           int err;
                  ^
                   = 0
   13 warnings generated.


vim +203 fs/crypto/keysetup.c

8094c3ceb21ad93 fs/crypto/keyinfo.c  Eric Biggers      2019-01-06  186  
7b4d2231643f5a9 fs/crypto/keysetup.c Sweet Tea Dorminy 2023-06-28  187  static int setup_new_mode_prepared_key(struct fscrypt_master_key *mk,
7b4d2231643f5a9 fs/crypto/keysetup.c Sweet Tea Dorminy 2023-06-28  188  				       struct fscrypt_prepared_key *prep_key,
7b4d2231643f5a9 fs/crypto/keysetup.c Sweet Tea Dorminy 2023-06-28  189  				       const struct fscrypt_info *ci,
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  190  				       u8 hkdf_context, bool include_fs_uuid)
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  191  {
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  192  	const struct inode *inode = ci->ci_inode;
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  193  	const struct super_block *sb = inode->i_sb;
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  194  	struct fscrypt_mode *mode = ci->ci_mode;
85af90e57ce9697 fs/crypto/keysetup.c Eric Biggers      2019-12-09  195  	const u8 mode_num = mode - fscrypt_modes;
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  196  	u8 mode_key[FSCRYPT_MAX_KEY_SIZE];
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  197  	u8 hkdf_info[sizeof(mode_num) + sizeof(sb->s_uuid)];
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  198  	unsigned int hkdf_infolen = 0;
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  199  	int err;
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  200  
e3b1078bedd323d fs/crypto/keysetup.c Eric Biggers      2020-05-15  201  	mutex_lock(&fscrypt_mode_key_setup_mutex);
e3b1078bedd323d fs/crypto/keysetup.c Eric Biggers      2020-05-15  202  
5fee36095cda45d fs/crypto/keysetup.c Satya Tangirala   2020-07-02 @203  	if (fscrypt_is_key_prepared(prep_key, ci))
7b4d2231643f5a9 fs/crypto/keysetup.c Sweet Tea Dorminy 2023-06-28  204  		goto out_unlock;
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  205  
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  206  	BUILD_BUG_ON(sizeof(mode_num) != 1);
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  207  	BUILD_BUG_ON(sizeof(sb->s_uuid) != 16);
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  208  	BUILD_BUG_ON(sizeof(hkdf_info) != 17);
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  209  	hkdf_info[hkdf_infolen++] = mode_num;
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  210  	if (include_fs_uuid) {
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  211  		memcpy(&hkdf_info[hkdf_infolen], &sb->s_uuid,
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  212  		       sizeof(sb->s_uuid));
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  213  		hkdf_infolen += sizeof(sb->s_uuid);
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  214  	}
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  215  	err = fscrypt_hkdf_expand(&mk->mk_secret.hkdf,
b103fb7653fff09 fs/crypto/keysetup.c Eric Biggers      2019-10-24  216  				  hkdf_context, hkdf_info, hkdf_infolen,
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  217  				  mode_key, mode->keysize);
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  218  	if (err)
e3b1078bedd323d fs/crypto/keysetup.c Eric Biggers      2020-05-15  219  		goto out_unlock;
5fee36095cda45d fs/crypto/keysetup.c Satya Tangirala   2020-07-02  220  	err = fscrypt_prepare_key(prep_key, mode_key, ci);
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  221  	memzero_explicit(mode_key, mode->keysize);
7b4d2231643f5a9 fs/crypto/keysetup.c Sweet Tea Dorminy 2023-06-28  222  
e3b1078bedd323d fs/crypto/keysetup.c Eric Biggers      2020-05-15  223  out_unlock:
e3b1078bedd323d fs/crypto/keysetup.c Eric Biggers      2020-05-15  224  	mutex_unlock(&fscrypt_mode_key_setup_mutex);
e3b1078bedd323d fs/crypto/keysetup.c Eric Biggers      2020-05-15  225  	return err;
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  226  }
5dae460c2292dbb fs/crypto/keysetup.c Eric Biggers      2019-08-04  227
diff mbox series

Patch

diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 727d473b6b03..69bd27b7e9d8 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;
-	}
-
 	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 &