* [PATCH v1 1/7] fscrypt: add new pooled prepared keys.
2023-04-19 2:42 [PATCH v1 0/7] fscrypt: add pooled prepared keys facility Sweet Tea Dorminy
@ 2023-04-19 2:42 ` Sweet Tea Dorminy
2023-04-19 2:42 ` [PATCH v1 2/7] fscrypt: set up pooled keys upon first use Sweet Tea Dorminy
` (6 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Sweet Tea Dorminy @ 2023-04-19 2:42 UTC (permalink / raw)
To: Eric Biggers, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt,
kernel-team
Cc: Sweet Tea Dorminy
For extent-based encryption, we need to avoid allocating prepared keys
at IO time, since crypto_skcipher allocation takes a lock and can do IO
itself. As such, we will need to use pooled prepared keys. This change
begins adding pooled prepared keys, but doesn't use them yet.
For testing, fscrypt_using_pooled_prepared_keys() can be changed to use
pooled keys for leaf inodes, so that pooled prepared keys are used for
contents encryption for v2 default-key policies.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/fscrypt_private.h | 9 ++++++
fs/crypto/keysetup.c | 62 ++++++++++++++++++++++++++++++++-----
2 files changed, 63 insertions(+), 8 deletions(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index eb302e342fb9..4942a8ae2061 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -30,6 +30,11 @@
#define FSCRYPT_CONTEXT_V1 1
#define FSCRYPT_CONTEXT_V2 2
+#define FSCRYPT_POLICY_FLAGS_KEY_MASK \
+ (FSCRYPT_POLICY_FLAG_DIRECT_KEY \
+ | FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64 \
+ | FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32)
+
/* Keep this in sync with include/uapi/linux/fscrypt.h */
#define FSCRYPT_MODE_MAX FSCRYPT_MODE_AES_256_HCTR2
@@ -185,11 +190,15 @@ struct fscrypt_symlink_data {
* part of a fscrypt_master_key, shared between all
* users of this master key having this mode and
* policy.
+ * @FSCRYPT_KEY_POOLED: this prepared key is embedded in a
+ * fscrypt_pooled_prepared_key. It should be returned to
+ * its pool when no longer in use.
*/
enum fscrypt_prepared_key_type {
FSCRYPT_KEY_PER_INFO = 1,
FSCRYPT_KEY_DIRECT_V1,
FSCRYPT_KEY_MASTER_KEY,
+ FSCRYPT_KEY_POOLED,
} __packed;
/**
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 9cd60e09b0c5..171114fd5590 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -89,6 +89,10 @@ struct fscrypt_mode fscrypt_modes[] = {
static DEFINE_MUTEX(fscrypt_mode_key_setup_mutex);
+struct fscrypt_pooled_prepared_key {
+ struct fscrypt_prepared_key prep_key;
+};
+
static struct fscrypt_mode *
select_encryption_mode(const union fscrypt_policy *policy,
const struct inode *inode)
@@ -117,6 +121,21 @@ static int lock_master_key(struct fscrypt_master_key *mk)
return 0;
}
+static inline bool
+fscrypt_using_pooled_prepared_key(const struct fscrypt_info *ci)
+{
+ if (ci->ci_policy.version != FSCRYPT_POLICY_V2)
+ return false;
+ if (ci->ci_policy.v2.flags & FSCRYPT_POLICY_FLAGS_KEY_MASK)
+ return false;
+ if (fscrypt_using_inline_encryption(ci))
+ return false;
+
+ if (!S_ISREG(ci->ci_inode->i_mode))
+ return false;
+ return false;
+}
+
/*
* Prepare the crypto transform object or blk-crypto key in @prep_key, given the
* raw key, encryption mode (@ci->ci_mode), flag indicating which encryption
@@ -140,7 +159,6 @@ int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
return err;
}
-
/* Create a symmetric cipher object for the given encryption mode */
static struct crypto_skcipher *
fscrypt_allocate_skcipher(struct fscrypt_mode *mode,
@@ -213,14 +231,31 @@ int fscrypt_set_per_file_enc_key(struct fscrypt_info *ci, const u8 *raw_key)
{
int err;
- ci->ci_enc_key = kzalloc(sizeof(*ci->ci_enc_key), GFP_KERNEL);
- if (!ci->ci_enc_key)
- return -ENOMEM;
+ if (fscrypt_using_pooled_prepared_key(ci)) {
+ struct fscrypt_pooled_prepared_key *pooled_key;
- ci->ci_enc_key->type = FSCRYPT_KEY_PER_INFO;
- err = fscrypt_allocate_key_member(ci->ci_enc_key, ci);
- if (err)
- return err;
+ pooled_key = kzalloc(sizeof(*pooled_key), GFP_KERNEL);
+ if (!pooled_key)
+ return -ENOMEM;
+
+ err = fscrypt_allocate_key_member(&pooled_key->prep_key, ci);
+ if (err) {
+ kfree(pooled_key);
+ return err;
+ }
+
+ pooled_key->prep_key.type = FSCRYPT_KEY_POOLED;
+ ci->ci_enc_key = &pooled_key->prep_key;
+ } else {
+ ci->ci_enc_key = kzalloc(sizeof(*ci->ci_enc_key), GFP_KERNEL);
+ if (!ci->ci_enc_key)
+ return -ENOMEM;
+
+ ci->ci_enc_key->type = FSCRYPT_KEY_PER_INFO;
+ err = fscrypt_allocate_key_member(ci->ci_enc_key, ci);
+ if (err)
+ return err;
+ }
return fscrypt_prepare_key(ci->ci_enc_key, raw_key, ci);
}
@@ -616,6 +651,17 @@ static void put_crypt_info(struct fscrypt_info *ci)
ci->ci_enc_key);
kfree_sensitive(ci->ci_enc_key);
}
+ if (type == FSCRYPT_KEY_POOLED) {
+ struct fscrypt_pooled_prepared_key *pooled_key;
+
+ pooled_key = container_of(ci->ci_enc_key,
+ struct fscrypt_pooled_prepared_key,
+ prep_key);
+
+ fscrypt_destroy_prepared_key(ci->ci_inode->i_sb,
+ ci->ci_enc_key);
+ kfree_sensitive(pooled_key);
+ }
}
mk = ci->ci_master_key;
--
2.40.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v1 2/7] fscrypt: set up pooled keys upon first use
2023-04-19 2:42 [PATCH v1 0/7] fscrypt: add pooled prepared keys facility Sweet Tea Dorminy
2023-04-19 2:42 ` [PATCH v1 1/7] fscrypt: add new pooled prepared keys Sweet Tea Dorminy
@ 2023-04-19 2:42 ` Sweet Tea Dorminy
2023-04-19 2:42 ` [PATCH v1 3/7] fscrypt: add pooling of pooled prepared keys Sweet Tea Dorminy
` (5 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Sweet Tea Dorminy @ 2023-04-19 2:42 UTC (permalink / raw)
To: Eric Biggers, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt,
kernel-team
Cc: Sweet Tea Dorminy
This change starts setting up pooled prepared keys only when they are
first used, which is a step toward how they will be used for
extent-based encryption. While currently they're still allocated as part
of the info, this defers actual key setup to usage time, and adds error
handling needed for this.
If used on leaf inodes, this passes most tests, except the tests which
assert that one can open a file, forget the master key, and still do IO
with the cached prepared key in the file's info. Since the master key is
used at first IO time to set up the actual prepared key, the expectation
that the prepared key is already cached is violated.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/crypto.c | 4 +++-
fs/crypto/fscrypt_private.h | 2 ++
fs/crypto/keysetup.c | 33 +++++++++++++++++++++++++++++++++
3 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 9f3bda18c797..91ef520a9961 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -108,9 +108,11 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
DECLARE_CRYPTO_WAIT(wait);
struct scatterlist dst, src;
struct fscrypt_info *ci = inode->i_crypt_info;
- struct crypto_skcipher *tfm = ci->ci_enc_key->tfm;
+ struct crypto_skcipher *tfm = fscrypt_get_contents_tfm(ci);
int res = 0;
+ if (IS_ERR(tfm))
+ return PTR_ERR(tfm);
if (WARN_ON_ONCE(len <= 0))
return -EINVAL;
if (WARN_ON_ONCE(len % FSCRYPT_CONTENTS_ALIGNMENT != 0))
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 4942a8ae2061..143dc039bb03 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -624,6 +624,8 @@ struct fscrypt_mode {
extern struct fscrypt_mode fscrypt_modes[];
+struct crypto_skcipher *fscrypt_get_contents_tfm(struct fscrypt_info *ci);
+
int fscrypt_allocate_key_member(struct fscrypt_prepared_key *prep_key,
const struct fscrypt_info *ci);
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 171114fd5590..c179a478020a 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -93,6 +93,10 @@ struct fscrypt_pooled_prepared_key {
struct fscrypt_prepared_key prep_key;
};
+/* Forward declaration so that all the prepared key handling can stay together */
+static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
+ struct fscrypt_master_key *mk);
+
static struct fscrypt_mode *
select_encryption_mode(const union fscrypt_policy *policy,
const struct inode *inode)
@@ -159,6 +163,31 @@ int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
return err;
}
+struct crypto_skcipher *fscrypt_get_contents_tfm(struct fscrypt_info *ci)
+{
+ int err;
+ struct fscrypt_master_key *mk = ci->ci_master_key;
+ unsigned int allocflags;
+
+ if (!fscrypt_using_pooled_prepared_key(ci))
+ return ci->ci_enc_key->tfm;
+
+ err = lock_master_key(mk);
+ if (err) {
+ up_read(&mk->mk_sem);
+ return ERR_PTR(err);
+ }
+
+ allocflags = memalloc_nofs_save();
+ err = fscrypt_setup_v2_file_key(ci, mk);
+ up_read(&mk->mk_sem);
+ memalloc_nofs_restore(allocflags);
+ if (err)
+ return ERR_PTR(err);
+
+ return ci->ci_enc_key->tfm;
+}
+
/* Create a symmetric cipher object for the given encryption mode */
static struct crypto_skcipher *
fscrypt_allocate_skcipher(struct fscrypt_mode *mode,
@@ -234,6 +263,9 @@ int fscrypt_set_per_file_enc_key(struct fscrypt_info *ci, const u8 *raw_key)
if (fscrypt_using_pooled_prepared_key(ci)) {
struct fscrypt_pooled_prepared_key *pooled_key;
+ if (ci->ci_enc_key)
+ return fscrypt_prepare_key(ci->ci_enc_key, raw_key, ci);
+
pooled_key = kzalloc(sizeof(*pooled_key), GFP_KERNEL);
if (!pooled_key)
return -ENOMEM;
@@ -246,6 +278,7 @@ int fscrypt_set_per_file_enc_key(struct fscrypt_info *ci, const u8 *raw_key)
pooled_key->prep_key.type = FSCRYPT_KEY_POOLED;
ci->ci_enc_key = &pooled_key->prep_key;
+ return 0;
} else {
ci->ci_enc_key = kzalloc(sizeof(*ci->ci_enc_key), GFP_KERNEL);
if (!ci->ci_enc_key)
--
2.40.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v1 3/7] fscrypt: add pooling of pooled prepared keys.
2023-04-19 2:42 [PATCH v1 0/7] fscrypt: add pooled prepared keys facility Sweet Tea Dorminy
2023-04-19 2:42 ` [PATCH v1 1/7] fscrypt: add new pooled prepared keys Sweet Tea Dorminy
2023-04-19 2:42 ` [PATCH v1 2/7] fscrypt: set up pooled keys upon first use Sweet Tea Dorminy
@ 2023-04-19 2:42 ` Sweet Tea Dorminy
2023-04-19 2:42 ` [PATCH v1 4/7] fscrypt: add pooled prepared key locking around use Sweet Tea Dorminy
` (4 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Sweet Tea Dorminy @ 2023-04-19 2:42 UTC (permalink / raw)
To: Eric Biggers, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt,
kernel-team
Cc: Sweet Tea Dorminy
This change adds the actual infrastructure to set up prepared key pools.
It sets up one key pool per mode per master key, starting out with one
prepared key each. However, it continues to allocate a new prepared key
at each new relevant info creation, which is added to the relevant pool
and then immediately pulled back out for the info.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/fscrypt_private.h | 17 +++
fs/crypto/keyring.c | 7 +
fs/crypto/keysetup.c | 273 +++++++++++++++++++++++++-----------
3 files changed, 217 insertions(+), 80 deletions(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 143dc039bb03..86a5a7dbd3be 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -456,6 +456,19 @@ struct fscrypt_master_key_secret {
} __randomize_layout;
+/*
+ * fscrypt_key_pool - a management structure which handles a number of opaque
+ * fscrypt_pooled_prepared_keys (in keysetup.c).
+ */
+struct fscrypt_key_pool {
+ /* Mutex protecting all the fields here */
+ struct mutex mutex;
+ /* A list of active keys */
+ struct list_head active_keys;
+ /* Inactive keys available for use */
+ struct list_head free_keys;
+};
+
/*
* fscrypt_master_key - an in-use master key
*
@@ -545,6 +558,7 @@ struct fscrypt_master_key {
struct fscrypt_prepared_key mk_direct_keys[FSCRYPT_MODE_MAX + 1];
struct fscrypt_prepared_key mk_iv_ino_lblk_64_keys[FSCRYPT_MODE_MAX + 1];
struct fscrypt_prepared_key mk_iv_ino_lblk_32_keys[FSCRYPT_MODE_MAX + 1];
+ struct fscrypt_key_pool mk_key_pools[FSCRYPT_MODE_MAX + 1];
/* Hash key for inode numbers. Initialized only when needed. */
siphash_key_t mk_ino_hash_key;
@@ -624,6 +638,9 @@ struct fscrypt_mode {
extern struct fscrypt_mode fscrypt_modes[];
+void fscrypt_init_key_pool(struct fscrypt_key_pool *pool, size_t mode_num);
+void fscrypt_destroy_key_pool(struct fscrypt_key_pool *pool);
+
struct crypto_skcipher *fscrypt_get_contents_tfm(struct fscrypt_info *ci);
int fscrypt_allocate_key_member(struct fscrypt_prepared_key *prep_key,
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 7cbb1fd872ac..a24cbf901faf 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -76,6 +76,10 @@ void fscrypt_put_master_key(struct fscrypt_master_key *mk)
WARN_ON_ONCE(refcount_read(&mk->mk_active_refs) != 0);
key_put(mk->mk_users);
mk->mk_users = NULL;
+
+ for (int i = 0; i <= FSCRYPT_MODE_MAX; i++)
+ fscrypt_destroy_key_pool(&mk->mk_key_pools[i]);
+
call_rcu(&mk->mk_rcu_head, fscrypt_free_master_key);
}
@@ -429,6 +433,9 @@ static int add_new_master_key(struct super_block *sb,
INIT_LIST_HEAD(&mk->mk_decrypted_inodes);
spin_lock_init(&mk->mk_decrypted_inodes_lock);
+ for (size_t i = 0; i <= FSCRYPT_MODE_MAX; i++)
+ fscrypt_init_key_pool(&mk->mk_key_pools[i], i);
+
if (mk_spec->type == FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER) {
err = allocate_master_key_users_keyring(mk);
if (err)
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index c179a478020a..6e9f02620f18 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -90,7 +90,13 @@ struct fscrypt_mode fscrypt_modes[] = {
static DEFINE_MUTEX(fscrypt_mode_key_setup_mutex);
struct fscrypt_pooled_prepared_key {
+ /*
+ * Must be held when the prep key is in use or the key is being
+ * returned.
+ */
+ struct mutex mutex;
struct fscrypt_prepared_key prep_key;
+ struct list_head pool_link;
};
/* Forward declaration so that all the prepared key handling can stay together */
@@ -125,69 +131,6 @@ static int lock_master_key(struct fscrypt_master_key *mk)
return 0;
}
-static inline bool
-fscrypt_using_pooled_prepared_key(const struct fscrypt_info *ci)
-{
- if (ci->ci_policy.version != FSCRYPT_POLICY_V2)
- return false;
- if (ci->ci_policy.v2.flags & FSCRYPT_POLICY_FLAGS_KEY_MASK)
- return false;
- if (fscrypt_using_inline_encryption(ci))
- return false;
-
- if (!S_ISREG(ci->ci_inode->i_mode))
- return false;
- return false;
-}
-
-/*
- * Prepare the crypto transform object or blk-crypto key in @prep_key, given the
- * raw key, encryption mode (@ci->ci_mode), flag indicating which encryption
- * implementation (fs-layer or blk-crypto) will be used (@ci->ci_inlinecrypt),
- * and IV generation method (@ci->ci_policy.flags). The relevant member must
- * already be allocated and set in @prep_key.
- */
-int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
- const u8 *raw_key, const struct fscrypt_info *ci)
-{
- int err;
- bool inlinecrypt = fscrypt_using_inline_encryption(ci);
-
- if (inlinecrypt) {
- err = fscrypt_prepare_inline_crypt_key(prep_key, raw_key, ci);
- } else {
- err = crypto_skcipher_setkey(prep_key->tfm, raw_key,
- ci->ci_mode->keysize);
- }
-
- return err;
-}
-
-struct crypto_skcipher *fscrypt_get_contents_tfm(struct fscrypt_info *ci)
-{
- int err;
- struct fscrypt_master_key *mk = ci->ci_master_key;
- unsigned int allocflags;
-
- if (!fscrypt_using_pooled_prepared_key(ci))
- return ci->ci_enc_key->tfm;
-
- err = lock_master_key(mk);
- if (err) {
- up_read(&mk->mk_sem);
- return ERR_PTR(err);
- }
-
- allocflags = memalloc_nofs_save();
- err = fscrypt_setup_v2_file_key(ci, mk);
- up_read(&mk->mk_sem);
- memalloc_nofs_restore(allocflags);
- if (err)
- return ERR_PTR(err);
-
- return ci->ci_enc_key->tfm;
-}
-
/* Create a symmetric cipher object for the given encryption mode */
static struct crypto_skcipher *
fscrypt_allocate_skcipher(struct fscrypt_mode *mode,
@@ -230,6 +173,173 @@ fscrypt_allocate_skcipher(struct fscrypt_mode *mode,
return ERR_PTR(err);
}
+static inline bool
+fscrypt_using_pooled_prepared_key(const struct fscrypt_info *ci)
+{
+ if (ci->ci_policy.version != FSCRYPT_POLICY_V2)
+ return false;
+ if (ci->ci_policy.v2.flags & FSCRYPT_POLICY_FLAGS_KEY_MASK)
+ return false;
+ if (fscrypt_using_inline_encryption(ci))
+ return false;
+
+ if (!S_ISREG(ci->ci_inode->i_mode))
+ return false;
+ return false;
+}
+
+static void fscrypt_return_key_to_pool(struct fscrypt_key_pool *pool,
+ struct fscrypt_pooled_prepared_key *key)
+{
+ mutex_lock(&pool->mutex);
+ list_move(&key->pool_link, &pool->free_keys);
+ mutex_unlock(&pool->mutex);
+}
+
+static int fscrypt_allocate_new_pooled_key(struct fscrypt_key_pool *pool,
+ struct fscrypt_mode *mode)
+{
+ struct fscrypt_pooled_prepared_key *pooled_key;
+ struct crypto_skcipher *tfm;
+
+ pooled_key = kzalloc(sizeof(*pooled_key), GFP_KERNEL);
+ if (!pooled_key)
+ return -ENOMEM;
+
+ tfm = fscrypt_allocate_skcipher(mode, NULL);
+ if (IS_ERR(tfm)) {
+ kfree(pooled_key);
+ return PTR_ERR(tfm);
+ }
+
+ pooled_key->prep_key.tfm = tfm;
+ pooled_key->prep_key.type = FSCRYPT_KEY_POOLED;
+ mutex_init(&pooled_key->mutex);
+ INIT_LIST_HEAD(&pooled_key->pool_link);
+ fscrypt_return_key_to_pool(pool, pooled_key);
+ return 0;
+}
+
+/*
+ * Gets a key out of the free list, and locks it for use.
+ */
+static struct fscrypt_pooled_prepared_key *
+fscrypt_get_key_from_pool(struct fscrypt_key_pool *pool)
+{
+ struct fscrypt_pooled_prepared_key *key;
+
+ mutex_lock(&pool->mutex);
+ if (WARN_ON_ONCE(list_empty(&pool->free_keys))) {
+ mutex_unlock(&pool->mutex);
+ return ERR_PTR(-EBUSY);
+ }
+ key = list_first_entry(&pool->free_keys,
+ struct fscrypt_pooled_prepared_key, pool_link);
+
+ list_move(&key->pool_link, &pool->active_keys);
+ mutex_unlock(&pool->mutex);
+ return key;
+}
+
+/*
+ * Do initial setup for a particular key pool, allocated as part of an array
+ */
+void fscrypt_init_key_pool(struct fscrypt_key_pool *pool, size_t modenum)
+{
+ struct fscrypt_mode *mode = &fscrypt_modes[modenum];
+
+ mutex_init(&pool->mutex);
+ INIT_LIST_HEAD(&pool->active_keys);
+ INIT_LIST_HEAD(&pool->free_keys);
+
+ /*
+ * Always try to allocate one pooled key in
+ * case we never have another opportunity. But if it doesn't succeed,
+ * it's fine, we just need to never use the pool.
+ */
+ fscrypt_allocate_new_pooled_key(pool, mode);
+}
+
+/*
+ * Destroy a particular key pool, allocated as part
+ * of an array, freeing all prepared keys within.
+ */
+void fscrypt_destroy_key_pool(struct fscrypt_key_pool *pool)
+{
+ struct fscrypt_pooled_prepared_key *tmp;
+
+ mutex_lock(&pool->mutex);
+ WARN_ON_ONCE(!list_empty(&pool->active_keys));
+ while (!list_empty(&pool->active_keys)) {
+ tmp = list_first_entry(&pool->active_keys,
+ struct fscrypt_pooled_prepared_key,
+ pool_link);
+ fscrypt_destroy_prepared_key(NULL, &tmp->prep_key);
+ list_del_init(&tmp->pool_link);
+ kfree(tmp);
+ }
+ while (!list_empty(&pool->free_keys)) {
+ tmp = list_first_entry(&pool->free_keys,
+ struct fscrypt_pooled_prepared_key,
+ pool_link);
+ fscrypt_destroy_prepared_key(NULL, &tmp->prep_key);
+ list_del_init(&tmp->pool_link);
+ kfree(tmp);
+ }
+ mutex_unlock(&pool->mutex);
+}
+
+/*
+ * Prepare the crypto transform object or blk-crypto key in @prep_key, given the
+ * raw key, encryption mode (@ci->ci_mode), flag indicating which encryption
+ * implementation (fs-layer or blk-crypto) will be used (@ci->ci_inlinecrypt),
+ * and IV generation method (@ci->ci_policy.flags). The relevant member must
+ * already be allocated and set in @prep_key.
+ */
+int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
+ const u8 *raw_key, const struct fscrypt_info *ci)
+{
+ int err;
+ bool inlinecrypt = fscrypt_using_inline_encryption(ci);
+
+ if (inlinecrypt) {
+ err = fscrypt_prepare_inline_crypt_key(prep_key, raw_key, ci);
+ } else {
+ err = crypto_skcipher_setkey(prep_key->tfm, raw_key,
+ ci->ci_mode->keysize);
+ }
+
+ return err;
+}
+
+struct crypto_skcipher *fscrypt_get_contents_tfm(struct fscrypt_info *ci)
+{
+ int err;
+ struct fscrypt_master_key *mk = ci->ci_master_key;
+ unsigned int allocflags;
+
+ if (!fscrypt_using_pooled_prepared_key(ci))
+ return ci->ci_enc_key->tfm;
+
+ if (ci->ci_enc_key)
+ return ci->ci_enc_key->tfm;
+
+ err = lock_master_key(mk);
+ if (err) {
+ up_read(&mk->mk_sem);
+ return ERR_PTR(err);
+ }
+
+ allocflags = memalloc_nofs_save();
+ err = fscrypt_setup_v2_file_key(ci, mk);
+ up_read(&mk->mk_sem);
+ memalloc_nofs_restore(allocflags);
+ if (err)
+ return ERR_PTR(err);
+
+ return ci->ci_enc_key->tfm;
+}
+
/* Allocate the relevant encryption member for the prepared key */
int fscrypt_allocate_key_member(struct fscrypt_prepared_key *prep_key,
const struct fscrypt_info *ci)
@@ -262,23 +372,19 @@ int fscrypt_set_per_file_enc_key(struct fscrypt_info *ci, const u8 *raw_key)
if (fscrypt_using_pooled_prepared_key(ci)) {
struct fscrypt_pooled_prepared_key *pooled_key;
+ struct fscrypt_master_key *mk = ci->ci_master_key;
+ const u8 mode_num = ci->ci_mode - fscrypt_modes;
+ struct fscrypt_key_pool *pool = &mk->mk_key_pools[mode_num];
- if (ci->ci_enc_key)
- return fscrypt_prepare_key(ci->ci_enc_key, raw_key, ci);
-
- pooled_key = kzalloc(sizeof(*pooled_key), GFP_KERNEL);
- if (!pooled_key)
- return -ENOMEM;
-
- err = fscrypt_allocate_key_member(&pooled_key->prep_key, ci);
- if (err) {
- kfree(pooled_key);
+ err = fscrypt_allocate_new_pooled_key(pool, ci->ci_mode);
+ if (err)
return err;
- }
- pooled_key->prep_key.type = FSCRYPT_KEY_POOLED;
+ pooled_key = fscrypt_get_key_from_pool(pool);
+ if (IS_ERR(pooled_key))
+ return PTR_ERR(pooled_key);
+
ci->ci_enc_key = &pooled_key->prep_key;
- return 0;
} else {
ci->ci_enc_key = kzalloc(sizeof(*ci->ci_enc_key), GFP_KERNEL);
if (!ci->ci_enc_key)
@@ -527,6 +633,10 @@ static int fscrypt_setup_file_key(struct fscrypt_info *ci,
{
int err;
+ if (fscrypt_using_pooled_prepared_key(ci)) {
+ return 0;
+ }
+
if (!mk) {
if (ci->ci_policy.version != FSCRYPT_POLICY_V1)
return -ENOKEY;
@@ -674,6 +784,8 @@ static void put_crypt_info(struct fscrypt_info *ci)
if (!ci)
return;
+ mk = ci->ci_master_key;
+
if (ci->ci_enc_key) {
enum fscrypt_prepared_key_type type = ci->ci_enc_key->type;
@@ -686,18 +798,19 @@ static void put_crypt_info(struct fscrypt_info *ci)
}
if (type == FSCRYPT_KEY_POOLED) {
struct fscrypt_pooled_prepared_key *pooled_key;
+ const u8 mode_num = ci->ci_mode - fscrypt_modes;
pooled_key = container_of(ci->ci_enc_key,
struct fscrypt_pooled_prepared_key,
prep_key);
- fscrypt_destroy_prepared_key(ci->ci_inode->i_sb,
- ci->ci_enc_key);
- kfree_sensitive(pooled_key);
+ fscrypt_return_key_to_pool(&mk->mk_key_pools[mode_num],
+ pooled_key);
+ ci->ci_enc_key = NULL;
}
}
- mk = ci->ci_master_key;
+
if (mk) {
/*
* Remove this inode from the list of inodes that were unlocked
--
2.40.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v1 4/7] fscrypt: add pooled prepared key locking around use
2023-04-19 2:42 [PATCH v1 0/7] fscrypt: add pooled prepared keys facility Sweet Tea Dorminy
` (2 preceding siblings ...)
2023-04-19 2:42 ` [PATCH v1 3/7] fscrypt: add pooling of pooled prepared keys Sweet Tea Dorminy
@ 2023-04-19 2:42 ` Sweet Tea Dorminy
2023-04-19 2:42 ` [PATCH v1 5/7] fscrypt: reclaim pooled prepared keys under pressure Sweet Tea Dorminy
` (3 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Sweet Tea Dorminy @ 2023-04-19 2:42 UTC (permalink / raw)
To: Eric Biggers, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt,
kernel-team
Cc: Sweet Tea Dorminy
So far the prepared key pools have no provision for running out of
prepared keys. However, it is necessary to add some mechanism to request
some infos free their pooled prepared key in response to pool pressure,
and in order to do that, we must add a per-pooled-prepared-key facility
to mark the critical region where the info can't tolerate losing its
prepared key.
Thus, add locking to infos using pooled keys around the time when the
key is actually in use. This doesn't actually add stealing a prepared
key from an existing info, but prepares to do so.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/crypto.c | 24 +++--
fs/crypto/fscrypt_private.h | 1 +
fs/crypto/keysetup.c | 173 ++++++++++++++++++++++++++++--------
3 files changed, 153 insertions(+), 45 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 91ef520a9961..6b1b0953e7cc 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -113,16 +113,22 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
if (IS_ERR(tfm))
return PTR_ERR(tfm);
- if (WARN_ON_ONCE(len <= 0))
- return -EINVAL;
- if (WARN_ON_ONCE(len % FSCRYPT_CONTENTS_ALIGNMENT != 0))
- return -EINVAL;
+ if (WARN_ON_ONCE(len <= 0)) {
+ res = -EINVAL;
+ goto unlock;
+ }
+ if (WARN_ON_ONCE(len % FSCRYPT_CONTENTS_ALIGNMENT != 0)) {
+ res = -EINVAL;
+ goto unlock;
+ }
fscrypt_generate_iv(&iv, lblk_num, ci);
req = skcipher_request_alloc(tfm, gfp_flags);
- if (!req)
- return -ENOMEM;
+ if (!req) {
+ res = -ENOMEM;
+ goto unlock;
+ }
skcipher_request_set_callback(
req, CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
@@ -141,9 +147,11 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
if (res) {
fscrypt_err(inode, "%scryption failed for block %llu: %d",
(rw == FS_DECRYPT ? "De" : "En"), lblk_num, res);
- return res;
}
- return 0;
+
+unlock:
+ fscrypt_unlock_key_if_pooled(ci);
+ return res;
}
/**
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 86a5a7dbd3be..2737545ce4a6 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -642,6 +642,7 @@ void fscrypt_init_key_pool(struct fscrypt_key_pool *pool, size_t mode_num);
void fscrypt_destroy_key_pool(struct fscrypt_key_pool *pool);
struct crypto_skcipher *fscrypt_get_contents_tfm(struct fscrypt_info *ci);
+void fscrypt_unlock_key_if_pooled(struct fscrypt_info *ci);
int fscrypt_allocate_key_member(struct fscrypt_prepared_key *prep_key,
const struct fscrypt_info *ci);
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 6e9f02620f18..aed60280ad32 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -97,6 +97,8 @@ struct fscrypt_pooled_prepared_key {
struct mutex mutex;
struct fscrypt_prepared_key prep_key;
struct list_head pool_link;
+ /* NULL when it's in the pool. */
+ struct fscrypt_info *owner;
};
/* Forward declaration so that all the prepared key handling can stay together */
@@ -188,12 +190,96 @@ fscrypt_using_pooled_prepared_key(const struct fscrypt_info *ci)
return false;
}
-static void fscrypt_return_key_to_pool(struct fscrypt_key_pool *pool,
- struct fscrypt_pooled_prepared_key *key)
+static struct fscrypt_info *
+get_pooled_key_owner(struct fscrypt_pooled_prepared_key *key)
{
- mutex_lock(&pool->mutex);
+ /*
+ * Pairs with the smp_store_release() in fscrypt_get_key_from_pool()
+ * and __fscrypt_return_key_to_pool() below. The owner is
+ * potentially unstable unless the key's lock is taken.
+ */
+ return smp_load_acquire(&key->owner);
+}
+
+/*
+ * Lock the prepared key currently in ci->ci_enc_key, if it hasn't been stolen
+ * for the use of some other ci. Once this function succeeds, the prepared
+ * key cannot be stolen by another ci until fscrypt_unlock_key_if_pooled() is
+ * called.
+ *
+ * Returns 0 if the lock was successful, -ESTALE if the pooled key is now owned
+ * by some other ci (and ci->ci_enc_key is reset to NULL)
+ */
+static int fscrypt_lock_pooled_key(struct fscrypt_info *ci)
+{
+ struct fscrypt_pooled_prepared_key *pooled_key =
+ container_of(ci->ci_enc_key, struct fscrypt_pooled_prepared_key,
+ prep_key);
+
+ /* Peek to see if someone's definitely stolen the pooled key */
+ if (get_pooled_key_owner(pooled_key) != ci)
+ goto stolen;
+
+ /*
+ * Try to grab the lock. If we don't immediately succeed, some other
+ * ci has stolen the key.
+ */
+ if (!mutex_trylock(&pooled_key->mutex))
+ goto stolen;
+
+ /* We got the lock! Make sure we're still the owner. */
+ if (get_pooled_key_owner(pooled_key) != ci) {
+ mutex_unlock(&pooled_key->mutex);
+ goto stolen;
+ }
+
+ return 0;
+
+stolen:
+ ci->ci_enc_key = NULL;
+ return -ESTALE;
+}
+
+void fscrypt_unlock_key_if_pooled(struct fscrypt_info *ci)
+{
+ struct fscrypt_pooled_prepared_key *pooled_key;
+
+ if (!fscrypt_using_pooled_prepared_key(ci))
+ return;
+
+ pooled_key = container_of(ci->ci_enc_key,
+ struct fscrypt_pooled_prepared_key,
+ prep_key);
+
+ mutex_unlock(&pooled_key->mutex);
+}
+
+static void __fscrypt_return_key_to_pool(struct fscrypt_key_pool *pool,
+ struct fscrypt_pooled_prepared_key *key)
+{
+ /* Pairs with the acquire in get_pooled_key_owner() */
+ smp_store_release(&key->owner, NULL);
list_move(&key->pool_link, &pool->free_keys);
mutex_unlock(&pool->mutex);
+ mutex_unlock(&key->mutex);
+}
+
+static void fscrypt_return_key_to_pool(struct fscrypt_info *ci)
+{
+ struct fscrypt_pooled_prepared_key *pooled_key;
+ const u8 mode_num = ci->ci_mode - fscrypt_modes;
+ struct fscrypt_master_key *mk = ci->ci_master_key;
+
+ mutex_lock(&pool->mutex);
+ /* Try to lock the key. If we don't, it's already been stolen. */
+ if (fscrypt_lock_pooled_key(ci) != 0)
+ return;
+
+ pooled_key = container_of(ci->ci_enc_key,
+ struct fscrypt_pooled_prepared_key,
+ prep_key);
+
+ __fscrypt_return_key_to_pool(&mk->mk_key_pools[mode_num], pooled_key);
}
static int fscrypt_allocate_new_pooled_key(struct fscrypt_key_pool *pool,
@@ -216,29 +302,34 @@ static int fscrypt_allocate_new_pooled_key(struct fscrypt_key_pool *pool,
pooled_key->prep_key.type = FSCRYPT_KEY_POOLED;
mutex_init(&pooled_key->mutex);
INIT_LIST_HEAD(&pooled_key->pool_link);
- fscrypt_return_key_to_pool(pool, pooled_key);
+ mutex_lock(&pooled_key->mutex);
+ __fscrypt_return_key_to_pool(pool, pooled_key);
return 0;
}
/*
* Gets a key out of the free list, and locks it for use.
*/
-static struct fscrypt_pooled_prepared_key *
-fscrypt_get_key_from_pool(struct fscrypt_key_pool *pool)
+static int fscrypt_get_key_from_pool(struct fscrypt_key_pool *pool,
+ struct fscrypt_info *ci)
{
struct fscrypt_pooled_prepared_key *key;
mutex_lock(&pool->mutex);
if (WARN_ON_ONCE(list_empty(&pool->free_keys))) {
mutex_unlock(&pool->mutex);
- return ERR_PTR(-EBUSY);
+ return -EBUSY;
}
key = list_first_entry(&pool->free_keys,
struct fscrypt_pooled_prepared_key, pool_link);
+ /* Pairs with the acquire in get_pooled_key_owner() */
+ smp_store_release(&key->owner, ci);
+ ci->ci_enc_key = &key->prep_key;
list_move(&key->pool_link, &pool->active_keys);
+
mutex_unlock(&pool->mutex);
- return key;
+ return 0;
}
/*
@@ -317,12 +408,22 @@ struct crypto_skcipher *fscrypt_get_contents_tfm(struct fscrypt_info *ci)
int err;
struct fscrypt_master_key *mk = ci->ci_master_key;
unsigned int allocflags;
+ const u8 mode_num = ci->ci_mode - fscrypt_modes;
+ struct fscrypt_key_pool *pool = &mk->mk_key_pools[mode_num];
if (!fscrypt_using_pooled_prepared_key(ci))
return ci->ci_enc_key->tfm;
- if (ci->ci_enc_key)
- return ci->ci_enc_key->tfm;
+ /* do we need to set it up? */
+ if (!ci->ci_enc_key)
+ goto lock_for_new_key;
+
+ if (fscrypt_lock_pooled_key(ci) != 0)
+ goto lock_for_new_key;
+
+ return ci->ci_enc_key->tfm;
+
+lock_for_new_key:
err = lock_master_key(mk);
if (err) {
@@ -330,6 +431,17 @@ struct crypto_skcipher *fscrypt_get_contents_tfm(struct fscrypt_info *ci)
return ERR_PTR(err);
}
+setup_new_key:
+ err = fscrypt_get_key_from_pool(pool, ci);
+ if (err) {
+ up_read(&mk->mk_sem);
+ return ERR_PTR(err);
+ }
+
+ /* Make sure nobody stole our fresh key already */
+ if (fscrypt_lock_pooled_key(ci) != 0)
+ goto setup_new_key;
+
allocflags = memalloc_nofs_save();
err = fscrypt_setup_v2_file_key(ci, mk);
up_read(&mk->mk_sem);
@@ -371,20 +483,6 @@ int fscrypt_set_per_file_enc_key(struct fscrypt_info *ci, const u8 *raw_key)
int err;
if (fscrypt_using_pooled_prepared_key(ci)) {
- struct fscrypt_pooled_prepared_key *pooled_key;
- struct fscrypt_master_key *mk = ci->ci_master_key;
- const u8 mode_num = ci->ci_mode - fscrypt_modes;
- struct fscrypt_key_pool *pool = &mk->mk_key_pools[mode_num];
-
- err = fscrypt_allocate_new_pooled_key(pool, ci->ci_mode);
- if (err)
- return err;
-
- pooled_key = fscrypt_get_key_from_pool(pool);
- if (IS_ERR(pooled_key))
- return PTR_ERR(pooled_key);
-
- ci->ci_enc_key = &pooled_key->prep_key;
} else {
ci->ci_enc_key = kzalloc(sizeof(*ci->ci_enc_key), GFP_KERNEL);
if (!ci->ci_enc_key)
@@ -797,16 +895,7 @@ static void put_crypt_info(struct fscrypt_info *ci)
kfree_sensitive(ci->ci_enc_key);
}
if (type == FSCRYPT_KEY_POOLED) {
- struct fscrypt_pooled_prepared_key *pooled_key;
- const u8 mode_num = ci->ci_mode - fscrypt_modes;
-
- pooled_key = container_of(ci->ci_enc_key,
- struct fscrypt_pooled_prepared_key,
- prep_key);
-
- fscrypt_return_key_to_pool(&mk->mk_key_pools[mode_num],
- pooled_key);
- ci->ci_enc_key = NULL;
+ fscrypt_return_key_to_pool(ci);
}
}
@@ -866,9 +955,19 @@ fscrypt_setup_encryption_info(struct inode *inode,
if (res)
goto out;
- res = fscrypt_setup_file_key(crypt_info, mk);
- if (res)
- goto out;
+ if (!fscrypt_using_pooled_prepared_key(crypt_info)) {
+ res = fscrypt_setup_file_key(crypt_info, mk);
+ if (res)
+ goto out;
+ } else {
+ const u8 mode_num = mode - fscrypt_modes;
+ struct fscrypt_key_pool *pool = &mk->mk_key_pools[mode_num];
+
+ res = fscrypt_allocate_new_pooled_key(pool, mode);
+ if (res)
+ return res;
+ }
+
/*
* Derive a secret dirhash key for directories that need it. It
--
2.40.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v1 5/7] fscrypt: reclaim pooled prepared keys under pressure
2023-04-19 2:42 [PATCH v1 0/7] fscrypt: add pooled prepared keys facility Sweet Tea Dorminy
` (3 preceding siblings ...)
2023-04-19 2:42 ` [PATCH v1 4/7] fscrypt: add pooled prepared key locking around use Sweet Tea Dorminy
@ 2023-04-19 2:42 ` Sweet Tea Dorminy
2023-04-19 2:42 ` [PATCH v1 6/7] fscrypt: add facility to shrink a pool of keys Sweet Tea Dorminy
` (2 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Sweet Tea Dorminy @ 2023-04-19 2:42 UTC (permalink / raw)
To: Eric Biggers, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt,
kernel-team
Cc: Sweet Tea Dorminy
While it is to be hoped that the pools of prepared keys are sized
appropriately, there is no way to guarantee that they will never have
pressure short of expanding the pool for every user -- and that defeats
the goal of not allocating a prepared key for every user. So, a
mechanism to 'steal' a prepared key from its current owner must be
added, to support another info using the prepared key.
This adds that stealing mechanism. Currently, on pressure, the head of
the active prepared key list is stolen; this selection mechanism is
likely suboptimal, and is improved in a later change.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/keysetup.c | 47 +++++++++++++++++++++++++++++++-------------
1 file changed, 33 insertions(+), 14 deletions(-)
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index aed60280ad32..c12a26c81611 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -260,8 +260,8 @@ static void __fscrypt_return_key_to_pool(struct fscrypt_key_pool *pool,
/* Pairs with the acquire in get_pooled_key_owner() */
smp_store_release(&key->owner, NULL);
list_move(&key->pool_link, &pool->free_keys);
- mutex_unlock(&pool->mutex);
mutex_unlock(&key->mutex);
+ mutex_unlock(&pool->mutex);
}
static void fscrypt_return_key_to_pool(struct fscrypt_info *ci)
@@ -269,17 +269,20 @@ static void fscrypt_return_key_to_pool(struct fscrypt_info *ci)
struct fscrypt_pooled_prepared_key *pooled_key;
const u8 mode_num = ci->ci_mode - fscrypt_modes;
struct fscrypt_master_key *mk = ci->ci_master_key;
+ struct fscrypt_key_pool *pool = &mk->mk_key_pools[mode_num];
mutex_lock(&pool->mutex);
/* Try to lock the key. If we don't, it's already been stolen. */
- if (fscrypt_lock_pooled_key(ci) != 0)
+ if (fscrypt_lock_pooled_key(ci) != 0) {
+ mutex_unlock(&pool->mutex);
return;
+ }
pooled_key = container_of(ci->ci_enc_key,
struct fscrypt_pooled_prepared_key,
prep_key);
- __fscrypt_return_key_to_pool(&mk->mk_key_pools[mode_num], pooled_key);
+ __fscrypt_return_key_to_pool(pool, pooled_key);
}
static int fscrypt_allocate_new_pooled_key(struct fscrypt_key_pool *pool,
@@ -302,13 +305,33 @@ static int fscrypt_allocate_new_pooled_key(struct fscrypt_key_pool *pool,
pooled_key->prep_key.type = FSCRYPT_KEY_POOLED;
mutex_init(&pooled_key->mutex);
INIT_LIST_HEAD(&pooled_key->pool_link);
+ mutex_lock(&pool->mutex);
mutex_lock(&pooled_key->mutex);
__fscrypt_return_key_to_pool(pool, pooled_key);
return 0;
}
+static struct fscrypt_pooled_prepared_key *
+fscrypt_select_victim_key(struct fscrypt_key_pool *pool)
+{
+ return list_first_entry(&pool->active_keys,
+ struct fscrypt_pooled_prepared_key, pool_link);
+}
+
+static void fscrypt_steal_an_active_key(struct fscrypt_key_pool *pool)
+{
+ struct fscrypt_pooled_prepared_key *key;
+
+ key = fscrypt_select_victim_key(pool);
+ mutex_lock(&key->mutex);
+ /* Pairs with the acquire in get_pooled_key_owner() */
+ smp_store_release(&key->owner, NULL);
+ list_move(&key->pool_link, &pool->free_keys);
+ mutex_unlock(&key->mutex);
+}
+
/*
- * Gets a key out of the free list, and locks it for use.
+ * Gets a key out of the free list, possibly stealing it along the way.
*/
static int fscrypt_get_key_from_pool(struct fscrypt_key_pool *pool,
struct fscrypt_info *ci)
@@ -316,10 +339,14 @@ static int fscrypt_get_key_from_pool(struct fscrypt_key_pool *pool,
struct fscrypt_pooled_prepared_key *key;
mutex_lock(&pool->mutex);
- if (WARN_ON_ONCE(list_empty(&pool->free_keys))) {
+ if (list_empty(&pool->free_keys) && list_empty(&pool->active_keys)) {
mutex_unlock(&pool->mutex);
- return -EBUSY;
+ return -EINVAL;
}
+
+ if (list_empty(&pool->free_keys))
+ fscrypt_steal_an_active_key(pool);
+
key = list_first_entry(&pool->free_keys,
struct fscrypt_pooled_prepared_key, pool_link);
@@ -959,16 +986,8 @@ fscrypt_setup_encryption_info(struct inode *inode,
res = fscrypt_setup_file_key(crypt_info, mk);
if (res)
goto out;
- } else {
- const u8 mode_num = mode - fscrypt_modes;
- struct fscrypt_key_pool *pool = &mk->mk_key_pools[mode_num];
-
- res = fscrypt_allocate_new_pooled_key(pool, mode);
- if (res)
- return res;
}
-
/*
* Derive a secret dirhash key for directories that need it. It
* should be impossible to set flags such that a v1 policy sets
--
2.40.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v1 6/7] fscrypt: add facility to shrink a pool of keys
2023-04-19 2:42 [PATCH v1 0/7] fscrypt: add pooled prepared keys facility Sweet Tea Dorminy
` (4 preceding siblings ...)
2023-04-19 2:42 ` [PATCH v1 5/7] fscrypt: reclaim pooled prepared keys under pressure Sweet Tea Dorminy
@ 2023-04-19 2:42 ` Sweet Tea Dorminy
2023-04-19 2:42 ` [PATCH v1 7/7] fscrypt: add lru mechanism to choose pooled key Sweet Tea Dorminy
2023-05-02 3:47 ` [PATCH v1 0/7] fscrypt: add pooled prepared keys facility Eric Biggers
7 siblings, 0 replies; 14+ messages in thread
From: Sweet Tea Dorminy @ 2023-04-19 2:42 UTC (permalink / raw)
To: Eric Biggers, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt,
kernel-team
Cc: Sweet Tea Dorminy
One expected mechanism to size the pools for extent-based encryption is
to allocate one new pooled prepared key when each inode using such is
opened, and to free it when that inode is closed. This provides roughly
equally many active prepared keys as the non-extent-based scheme, one
prepared key per 'leaf' inode for contents encryption.
This change introduces the ability to shrink a prepared key pool by one
key, including deferring freeing until the next prepared key is freed
back to the pool. This avoids issues with stealing a prepared key from
an info and then freeing it while the old info still references it.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/fscrypt_private.h | 4 ++++
fs/crypto/keysetup.c | 43 ++++++++++++++++++++++++++++++-------
2 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 2737545ce4a6..cc6e4a2a942c 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -467,6 +467,10 @@ struct fscrypt_key_pool {
struct list_head active_keys;
/* Inactive keys available for use */
struct list_head free_keys;
+ /* Count of keys currently managed */
+ size_t count;
+ /* Count of keys desired. Oft equal to count, but can be less. */
+ size_t desired;
};
/*
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index c12a26c81611..57c343b78abc 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -254,6 +254,22 @@ void fscrypt_unlock_key_if_pooled(struct fscrypt_info *ci)
mutex_unlock(&pooled_key->mutex);
}
+/*
+ * Free one key from the free list.
+ */
+static void __fscrypt_free_one_free_key(struct fscrypt_key_pool *pool)
+{
+ struct fscrypt_pooled_prepared_key *tmp;
+
+ tmp = list_first_entry(&pool->free_keys,
+ struct fscrypt_pooled_prepared_key,
+ pool_link);
+ fscrypt_destroy_prepared_key(NULL, &tmp->prep_key);
+ list_del_init(&tmp->pool_link);
+ kfree(tmp);
+ pool->count--;
+}
+
static void __fscrypt_return_key_to_pool(struct fscrypt_key_pool *pool,
struct fscrypt_pooled_prepared_key *key)
{
@@ -261,6 +277,8 @@ static void __fscrypt_return_key_to_pool(struct fscrypt_key_pool *pool,
smp_store_release(&key->owner, NULL);
list_move(&key->pool_link, &pool->free_keys);
mutex_unlock(&key->mutex);
+ if (pool->count > pool->desired)
+ __fscrypt_free_one_free_key(pool);
mutex_unlock(&pool->mutex);
}
@@ -306,6 +324,8 @@ static int fscrypt_allocate_new_pooled_key(struct fscrypt_key_pool *pool,
mutex_init(&pooled_key->mutex);
INIT_LIST_HEAD(&pooled_key->pool_link);
mutex_lock(&pool->mutex);
+ pool->count++;
+ pool->desired++;
mutex_lock(&pooled_key->mutex);
__fscrypt_return_key_to_pool(pool, pooled_key);
return 0;
@@ -359,6 +379,18 @@ static int fscrypt_get_key_from_pool(struct fscrypt_key_pool *pool,
return 0;
}
+/*
+ * Shrink the pool by one key.
+ */
+static void fscrypt_shrink_key_pool(struct fscrypt_key_pool *pool)
+{
+ mutex_lock(&pool->mutex);
+ pool->desired--;
+ if (!list_empty(&pool->free_keys))
+ __fscrypt_free_one_free_key(pool);
+ mutex_unlock(&pool->mutex);
+}
+
/*
* Do initial setup for a particular key pool, allocated as part of an array
*/
@@ -396,14 +428,9 @@ void fscrypt_destroy_key_pool(struct fscrypt_key_pool *pool)
list_del_init(&tmp->pool_link);
kfree(tmp);
}
- while (!list_empty(&pool->free_keys)) {
- tmp = list_first_entry(&pool->free_keys,
- struct fscrypt_pooled_prepared_key,
- pool_link);
- fscrypt_destroy_prepared_key(NULL, &tmp->prep_key);
- list_del_init(&tmp->pool_link);
- kfree(tmp);
- }
+ while (!list_empty(&pool->free_keys))
+ __fscrypt_free_one_free_key(pool);
+
mutex_unlock(&pool->mutex);
}
--
2.40.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v1 7/7] fscrypt: add lru mechanism to choose pooled key
2023-04-19 2:42 [PATCH v1 0/7] fscrypt: add pooled prepared keys facility Sweet Tea Dorminy
` (5 preceding siblings ...)
2023-04-19 2:42 ` [PATCH v1 6/7] fscrypt: add facility to shrink a pool of keys Sweet Tea Dorminy
@ 2023-04-19 2:42 ` Sweet Tea Dorminy
2023-05-02 3:47 ` [PATCH v1 0/7] fscrypt: add pooled prepared keys facility Eric Biggers
7 siblings, 0 replies; 14+ messages in thread
From: Sweet Tea Dorminy @ 2023-04-19 2:42 UTC (permalink / raw)
To: Eric Biggers, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt,
kernel-team
Cc: Sweet Tea Dorminy
This improves the previous mechanism to select a key to steal: it keeps
track of the LRU active key, and steals that one. While the previous
mechanism isn't an invalid strategy, it is probably inferior and results
in excessive churn in very active keys.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/fscrypt_private.h | 6 +++++-
fs/crypto/keysetup.c | 39 ++++++++++++++++++++++++++++++++++---
2 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index cc6e4a2a942c..c23b222ca30b 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -461,7 +461,7 @@ struct fscrypt_master_key_secret {
* fscrypt_pooled_prepared_keys (in keysetup.c).
*/
struct fscrypt_key_pool {
- /* Mutex protecting all the fields here */
+ /* Mutex protecting almost everything */
struct mutex mutex;
/* A list of active keys */
struct list_head active_keys;
@@ -471,6 +471,10 @@ struct fscrypt_key_pool {
size_t count;
/* Count of keys desired. Oft equal to count, but can be less. */
size_t desired;
+ /* Mutex protecting the lru list*/
+ struct mutex lru_mutex;
+ /* Same list of active keys, just ordered by usage. Head is oldest. */
+ struct list_head active_lru;
};
/*
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 57c343b78abc..d498b89cd097 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -92,11 +92,13 @@ static DEFINE_MUTEX(fscrypt_mode_key_setup_mutex);
struct fscrypt_pooled_prepared_key {
/*
* Must be held when the prep key is in use or the key is being
- * returned.
+ * returned. Must not be held for interactions with pool_link and
+ * pool_lru_link, which are protected by the pool locks.
*/
struct mutex mutex;
struct fscrypt_prepared_key prep_key;
struct list_head pool_link;
+ struct list_head pool_lru_link;
/* NULL when it's in the pool. */
struct fscrypt_info *owner;
};
@@ -201,6 +203,18 @@ get_pooled_key_owner(struct fscrypt_pooled_prepared_key *key)
return smp_load_acquire(&key->owner);
}
+/*
+ * Must be called with the key lock held and the pool lock not held.
+ */
+static void update_lru(struct fscrypt_key_pool *pool,
+ struct fscrypt_pooled_prepared_key *key)
+{
+ mutex_lock(&pool->lru_mutex);
+ /* Bump this key up to the most recent end of the pool's lru list. */
+ list_move_tail(&pool->active_lru, &key->pool_lru_link);
+ mutex_unlock(&pool->lru_mutex);
+}
+
/*
* Lock the prepared key currently in ci->ci_enc_key, if it hasn't been stolen
* for the use of some other ci. Once this function succeeds, the prepared
@@ -215,6 +229,9 @@ static int fscrypt_lock_pooled_key(struct fscrypt_info *ci)
struct fscrypt_pooled_prepared_key *pooled_key =
container_of(ci->ci_enc_key, struct fscrypt_pooled_prepared_key,
prep_key);
+ struct fscrypt_master_key *mk = ci->ci_master_key;
+ const u8 mode_num = ci->ci_mode - fscrypt_modes;
+ struct fscrypt_key_pool *pool = &mk->mk_key_pools[mode_num];
/* Peek to see if someone's definitely stolen the pooled key */
if (get_pooled_key_owner(pooled_key) != ci)
@@ -233,6 +250,8 @@ static int fscrypt_lock_pooled_key(struct fscrypt_info *ci)
goto stolen;
}
+ update_lru(pool, pooled_key);
+
return 0;
stolen:
@@ -270,12 +289,16 @@ static void __fscrypt_free_one_free_key(struct fscrypt_key_pool *pool)
pool->count--;
}
+/* Must be called with the pool mutex held. Releases it. */
static void __fscrypt_return_key_to_pool(struct fscrypt_key_pool *pool,
struct fscrypt_pooled_prepared_key *key)
{
/* Pairs with the acquire in get_pooled_key_owner() */
smp_store_release(&key->owner, NULL);
list_move(&key->pool_link, &pool->free_keys);
+ mutex_lock(&pool->lru_mutex);
+ list_del_init(&key->pool_lru_link);
+ mutex_unlock(&pool->lru_mutex);
mutex_unlock(&key->mutex);
if (pool->count > pool->desired)
__fscrypt_free_one_free_key(pool);
@@ -323,6 +346,7 @@ static int fscrypt_allocate_new_pooled_key(struct fscrypt_key_pool *pool,
pooled_key->prep_key.type = FSCRYPT_KEY_POOLED;
mutex_init(&pooled_key->mutex);
INIT_LIST_HEAD(&pooled_key->pool_link);
+ INIT_LIST_HEAD(&pooled_key->pool_lru_link);
mutex_lock(&pool->mutex);
pool->count++;
pool->desired++;
@@ -334,8 +358,15 @@ static int fscrypt_allocate_new_pooled_key(struct fscrypt_key_pool *pool,
static struct fscrypt_pooled_prepared_key *
fscrypt_select_victim_key(struct fscrypt_key_pool *pool)
{
- return list_first_entry(&pool->active_keys,
- struct fscrypt_pooled_prepared_key, pool_link);
+ struct fscrypt_pooled_prepared_key *key;
+
+ mutex_lock(&pool->lru_mutex);
+ key = list_first_entry(&pool->active_lru,
+ struct fscrypt_pooled_prepared_key,
+ pool_lru_link);
+ list_del_init(&key->pool_lru_link);
+ mutex_unlock(&pool->lru_mutex);
+ return key;
}
static void fscrypt_steal_an_active_key(struct fscrypt_key_pool *pool)
@@ -399,7 +430,9 @@ void fscrypt_init_key_pool(struct fscrypt_key_pool *pool, size_t modenum)
struct fscrypt_mode *mode = &fscrypt_modes[modenum];
mutex_init(&pool->mutex);
+ mutex_init(&pool->lru_mutex);
INIT_LIST_HEAD(&pool->active_keys);
+ INIT_LIST_HEAD(&pool->active_lru);
INIT_LIST_HEAD(&pool->free_keys);
/*
--
2.40.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v1 0/7] fscrypt: add pooled prepared keys facility
2023-04-19 2:42 [PATCH v1 0/7] fscrypt: add pooled prepared keys facility Sweet Tea Dorminy
` (6 preceding siblings ...)
2023-04-19 2:42 ` [PATCH v1 7/7] fscrypt: add lru mechanism to choose pooled key Sweet Tea Dorminy
@ 2023-05-02 3:47 ` Eric Biggers
2023-05-05 12:15 ` Sweet Tea Dorminy
7 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2023-05-02 3:47 UTC (permalink / raw)
To: Sweet Tea Dorminy
Cc: Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, kernel-team
Hi Sweet Tea,
On Tue, Apr 18, 2023 at 10:42:09PM -0400, Sweet Tea Dorminy wrote:
> This is part two of two of preliminaries to extent-based encryption,
> adding a facility to pool pre-allocated prepared keys and use them at IO
> time.
>
> While arguably one structure within the feature, and not actually used
> in this changeset at that, it's a disjoint piece that has various taste
> questions so I've put it in its own changeset here for good or ill.
>
> The change has been tested by switching a false to true so as to use it
> for leaf inodes which are doing contents encryption, and then running
> the standard tests. Such a thing changes the timing of when the prepared
> key is set up, obviously, so that IO which begins after a master key
> secret is removed no longer succeeds; this fails generic/{580,581,593}
> which don't have that expectation. However, this code has no impact on
> tests if disabled.
>
> Known suboptimalities:
> -right now at the end nothing calls fscrypt_shrink_key_pool() and it
> throws an unused function warning.
> -right now it's hooked up to be used by leaf inodes not using inline
> encryption only. I don't know if there's any interest in pooling inode
> keys -- it could reduce memory usage on memory-constrained platforms --
> and if so using it for filename encryption also might make sense. On the
> other hand, if there's no interest, the code allowing use of it in the normal
> inode-info path is unnecessary.
> -right now it doesn't pool inline encryption objects either.
> -the initialization of a key pool for each mode spams the log with
> "Missing crypto API support" messages. Maybe the init of key pools
> should be the first time an info using pooled prepared keys is observed?
>
> Some questions:
>
> -does the pooling mechanism need to be extended to mode keys, which can
> easily be pre-allocated if needed?
> -does it need to be extended to v1 policies?
> -does it need to be behind a config option, perhaps with extent
> encryption?
> -should it be in its own, new file, since it adds a decent chunk of code
> to keysetup.c most of which is arguably key-agnostic?
>
> This changeset should apply atop the previous one, entitled
> 'fscrypt: rearrangements preliminary to extent encryption'
> lore.kernel.org/r/cover.1681837335.git.sweettea-kernel@dorminy.me
Sorry for the slow response; I've been catching up after being on vacation.
I've also been having trouble understanding out what this patchset is doing and
what the next part is likely to be after it.
I'm worried that this may be going down a path of something much more complex
than needed. It's hard to follow the new logic with the new data structures and
locks, keys being "stolen" from one file to another, etc. I'm also confused by
how the pooled keys are being assigned to a field in fscrypt_info, given that
fscrypt_info is for an inode, not an extent. So it's a bit unclear (at least to
me) how this proposal will lead to extent-based encryption.
As I mentioned earlier
(https://lore.kernel.org/r/Y7NQ1CvPyJiGRe00@sol.localdomain),
blk-crypto-fallback actually already solved the problem of caching
crypto_skcipher objects for I/O. And, it's possible for a filesystem to *only*
support blk-crypto, not filesystem-layer contents encryption. You'd just need
to put btrfs encryption behind a new kconfig option that is automatically
selected by CONFIG_FS_ENCRYPTION_INLINE_CRYPT && CONFIG_BLK_ENCRYPTION_FALLBACK.
(BTW, I'm thinking of simplifying the kconfig options by removing
CONFIG_FS_ENCRYPTION_INLINE_CRYPT. Then, the blk-crypto code in fs/crypto/ will
be built if CONFIG_FS_ENCRYPTION && CONFIG_BLK_INLINE_ENCRYPTION.)
Indeed, filesystem-layer contents encryption is a bit redundant these days now
that blk-crypto-fallback exists. I'm even tempted to make ext4 and f2fs support
blk-crypto only someday. That was sort of the original plan, actually...
So, I'm wondering if you've considered going the blk-crypto-fallback route?
I expect that it would be a lot easier than what you seem to be trying.
The main thing to consider would be exactly how to handle the key derivation.
In the long run, it might make sense to add key derivation support to
blk-crypto, as there is actually inline encryption hardware that supports
per-file key derivation in hardware already and it would be nice to support that
someday. But for now, maybe just have the filesystem derive the key for each
extent, upon first access to that extent, and cache the resulting blk_crypto_key
in the appropriate btrfs data structure for the extent ('struct extent_state'
maybe)? Can you think of any reason why that wouldn't work?
There is also the issue of authenticated encryption (which I understand you're
going to add after unauthenticated encryption is working). That will take some
work to add support to blk-crypto. However, the same would be true for
filesystem-layer contents encryption too. So I'm not sure that would be an
argument for filesystem-layer contents encryption over blk-crypto...
- Eric
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 0/7] fscrypt: add pooled prepared keys facility
2023-05-02 3:47 ` [PATCH v1 0/7] fscrypt: add pooled prepared keys facility Eric Biggers
@ 2023-05-05 12:15 ` Sweet Tea Dorminy
2023-05-05 22:40 ` Eric Biggers
0 siblings, 1 reply; 14+ messages in thread
From: Sweet Tea Dorminy @ 2023-05-05 12:15 UTC (permalink / raw)
To: Eric Biggers
Cc: Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, kernel-team
> As I mentioned earlier
> (https://lore.kernel.org/r/Y7NQ1CvPyJiGRe00@sol.localdomain),
> blk-crypto-fallback actually already solved the problem of caching
> crypto_skcipher objects for I/O. And, it's possible for a filesystem to *only*
> support blk-crypto, not filesystem-layer contents encryption. You'd just need
> to put btrfs encryption behind a new kconfig option that is automatically
> selected by CONFIG_FS_ENCRYPTION_INLINE_CRYPT && CONFIG_BLK_ENCRYPTION_FALLBACK.
>
> (BTW, I'm thinking of simplifying the kconfig options by removing
> CONFIG_FS_ENCRYPTION_INLINE_CRYPT. Then, the blk-crypto code in fs/crypto/ will
> be built if CONFIG_FS_ENCRYPTION && CONFIG_BLK_INLINE_ENCRYPTION.)
>
> Indeed, filesystem-layer contents encryption is a bit redundant these days now
> that blk-crypto-fallback exists. I'm even tempted to make ext4 and f2fs support
> blk-crypto only someday. That was sort of the original plan, actually...
>
> So, I'm wondering if you've considered going the blk-crypto-fallback route?
I did, and gave it a shot, but ran into problems because as far as I can
tell it requires having a bio to crypt. For verity data and inline
extents, there's no obvious bio, and even if we tried to construct a bio
pointing at the relevant data, it's not necessarily sector- sized or
aligned. I couldn't figure out a good way to make it work, but maybe
it's better to special-case those or there's something I'm not seeing.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 0/7] fscrypt: add pooled prepared keys facility
2023-05-05 12:15 ` Sweet Tea Dorminy
@ 2023-05-05 22:40 ` Eric Biggers
2023-05-06 0:35 ` Sweet Tea Dorminy
0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2023-05-05 22:40 UTC (permalink / raw)
To: Sweet Tea Dorminy
Cc: Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, kernel-team
On Fri, May 05, 2023 at 08:15:44AM -0400, Sweet Tea Dorminy wrote:
>
> > As I mentioned earlier
> > (https://lore.kernel.org/r/Y7NQ1CvPyJiGRe00@sol.localdomain),
> > blk-crypto-fallback actually already solved the problem of caching
> > crypto_skcipher objects for I/O. And, it's possible for a filesystem to *only*
> > support blk-crypto, not filesystem-layer contents encryption. You'd just need
> > to put btrfs encryption behind a new kconfig option that is automatically
> > selected by CONFIG_FS_ENCRYPTION_INLINE_CRYPT && CONFIG_BLK_ENCRYPTION_FALLBACK.
> >
> > (BTW, I'm thinking of simplifying the kconfig options by removing
> > CONFIG_FS_ENCRYPTION_INLINE_CRYPT. Then, the blk-crypto code in fs/crypto/ will
> > be built if CONFIG_FS_ENCRYPTION && CONFIG_BLK_INLINE_ENCRYPTION.)
> >
> > Indeed, filesystem-layer contents encryption is a bit redundant these days now
> > that blk-crypto-fallback exists. I'm even tempted to make ext4 and f2fs support
> > blk-crypto only someday. That was sort of the original plan, actually...
> >
> > So, I'm wondering if you've considered going the blk-crypto-fallback route?
>
> I did, and gave it a shot, but ran into problems because as far as I can
> tell it requires having a bio to crypt. For verity data and inline extents,
> there's no obvious bio, and even if we tried to construct a bio pointing at
> the relevant data, it's not necessarily sector- sized or aligned. I couldn't
> figure out a good way to make it work, but maybe it's better to special-case
> those or there's something I'm not seeing.
ext4 and f2fs just don't use inline data on encrypted files. I.e. when an encrypted file is
created, it always uses non-inline data. Is that an option for btrfs?
For the verity metadata, how are you thinking of encrypting it, exactly? Verity metadata is
immutable once written, so surely it avoids many of the issues you are dealing with for extents? It
should just need one key, and that key could be set up at file open time. So I don't think it will
need the key pool at all?
- Eric
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 0/7] fscrypt: add pooled prepared keys facility
2023-05-05 22:40 ` Eric Biggers
@ 2023-05-06 0:35 ` Sweet Tea Dorminy
2023-05-15 6:40 ` Eric Biggers
0 siblings, 1 reply; 14+ messages in thread
From: Sweet Tea Dorminy @ 2023-05-06 0:35 UTC (permalink / raw)
To: Eric Biggers
Cc: Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, kernel-team
On 5/5/23 18:40, Eric Biggers wrote:
> On Fri, May 05, 2023 at 08:15:44AM -0400, Sweet Tea Dorminy wrote:
>>
>>> As I mentioned earlier
>>> (https://lore.kernel.org/r/Y7NQ1CvPyJiGRe00@sol.localdomain),
>>> blk-crypto-fallback actually already solved the problem of caching
>>> crypto_skcipher objects for I/O. And, it's possible for a filesystem to *only*
>>> support blk-crypto, not filesystem-layer contents encryption. You'd just need
>>> to put btrfs encryption behind a new kconfig option that is automatically
>>> selected by CONFIG_FS_ENCRYPTION_INLINE_CRYPT && CONFIG_BLK_ENCRYPTION_FALLBACK.
>>>
>>> (BTW, I'm thinking of simplifying the kconfig options by removing
>>> CONFIG_FS_ENCRYPTION_INLINE_CRYPT. Then, the blk-crypto code in fs/crypto/ will
>>> be built if CONFIG_FS_ENCRYPTION && CONFIG_BLK_INLINE_ENCRYPTION.)
>>>
>>> Indeed, filesystem-layer contents encryption is a bit redundant these days now
>>> that blk-crypto-fallback exists. I'm even tempted to make ext4 and f2fs support
>>> blk-crypto only someday. That was sort of the original plan, actually...
>>>
>>> So, I'm wondering if you've considered going the blk-crypto-fallback route?
>>
>> I did, and gave it a shot, but ran into problems because as far as I can
>> tell it requires having a bio to crypt. For verity data and inline extents,
>> there's no obvious bio, and even if we tried to construct a bio pointing at
>> the relevant data, it's not necessarily sector- sized or aligned. I couldn't
>> figure out a good way to make it work, but maybe it's better to special-case
>> those or there's something I'm not seeing.
>
> ext4 and f2fs just don't use inline data on encrypted files. I.e. when an encrypted file is
> created, it always uses non-inline data. Is that an option for btrfs?
It's not impossible (though it's been viewed as a fair deficiency in
last year's changesets), but it's not the only user of data needing
encryption stored inline instead of separately:
>
> For the verity metadata, how are you thinking of encrypting it, exactly? Verity metadata is
> immutable once written, so surely it avoids many of the issues you are dealing with for extents? It
> should just need one key, and that key could be set up at file open time. So I don't think it will
> need the key pool at all?
Yes, it should be able to use whatever the interface is for extent
encryption, whether that uses pooled keys or something else. However,
btrfs stores verity data in 2k chunks in the tree, similar to inline
data, so it has the same difficulties.
(I realized after sending that we also want to encrypt xattrs, which are
similarly stored as items in the tree instead of blocks on disk.)
We could have separate pools for inline and non-inline prepared keys (or
not pool inline keys at all?)
Thanks!
Sweet Tea
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 0/7] fscrypt: add pooled prepared keys facility
2023-05-06 0:35 ` Sweet Tea Dorminy
@ 2023-05-15 6:40 ` Eric Biggers
2023-05-16 18:34 ` Sweet Tea Dorminy
0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2023-05-15 6:40 UTC (permalink / raw)
To: Sweet Tea Dorminy
Cc: Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, kernel-team
Hi Sweet Tea,
On Fri, May 05, 2023 at 08:35:53PM -0400, Sweet Tea Dorminy wrote:
>
>
> On 5/5/23 18:40, Eric Biggers wrote:
> > On Fri, May 05, 2023 at 08:15:44AM -0400, Sweet Tea Dorminy wrote:
> > >
> > > > As I mentioned earlier
> > > > (https://lore.kernel.org/r/Y7NQ1CvPyJiGRe00@sol.localdomain),
> > > > blk-crypto-fallback actually already solved the problem of caching
> > > > crypto_skcipher objects for I/O. And, it's possible for a filesystem to *only*
> > > > support blk-crypto, not filesystem-layer contents encryption. You'd just need
> > > > to put btrfs encryption behind a new kconfig option that is automatically
> > > > selected by CONFIG_FS_ENCRYPTION_INLINE_CRYPT && CONFIG_BLK_ENCRYPTION_FALLBACK.
> > > >
> > > > (BTW, I'm thinking of simplifying the kconfig options by removing
> > > > CONFIG_FS_ENCRYPTION_INLINE_CRYPT. Then, the blk-crypto code in fs/crypto/ will
> > > > be built if CONFIG_FS_ENCRYPTION && CONFIG_BLK_INLINE_ENCRYPTION.)
> > > >
> > > > Indeed, filesystem-layer contents encryption is a bit redundant these days now
> > > > that blk-crypto-fallback exists. I'm even tempted to make ext4 and f2fs support
> > > > blk-crypto only someday. That was sort of the original plan, actually...
> > > >
> > > > So, I'm wondering if you've considered going the blk-crypto-fallback route?
> > >
> > > I did, and gave it a shot, but ran into problems because as far as I can
> > > tell it requires having a bio to crypt. For verity data and inline extents,
> > > there's no obvious bio, and even if we tried to construct a bio pointing at
> > > the relevant data, it's not necessarily sector- sized or aligned. I couldn't
> > > figure out a good way to make it work, but maybe it's better to special-case
> > > those or there's something I'm not seeing.
> >
> > ext4 and f2fs just don't use inline data on encrypted files. I.e. when an encrypted file is
> > created, it always uses non-inline data. Is that an option for btrfs?
>
> It's not impossible (though it's been viewed as a fair deficiency in last
> year's changesets), but it's not the only user of data needing encryption
> stored inline instead of separately:
> >
> > For the verity metadata, how are you thinking of encrypting it, exactly? Verity metadata is
> > immutable once written, so surely it avoids many of the issues you are dealing with for extents? It
> > should just need one key, and that key could be set up at file open time. So I don't think it will
> > need the key pool at all?
>
> Yes, it should be able to use whatever the interface is for extent
> encryption, whether that uses pooled keys or something else. However, btrfs
> stores verity data in 2k chunks in the tree, similar to inline data, so it
> has the same difficulties.
>
> (I realized after sending that we also want to encrypt xattrs, which are
> similarly stored as items in the tree instead of blocks on disk.)
>
> We could have separate pools for inline and non-inline prepared keys (or not
> pool inline keys at all?)
>
To clarify my suggestion, blk-crypto could be used for file contents
en/decryption at the same time that filesystem-layer crypto is used for verity
metadata en/decryption. blk-crypto and filesystem-layer crypto don't need to be
mutually exclusive, even on the same file.
Also, I'm glad that you're interested in xattr encryption, but unfortunately
it's a tough problem, and all the other filesystems that implement fscrypt have
left it out. You have enough other things to worry about, so I think leaving
xattr encryption out for now is the right call. Similarly, the other
filesystems that implement fscrypt have left out encryption of inline data,
instead opting to disable inline data on encrypted files.
Anyway, the main reason I'm sending this email is actually that I wanted to
mention another possible solution to the per-extent key problem that I just
became aware of. In v6.4-rc1, the crypto API added a new function
crypto_clone_tfm() which allocates a new tfm object, given an existing one.
Unlike crypto_alloc_tfm(), crypto_clone_tfm() doesn't take any locks. See:
https://lore.kernel.org/linux-crypto/ZDefxOq6Ax0JeTRH@gondor.apana.org.au/T/#u
For now, only shash and ahash tfms can be cloned. However, it looks like
support for cloning skcipher tfms could be added.
With "cloning" skcipher tfms, there could just be a crypto_skcipher per extent,
allocated on the I/O path. That would solve the problem we've been trying to
solve, without having to bring in the complexity of "pooled prepared keys".
I think we should go either with that or with the blk-crypto-fallback solution.
- Eric
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 0/7] fscrypt: add pooled prepared keys facility
2023-05-15 6:40 ` Eric Biggers
@ 2023-05-16 18:34 ` Sweet Tea Dorminy
0 siblings, 0 replies; 14+ messages in thread
From: Sweet Tea Dorminy @ 2023-05-16 18:34 UTC (permalink / raw)
To: Eric Biggers
Cc: Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, kernel-team
> To clarify my suggestion, blk-crypto could be used for file contents
> en/decryption at the same time that filesystem-layer crypto is used for verity
> metadata en/decryption. blk-crypto and filesystem-layer crypto don't need to be
> mutually exclusive, even on the same file.
That's a great point. I'm dropping these two patchsets and updating the
original one at the start of the year to use blk-crypto, as per our
discussions both here and at LSF.
>
> Also, I'm glad that you're interested in xattr encryption, but unfortunately
> it's a tough problem, and all the other filesystems that implement fscrypt have
> left it out. You have enough other things to worry about, so I think leaving
> xattr encryption out for now is the right call. Similarly, the other
> filesystems that implement fscrypt have left out encryption of inline data,
> instead opting to disable inline data on encrypted files.
A good point. I'll defer xattrs and inline data (and verity) for the
first round, and add in doing those with inode infos after getting
extent infos working well.
>
> Anyway, the main reason I'm sending this email is actually that I wanted to
> mention another possible solution to the per-extent key problem that I just
> became aware of. In v6.4-rc1, the crypto API added a new function
> crypto_clone_tfm() which allocates a new tfm object, given an existing one.
> Unlike crypto_alloc_tfm(), crypto_clone_tfm() doesn't take any locks. See:
> https://lore.kernel.org/linux-crypto/ZDefxOq6Ax0JeTRH@gondor.apana.org.au/T/#u
>
> For now, only shash and ahash tfms can be cloned. However, it looks like
> support for cloning skcipher tfms could be added.
>
> With "cloning" skcipher tfms, there could just be a crypto_skcipher per extent,
> allocated on the I/O path. That would solve the problem we've been trying to
> solve, without having to bring in the complexity of "pooled prepared keys".
Huh. A cool new thing for sure. I suppose one would have an initial tfm
per supported crypto alg, and clone it for each extent as needed. That's
definitely better than pooling prepared keys. I'll explore this after
everything else, and work on blk-crypto oriented for now.
Thanks!
Sweet Tea
^ permalink raw reply [flat|nested] 14+ messages in thread