fscrypt: stop holding extra request_queue references
authorEric Biggers <ebiggers@google.com>
Thu, 1 Sep 2022 19:32:07 +0000 (12:32 -0700)
committerEric Biggers <ebiggers@google.com>
Thu, 22 Sep 2022 03:33:06 +0000 (20:33 -0700)
Now that the fscrypt_master_key lifetime has been reworked to not be
subject to the quirks of the keyrings subsystem, blk_crypto_evict_key()
no longer gets called after the filesystem has already been unmounted.
Therefore, there is no longer any need to hold extra references to the
filesystem's request_queue(s).  (And these references didn't always do
their intended job anyway, as pinning a request_queue doesn't
necessarily pin the corresponding blk_crypto_profile.)

Stop taking these extra references.  Instead, just pass the super_block
to fscrypt_destroy_inline_crypt_key(), and use it to get the list of
block devices the key needs to be evicted from.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Link: https://lore.kernel.org/r/20220901193208.138056-3-ebiggers@kernel.org
fs/crypto/fscrypt_private.h
fs/crypto/inline_crypt.c
fs/crypto/keyring.c
fs/crypto/keysetup.c
fs/crypto/keysetup_v1.c

index 577cae7facb013324c76f0321938acde49073cb2..d5f68a0c5d1546403775e4544cf2f56a353c24f2 100644 (file)
@@ -184,7 +184,7 @@ struct fscrypt_symlink_data {
 struct fscrypt_prepared_key {
        struct crypto_skcipher *tfm;
 #ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
-       struct fscrypt_blk_crypto_key *blk_key;
+       struct blk_crypto_key *blk_key;
 #endif
 };
 
@@ -344,7 +344,8 @@ int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
                                     const u8 *raw_key,
                                     const struct fscrypt_info *ci);
 
-void fscrypt_destroy_inline_crypt_key(struct fscrypt_prepared_key *prep_key);
+void fscrypt_destroy_inline_crypt_key(struct super_block *sb,
+                                     struct fscrypt_prepared_key *prep_key);
 
 /*
  * Check whether the crypto transform or blk-crypto key has been allocated in
@@ -390,7 +391,8 @@ fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
 }
 
 static inline void
-fscrypt_destroy_inline_crypt_key(struct fscrypt_prepared_key *prep_key)
+fscrypt_destroy_inline_crypt_key(struct super_block *sb,
+                                struct fscrypt_prepared_key *prep_key)
 {
 }
 
@@ -600,7 +602,8 @@ extern struct fscrypt_mode fscrypt_modes[];
 int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
                        const u8 *raw_key, const struct fscrypt_info *ci);
 
-void fscrypt_destroy_prepared_key(struct fscrypt_prepared_key *prep_key);
+void fscrypt_destroy_prepared_key(struct super_block *sb,
+                                 struct fscrypt_prepared_key *prep_key);
 
 int fscrypt_set_per_file_enc_key(struct fscrypt_info *ci, const u8 *raw_key);
 
index 90f3e68f166e393fe6b14f5e54554abd20ee0f52..7d1e2ec722538acd39d0890396e9ce1739d674ef 100644 (file)
 
 #include "fscrypt_private.h"
 
-struct fscrypt_blk_crypto_key {
-       struct blk_crypto_key base;
-       int num_devs;
-       struct request_queue *devs[];
-};
-
 static int fscrypt_get_num_devices(struct super_block *sb)
 {
        if (sb->s_cop->get_num_devices)
@@ -162,49 +156,42 @@ int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
        const struct inode *inode = ci->ci_inode;
        struct super_block *sb = inode->i_sb;
        enum blk_crypto_mode_num crypto_mode = ci->ci_mode->blk_crypto_mode;
-       int num_devs = fscrypt_get_num_devices(sb);
-       int queue_refs = 0;
-       struct fscrypt_blk_crypto_key *blk_key;
+       struct blk_crypto_key *blk_key;
+       struct request_queue **devs;
+       unsigned int num_devs;
+       unsigned int i;
        int err;
-       int i;
 
-       blk_key = kzalloc(struct_size(blk_key, devs, num_devs), GFP_KERNEL);
+       blk_key = kmalloc(sizeof(*blk_key), GFP_KERNEL);
        if (!blk_key)
                return -ENOMEM;
 
-       blk_key->num_devs = num_devs;
-       fscrypt_get_devices(sb, num_devs, blk_key->devs);
-
-       err = blk_crypto_init_key(&blk_key->base, raw_key, crypto_mode,
+       err = blk_crypto_init_key(blk_key, raw_key, crypto_mode,
                                  fscrypt_get_dun_bytes(ci), sb->s_blocksize);
        if (err) {
                fscrypt_err(inode, "error %d initializing blk-crypto key", err);
                goto fail;
        }
 
-       /*
-        * We have to start using blk-crypto on all the filesystem's devices.
-        * We also have to save all the request_queue's for later so that the
-        * key can be evicted from them.  This is needed because some keys
-        * aren't destroyed until after the filesystem was already unmounted
-        * (namely, the per-mode keys in struct fscrypt_master_key).
-        */
+       /* Start using blk-crypto on all the filesystem's block devices. */
+       num_devs = fscrypt_get_num_devices(sb);
+       devs = kmalloc_array(num_devs, sizeof(*devs), GFP_KERNEL);
+       if (!devs) {
+               err = -ENOMEM;
+               goto fail;
+       }
+       fscrypt_get_devices(sb, num_devs, devs);
        for (i = 0; i < num_devs; i++) {
-               if (!blk_get_queue(blk_key->devs[i])) {
-                       fscrypt_err(inode, "couldn't get request_queue");
-                       err = -EAGAIN;
-                       goto fail;
-               }
-               queue_refs++;
-
-               err = blk_crypto_start_using_key(&blk_key->base,
-                                                blk_key->devs[i]);
-               if (err) {
-                       fscrypt_err(inode,
-                                   "error %d starting to use blk-crypto", err);
-                       goto fail;
-               }
+               err = blk_crypto_start_using_key(blk_key, devs[i]);
+               if (err)
+                       break;
        }
+       kfree(devs);
+       if (err) {
+               fscrypt_err(inode, "error %d starting to use blk-crypto", err);
+               goto fail;
+       }
+
        /*
         * Pairs with the smp_load_acquire() in fscrypt_is_key_prepared().
         * I.e., here we publish ->blk_key with a RELEASE barrier so that
@@ -215,24 +202,31 @@ int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
        return 0;
 
 fail:
-       for (i = 0; i < queue_refs; i++)
-               blk_put_queue(blk_key->devs[i]);
        kfree_sensitive(blk_key);
        return err;
 }
 
-void fscrypt_destroy_inline_crypt_key(struct fscrypt_prepared_key *prep_key)
+void fscrypt_destroy_inline_crypt_key(struct super_block *sb,
+                                     struct fscrypt_prepared_key *prep_key)
 {
-       struct fscrypt_blk_crypto_key *blk_key = prep_key->blk_key;
-       int i;
+       struct blk_crypto_key *blk_key = prep_key->blk_key;
+       struct request_queue **devs;
+       unsigned int num_devs;
+       unsigned int i;
 
-       if (blk_key) {
-               for (i = 0; i < blk_key->num_devs; i++) {
-                       blk_crypto_evict_key(blk_key->devs[i], &blk_key->base);
-                       blk_put_queue(blk_key->devs[i]);
-               }
-               kfree_sensitive(blk_key);
+       if (!blk_key)
+               return;
+
+       /* Evict the key from all the filesystem's block devices. */
+       num_devs = fscrypt_get_num_devices(sb);
+       devs = kmalloc_array(num_devs, sizeof(*devs), GFP_KERNEL);
+       if (devs) {
+               fscrypt_get_devices(sb, num_devs, devs);
+               for (i = 0; i < num_devs; i++)
+                       blk_crypto_evict_key(devs[i], blk_key);
+               kfree(devs);
        }
+       kfree_sensitive(blk_key);
 }
 
 bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode)
@@ -282,7 +276,7 @@ void fscrypt_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
        ci = inode->i_crypt_info;
 
        fscrypt_generate_dun(ci, first_lblk, dun);
-       bio_crypt_set_ctx(bio, &ci->ci_enc_key.blk_key->base, dun, gfp_mask);
+       bio_crypt_set_ctx(bio, ci->ci_enc_key.blk_key, dun, gfp_mask);
 }
 EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx);
 
@@ -369,7 +363,7 @@ bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
         * uses the same pointer.  I.e., there's currently no need to support
         * merging requests where the keys are the same but the pointers differ.
         */
-       if (bc->bc_key != &inode->i_crypt_info->ci_enc_key.blk_key->base)
+       if (bc->bc_key != inode->i_crypt_info->ci_enc_key.blk_key)
                return false;
 
        fscrypt_generate_dun(inode->i_crypt_info, next_lblk, next_dun);
index 9b98d6a576e6a0945d966c3e51d8167c6df6ba68..1cca09aa43f8b38fdce79258be9b29e9f0986b37 100644 (file)
@@ -105,9 +105,12 @@ void fscrypt_put_master_key_activeref(struct fscrypt_master_key *mk)
        WARN_ON(!list_empty(&mk->mk_decrypted_inodes));
 
        for (i = 0; i <= FSCRYPT_MODE_MAX; i++) {
-               fscrypt_destroy_prepared_key(&mk->mk_direct_keys[i]);
-               fscrypt_destroy_prepared_key(&mk->mk_iv_ino_lblk_64_keys[i]);
-               fscrypt_destroy_prepared_key(&mk->mk_iv_ino_lblk_32_keys[i]);
+               fscrypt_destroy_prepared_key(
+                               sb, &mk->mk_direct_keys[i]);
+               fscrypt_destroy_prepared_key(
+                               sb, &mk->mk_iv_ino_lblk_64_keys[i]);
+               fscrypt_destroy_prepared_key(
+                               sb, &mk->mk_iv_ino_lblk_32_keys[i]);
        }
        memzero_explicit(&mk->mk_ino_hash_key,
                         sizeof(mk->mk_ino_hash_key));
index e037a7b8e9e42bdb58bbdc67010781dfeab232ff..f7407071a952424ed236a67cb0465edbdeaa2d22 100644 (file)
@@ -154,10 +154,11 @@ int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
 }
 
 /* Destroy a crypto transform object and/or blk-crypto key. */
-void fscrypt_destroy_prepared_key(struct fscrypt_prepared_key *prep_key)
+void fscrypt_destroy_prepared_key(struct super_block *sb,
+                                 struct fscrypt_prepared_key *prep_key)
 {
        crypto_free_skcipher(prep_key->tfm);
-       fscrypt_destroy_inline_crypt_key(prep_key);
+       fscrypt_destroy_inline_crypt_key(sb, prep_key);
        memzero_explicit(prep_key, sizeof(*prep_key));
 }
 
@@ -494,7 +495,8 @@ static void put_crypt_info(struct fscrypt_info *ci)
        if (ci->ci_direct_key)
                fscrypt_put_direct_key(ci->ci_direct_key);
        else if (ci->ci_owns_key)
-               fscrypt_destroy_prepared_key(&ci->ci_enc_key);
+               fscrypt_destroy_prepared_key(ci->ci_inode->i_sb,
+                                            &ci->ci_enc_key);
 
        mk = ci->ci_master_key;
        if (mk) {
index 2762c53504323fff6ed531c32dfffeae46f11eed..75dabd9b27f9b6ce6c8f78d49e46ae76c22337c9 100644 (file)
@@ -143,6 +143,7 @@ invalid:
 
 /* Master key referenced by DIRECT_KEY policy */
 struct fscrypt_direct_key {
+       struct super_block              *dk_sb;
        struct hlist_node               dk_node;
        refcount_t                      dk_refcount;
        const struct fscrypt_mode       *dk_mode;
@@ -154,7 +155,7 @@ struct fscrypt_direct_key {
 static void free_direct_key(struct fscrypt_direct_key *dk)
 {
        if (dk) {
-               fscrypt_destroy_prepared_key(&dk->dk_key);
+               fscrypt_destroy_prepared_key(dk->dk_sb, &dk->dk_key);
                kfree_sensitive(dk);
        }
 }
@@ -231,6 +232,7 @@ fscrypt_get_direct_key(const struct fscrypt_info *ci, const u8 *raw_key)
        dk = kzalloc(sizeof(*dk), GFP_KERNEL);
        if (!dk)
                return ERR_PTR(-ENOMEM);
+       dk->dk_sb = ci->ci_inode->i_sb;
        refcount_set(&dk->dk_refcount, 1);
        dk->dk_mode = ci->ci_mode;
        err = fscrypt_prepare_key(&dk->dk_key, raw_key, ci);