| Message ID | 62170e01a2c0b107619018c859250c03b6023a57.1691505882.git.sweettea-kernel@dorminy.me (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fscrypt: add extent encryption | expand |
On Tue, Aug 08, 2023 at 01:08:31PM -0400, Sweet Tea Dorminy wrote: > btrfs sometimes frees extents while holding a mutex, which makes it > impossible to free an inlinecrypt prepared key since that requires > taking a semaphore. Therefore, we will need to offload prepared key > freeing into an asynchronous process (rcu is insufficient since that can > run in softirq context which is also incompatible with taking a > semaphore). In order to avoid use-after-free on the filesystem > superblock for keys being freed during shutdown, we need to cache the > list of devices that the key has been loaded into, so that we can later > remove it without reference to the superblock. > > Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me> > --- > fs/crypto/fscrypt_private.h | 13 +++++++++++-- > fs/crypto/inline_crypt.c | 20 +++++++++----------- > fs/crypto/keysetup.c | 2 +- > 3 files changed, 21 insertions(+), 14 deletions(-) > > diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h > index 03be2c136c0e..aba83509c735 100644 > --- a/fs/crypto/fscrypt_private.h > +++ b/fs/crypto/fscrypt_private.h > @@ -205,6 +205,16 @@ struct fscrypt_prepared_key { > struct crypto_skcipher *tfm; > #ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT > struct blk_crypto_key *blk_key; > + > + /* > + * The list of devices that have this block key. > + */ > + struct block_device **devices; > + > + /* > + * The number of devices in @ci_devices. > + */ > + size_t device_count; > #endif > enum fscrypt_prepared_key_type type; > }; Well, this is basically reverting commit 22e9947a4b2b, but doing it in a slightly different way. I worry about potentially bringing back problems from doing work after the filesystem has already been unmounted. Can't you just make the filesystem flush its eviction work items when it is being unmounted? - Eric
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h index 03be2c136c0e..aba83509c735 100644 --- a/fs/crypto/fscrypt_private.h +++ b/fs/crypto/fscrypt_private.h @@ -205,6 +205,16 @@ struct fscrypt_prepared_key { struct crypto_skcipher *tfm; #ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT struct blk_crypto_key *blk_key; + + /* + * The list of devices that have this block key. + */ + struct block_device **devices; + + /* + * The number of devices in @ci_devices. + */ + size_t device_count; #endif enum fscrypt_prepared_key_type type; }; @@ -470,8 +480,7 @@ 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 super_block *sb, - struct fscrypt_prepared_key *prep_key); +void fscrypt_destroy_inline_crypt_key(struct fscrypt_prepared_key *prep_key); /* * Check whether the crypto transform or blk-crypto key has been allocated in diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c index 76274b736e1a..91f8365b4194 100644 --- a/fs/crypto/inline_crypt.c +++ b/fs/crypto/inline_crypt.c @@ -185,12 +185,15 @@ int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key, if (err) break; } - kfree(devs); + if (err) { fscrypt_err(inode, "error %d starting to use blk-crypto", err); goto fail; } + prep_key->devices = devs; + prep_key->device_count = num_devs; + /* * Pairs with the smp_load_acquire() in fscrypt_is_key_prepared(). * I.e., here we publish ->blk_key with a RELEASE barrier so that @@ -205,24 +208,19 @@ int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key, return err; } -void fscrypt_destroy_inline_crypt_key(struct super_block *sb, - struct fscrypt_prepared_key *prep_key) +void fscrypt_destroy_inline_crypt_key(struct fscrypt_prepared_key *prep_key) { struct blk_crypto_key *blk_key = prep_key->blk_key; - struct block_device **devs; - unsigned int num_devs; unsigned int i; if (!blk_key) return; /* Evict the key from all the filesystem's block devices. */ - devs = fscrypt_get_devices(sb, &num_devs); - if (!IS_ERR(devs)) { - for (i = 0; i < num_devs; i++) - blk_crypto_evict_key(devs[i], blk_key); - kfree(devs); - } + for (i = 0; i < prep_key->device_count; i++) + blk_crypto_evict_key(prep_key->devices[i], blk_key); + + kfree(prep_key->devices); kfree_sensitive(blk_key); } diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c index 12c3851b7cd6..fe246229c869 100644 --- a/fs/crypto/keysetup.c +++ b/fs/crypto/keysetup.c @@ -195,7 +195,7 @@ 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(sb, prep_key); + fscrypt_destroy_inline_crypt_key(prep_key); memzero_explicit(prep_key, sizeof(*prep_key)); }
btrfs sometimes frees extents while holding a mutex, which makes it impossible to free an inlinecrypt prepared key since that requires taking a semaphore. Therefore, we will need to offload prepared key freeing into an asynchronous process (rcu is insufficient since that can run in softirq context which is also incompatible with taking a semaphore). In order to avoid use-after-free on the filesystem superblock for keys being freed during shutdown, we need to cache the list of devices that the key has been loaded into, so that we can later remove it without reference to the superblock. Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me> --- fs/crypto/fscrypt_private.h | 13 +++++++++++-- fs/crypto/inline_crypt.c | 20 +++++++++----------- fs/crypto/keysetup.c | 2 +- 3 files changed, 21 insertions(+), 14 deletions(-)