* [RFC PATCH 01/17] fscrypt: factor accessing inode->i_crypt_info
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-02 21:00 ` Eric Biggers
2023-01-01 5:06 ` [RFC PATCH 02/17] fscrypt: separate getting info for a specific block Sweet Tea Dorminy
` (16 subsequent siblings)
17 siblings, 1 reply; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
Currently, inode->i_crypt_info is accessed directly in many places;
the initial setting occurs in one place, via cmpxchg_release, and
the initial access is abstracted into fscrypt_get_info() which uses
smp_load_acquire(), but there are many direct accesses. While many of
them follow calls to fscrypt_get_info() on the same thread, verifying
this is not always trivial.
For instance, fscrypt_crypt_block() does not obviously follow a call to
fscrypt_get_info() on the same cpu; if some other mechanism does not
ensure a memory barrier, it is conceivable that a filesystem could call
fscrypt_crypt_block() on a cpu which had an old (NULL) i_crypt_info
cached. Even if the cpu does READ_ONCE(i_crypt_info), I believe it's
theoretically possible for it to see the old NULL value, since this
could be happening on a cpu which did not do the smp_load_acquire(). (I
may be misunderstanding, but memory-barriers.txt says that only the cpus
involved in an acquire/release chain are guaranteed to see the correct
order of operations, which seems to imply that a cpu which does not do
an acquire may be able to see a memory value from before the release.)
For safety, then, and so each site doesn't need to be individually
evaluated, this change factors all accesses of i_crypt_info to go
through fscrypt_get_info(), ensuring every access uses acquire and is
thus paired against an appropriate release.
(The same treatment is not necessary for setting i_crypt_info; the
only unprotected setting is during inode cleanup, which is inevitably
followed by freeing the inode; there are no uses past the unprotected
setting possible.)
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/crypto.c | 2 +-
fs/crypto/fname.c | 9 +++++----
fs/crypto/hooks.c | 2 +-
fs/crypto/inline_crypt.c | 10 +++++-----
fs/crypto/keysetup.c | 2 +-
fs/crypto/policy.c | 6 +++---
6 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index e78be66bbf01..2efd1da9df8d 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -107,7 +107,7 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
struct skcipher_request *req = NULL;
DECLARE_CRYPTO_WAIT(wait);
struct scatterlist dst, src;
- struct fscrypt_info *ci = inode->i_crypt_info;
+ struct fscrypt_info *ci = fscrypt_get_info(inode);
struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
int res = 0;
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 12bd61d20f69..6efb53cba523 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -100,7 +100,7 @@ int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
{
struct skcipher_request *req = NULL;
DECLARE_CRYPTO_WAIT(wait);
- const struct fscrypt_info *ci = inode->i_crypt_info;
+ const struct fscrypt_info *ci = fscrypt_get_info(inode);
struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
union fscrypt_iv iv;
struct scatterlist sg;
@@ -157,7 +157,7 @@ static int fname_decrypt(const struct inode *inode,
struct skcipher_request *req = NULL;
DECLARE_CRYPTO_WAIT(wait);
struct scatterlist src_sg, dst_sg;
- const struct fscrypt_info *ci = inode->i_crypt_info;
+ const struct fscrypt_info *ci = fscrypt_get_info(inode);
struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
union fscrypt_iv iv;
int res;
@@ -299,7 +299,8 @@ bool __fscrypt_fname_encrypted_size(const union fscrypt_policy *policy,
bool fscrypt_fname_encrypted_size(const struct inode *inode, u32 orig_len,
u32 max_len, u32 *encrypted_len_ret)
{
- return __fscrypt_fname_encrypted_size(&inode->i_crypt_info->ci_policy,
+ struct fscrypt_info *ci = fscrypt_get_info(inode);
+ return __fscrypt_fname_encrypted_size(&ci->ci_policy,
orig_len, max_len,
encrypted_len_ret);
}
@@ -568,7 +569,7 @@ EXPORT_SYMBOL_GPL(fscrypt_match_name);
*/
u64 fscrypt_fname_siphash(const struct inode *dir, const struct qstr *name)
{
- const struct fscrypt_info *ci = dir->i_crypt_info;
+ const struct fscrypt_info *ci = fscrypt_get_info(dir);
WARN_ON(!ci->ci_dirhash_key_initialized);
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index 7b8c5a1104b5..b605660fb3f1 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -152,7 +152,7 @@ int fscrypt_prepare_setflags(struct inode *inode,
err = fscrypt_require_key(inode);
if (err)
return err;
- ci = inode->i_crypt_info;
+ ci = fscrypt_get_info(inode);
if (ci->ci_policy.version != FSCRYPT_POLICY_V2)
return -EINVAL;
mk = ci->ci_master_key;
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index cea8b14007e6..4b1373715018 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -232,7 +232,7 @@ void fscrypt_destroy_inline_crypt_key(struct super_block *sb,
bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode)
{
- return inode->i_crypt_info->ci_inlinecrypt;
+ return fscrypt_get_info(inode)->ci_inlinecrypt;
}
EXPORT_SYMBOL_GPL(__fscrypt_inode_uses_inline_crypto);
@@ -274,7 +274,7 @@ void fscrypt_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
if (!fscrypt_inode_uses_inline_crypto(inode))
return;
- ci = inode->i_crypt_info;
+ ci = fscrypt_get_info(inode);
fscrypt_generate_dun(ci, first_lblk, dun);
bio_crypt_set_ctx(bio, ci->ci_enc_key.blk_key, dun, gfp_mask);
@@ -364,10 +364,10 @@ 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)
+ if (bc->bc_key != fscrypt_get_info(inode)->ci_enc_key.blk_key)
return false;
- fscrypt_generate_dun(inode->i_crypt_info, next_lblk, next_dun);
+ fscrypt_generate_dun(fscrypt_get_info(inode), next_lblk, next_dun);
return bio_crypt_dun_is_contiguous(bc, bio->bi_iter.bi_size, next_dun);
}
EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio);
@@ -469,7 +469,7 @@ u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks)
if (nr_blocks <= 1)
return nr_blocks;
- ci = inode->i_crypt_info;
+ ci = fscrypt_get_info(inode);
if (!(fscrypt_policy_flags(&ci->ci_policy) &
FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32))
return nr_blocks;
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index f7407071a952..ad56192305b3 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -706,7 +706,7 @@ EXPORT_SYMBOL_GPL(fscrypt_prepare_new_inode);
*/
void fscrypt_put_encryption_info(struct inode *inode)
{
- put_crypt_info(inode->i_crypt_info);
+ put_crypt_info(fscrypt_get_info(inode));
inode->i_crypt_info = NULL;
}
EXPORT_SYMBOL(fscrypt_put_encryption_info);
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 46757c3052ef..ccab27afd3cc 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -687,7 +687,7 @@ const union fscrypt_policy *fscrypt_policy_to_inherit(struct inode *dir)
err = fscrypt_require_key(dir);
if (err)
return ERR_PTR(err);
- return &dir->i_crypt_info->ci_policy;
+ return &fscrypt_get_info(dir)->ci_policy;
}
return fscrypt_get_dummy_policy(dir->i_sb);
@@ -706,7 +706,7 @@ const union fscrypt_policy *fscrypt_policy_to_inherit(struct inode *dir)
*/
int fscrypt_context_for_new_inode(void *ctx, struct inode *inode)
{
- struct fscrypt_info *ci = inode->i_crypt_info;
+ struct fscrypt_info *ci = fscrypt_get_info(inode);
BUILD_BUG_ON(sizeof(union fscrypt_context) !=
FSCRYPT_SET_CONTEXT_MAX_SIZE);
@@ -731,7 +731,7 @@ EXPORT_SYMBOL_GPL(fscrypt_context_for_new_inode);
*/
int fscrypt_set_context(struct inode *inode, void *fs_data)
{
- struct fscrypt_info *ci = inode->i_crypt_info;
+ struct fscrypt_info *ci = fscrypt_get_info(inode);
union fscrypt_context ctx;
int ctxsize;
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [RFC PATCH 01/17] fscrypt: factor accessing inode->i_crypt_info
2023-01-01 5:06 ` [RFC PATCH 01/17] fscrypt: factor accessing inode->i_crypt_info Sweet Tea Dorminy
@ 2023-01-02 21:00 ` Eric Biggers
0 siblings, 0 replies; 28+ messages in thread
From: Eric Biggers @ 2023-01-02 21:00 UTC (permalink / raw)
To: Sweet Tea Dorminy; +Cc: linux-fscrypt, paulcrowley, linux-btrfs, kernel-team
On Sun, Jan 01, 2023 at 12:06:05AM -0500, Sweet Tea Dorminy wrote:
> Currently, inode->i_crypt_info is accessed directly in many places;
> the initial setting occurs in one place, via cmpxchg_release, and
> the initial access is abstracted into fscrypt_get_info() which uses
> smp_load_acquire(), but there are many direct accesses. While many of
> them follow calls to fscrypt_get_info() on the same thread, verifying
> this is not always trivial.
>
> For instance, fscrypt_crypt_block() does not obviously follow a call to
> fscrypt_get_info() on the same cpu; if some other mechanism does not
> ensure a memory barrier, it is conceivable that a filesystem could call
> fscrypt_crypt_block() on a cpu which had an old (NULL) i_crypt_info
> cached. Even if the cpu does READ_ONCE(i_crypt_info), I believe it's
> theoretically possible for it to see the old NULL value, since this
> could be happening on a cpu which did not do the smp_load_acquire(). (I
> may be misunderstanding, but memory-barriers.txt says that only the cpus
> involved in an acquire/release chain are guaranteed to see the correct
> order of operations, which seems to imply that a cpu which does not do
> an acquire may be able to see a memory value from before the release.)
>
> For safety, then, and so each site doesn't need to be individually
> evaluated, this change factors all accesses of i_crypt_info to go
> through fscrypt_get_info(), ensuring every access uses acquire and is
> thus paired against an appropriate release.
>
> (The same treatment is not necessary for setting i_crypt_info; the
> only unprotected setting is during inode cleanup, which is inevitably
> followed by freeing the inode; there are no uses past the unprotected
> setting possible.)
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
This patch is not necessary. The rules for accessing ->i_crypt_info are
actually pretty simple: when it's unknown whether ->i_crypt_info has been set,
then it's necessary to use fscrypt_get_info() and check whether the resulting
pointer is NULL or not (or use fscrypt_has_encryption_key() which does both).
That's because another thread could set it concurrently.
In contrast, when it *is* known that ->i_crypt_info has been set, then that can
only be because fscrypt_has_encryption_key() was already executed on the same
thread, or because an operation that ensured the key is set up already happened.
For example, when doing I/O to a file, it's guaranteed that the file has been
opened. In either case, direct access is fine.
- Eric
^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH 02/17] fscrypt: separate getting info for a specific block
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 01/17] fscrypt: factor accessing inode->i_crypt_info Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 03/17] fscrypt: adjust effective lblks based on extents Sweet Tea Dorminy
` (15 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
For filesystems using extent-based encryption, the content of each extent will be
encrypted with a different fscrypt_info for each extent. Meanwhile,
directories and symlinks will continue to use the fscrypt_info for the
inode. Therefore, merely calling fscrypt_get_info() will be
insufficient; the caller must specifically request the inode info or the
info for a specific block.
Add that distinction, adding both fscrypt_get_inode_info() and
fscrypt_get_lblk_info(), and updating all callsites to call the
appropriate one.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/crypto.c | 3 ++-
fs/crypto/fname.c | 8 +++----
fs/crypto/fscrypt_private.h | 46 +++++++++++++++++++++++++++++++++++++
fs/crypto/hooks.c | 2 +-
fs/crypto/inline_crypt.c | 12 ++++++----
fs/crypto/keysetup.c | 4 ++--
fs/crypto/policy.c | 8 +++----
7 files changed, 66 insertions(+), 17 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 2efd1da9df8d..41c60c60b74c 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -107,7 +107,8 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
struct skcipher_request *req = NULL;
DECLARE_CRYPTO_WAIT(wait);
struct scatterlist dst, src;
- struct fscrypt_info *ci = fscrypt_get_info(inode);
+ struct fscrypt_info *ci = fscrypt_get_lblk_info(inode, lblk_num, NULL,
+ NULL);
struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
int res = 0;
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 6efb53cba523..e1474f24d014 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -100,7 +100,7 @@ int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
{
struct skcipher_request *req = NULL;
DECLARE_CRYPTO_WAIT(wait);
- const struct fscrypt_info *ci = fscrypt_get_info(inode);
+ const struct fscrypt_info *ci = fscrypt_get_inode_info(inode);
struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
union fscrypt_iv iv;
struct scatterlist sg;
@@ -157,7 +157,7 @@ static int fname_decrypt(const struct inode *inode,
struct skcipher_request *req = NULL;
DECLARE_CRYPTO_WAIT(wait);
struct scatterlist src_sg, dst_sg;
- const struct fscrypt_info *ci = fscrypt_get_info(inode);
+ const struct fscrypt_info *ci = fscrypt_get_inode_info(inode);
struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
union fscrypt_iv iv;
int res;
@@ -299,7 +299,7 @@ bool __fscrypt_fname_encrypted_size(const union fscrypt_policy *policy,
bool fscrypt_fname_encrypted_size(const struct inode *inode, u32 orig_len,
u32 max_len, u32 *encrypted_len_ret)
{
- struct fscrypt_info *ci = fscrypt_get_info(inode);
+ struct fscrypt_info *ci = fscrypt_get_inode_info(inode);
return __fscrypt_fname_encrypted_size(&ci->ci_policy,
orig_len, max_len,
encrypted_len_ret);
@@ -569,7 +569,7 @@ EXPORT_SYMBOL_GPL(fscrypt_match_name);
*/
u64 fscrypt_fname_siphash(const struct inode *dir, const struct qstr *name)
{
- const struct fscrypt_info *ci = fscrypt_get_info(dir);
+ const struct fscrypt_info *ci = fscrypt_get_inode_info(dir);
WARN_ON(!ci->ci_dirhash_key_initialized);
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index d5f68a0c5d15..2df28c6fe558 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -262,6 +262,52 @@ typedef enum {
FS_ENCRYPT,
} fscrypt_direction_t;
+/**
+ * fscrypt_get_inode_info() - get the fscrypt_info for a particular inode
+ *
+ * @inode: the inode in question
+ *
+ * For inode-based encryption, this will return the same info as
+ * fscrypt_get_lblk_info(). For extent-based encryption, for extentless
+ * files this will return the inode's info, otherwise it will return the info
+ * that new extents should inherit.
+ *
+ * Return: the appropriate fscrypt_info if there is one, else NULL.
+ */
+static inline struct fscrypt_info *
+fscrypt_get_inode_info(const struct inode *inode)
+{
+ return fscrypt_get_info(inode);
+}
+
+/**
+ * fscrypt_get_lblk_info() - get the fscrypt_info to crypt a particular block
+ *
+ * @inode: the inode to which the block belongs
+ * @lblk: the offset of the block within the file which the inode
+ * references
+ * @offset: a pointer to return the offset of the block from the first block
+ * that the info covers. For inode-based encryption, this will
+ * always be @lblk; for extent-based encryption, this will be in
+ * the range [0, lblk]. Can be NULL
+ * @extent_len: a pointer to return the minimum number of lblks starting at
+ * this offset which also belong to the same fscrypt_info. Can be
+ * NULL
+ *
+ * Return: the appropriate fscrypt_info if there is one, else NULL.
+ */
+static inline struct fscrypt_info *
+fscrypt_get_lblk_info(const struct inode *inode, u64 lblk, u64 *offset,
+ u64 *extent_len)
+{
+ if (offset)
+ *offset = lblk;
+ if (extent_len)
+ *extent_len = U64_MAX;
+
+ return fscrypt_get_info(inode);
+}
+
/* crypto.c */
extern struct kmem_cache *fscrypt_info_cachep;
int fscrypt_initialize(unsigned int cop_flags);
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index b605660fb3f1..929749c77440 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -152,7 +152,7 @@ int fscrypt_prepare_setflags(struct inode *inode,
err = fscrypt_require_key(inode);
if (err)
return err;
- ci = fscrypt_get_info(inode);
+ ci = fscrypt_get_inode_info(inode);
if (ci->ci_policy.version != FSCRYPT_POLICY_V2)
return -EINVAL;
mk = ci->ci_master_key;
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 4b1373715018..56d69b231875 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -232,7 +232,7 @@ void fscrypt_destroy_inline_crypt_key(struct super_block *sb,
bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode)
{
- return fscrypt_get_info(inode)->ci_inlinecrypt;
+ return fscrypt_get_inode_info(inode)->ci_inlinecrypt;
}
EXPORT_SYMBOL_GPL(__fscrypt_inode_uses_inline_crypto);
@@ -274,7 +274,7 @@ void fscrypt_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
if (!fscrypt_inode_uses_inline_crypto(inode))
return;
- ci = fscrypt_get_info(inode);
+ ci = fscrypt_get_lblk_info(inode, first_lblk, NULL, NULL);
fscrypt_generate_dun(ci, first_lblk, dun);
bio_crypt_set_ctx(bio, ci->ci_enc_key.blk_key, dun, gfp_mask);
@@ -353,21 +353,23 @@ bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
{
const struct bio_crypt_ctx *bc = bio->bi_crypt_context;
u64 next_dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
+ struct fscrypt_info *ci;
if (!!bc != fscrypt_inode_uses_inline_crypto(inode))
return false;
if (!bc)
return true;
+ ci = fscrypt_get_lblk_info(inode, next_lblk, NULL, NULL);
/*
* Comparing the key pointers is good enough, as all I/O for each key
* 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 != fscrypt_get_info(inode)->ci_enc_key.blk_key)
+ if (bc->bc_key != ci->ci_enc_key.blk_key)
return false;
- fscrypt_generate_dun(fscrypt_get_info(inode), next_lblk, next_dun);
+ fscrypt_generate_dun(ci, next_lblk, next_dun);
return bio_crypt_dun_is_contiguous(bc, bio->bi_iter.bi_size, next_dun);
}
EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio);
@@ -469,7 +471,7 @@ u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks)
if (nr_blocks <= 1)
return nr_blocks;
- ci = fscrypt_get_info(inode);
+ ci = fscrypt_get_lblk_info(inode, lblk, NULL, NULL);
if (!(fscrypt_policy_flags(&ci->ci_policy) &
FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32))
return nr_blocks;
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index ad56192305b3..87f28d666602 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -706,7 +706,7 @@ EXPORT_SYMBOL_GPL(fscrypt_prepare_new_inode);
*/
void fscrypt_put_encryption_info(struct inode *inode)
{
- put_crypt_info(fscrypt_get_info(inode));
+ put_crypt_info(fscrypt_get_inode_info(inode));
inode->i_crypt_info = NULL;
}
EXPORT_SYMBOL(fscrypt_put_encryption_info);
@@ -739,7 +739,7 @@ EXPORT_SYMBOL(fscrypt_free_inode);
*/
int fscrypt_drop_inode(struct inode *inode)
{
- const struct fscrypt_info *ci = fscrypt_get_info(inode);
+ const struct fscrypt_info *ci = fscrypt_get_inode_info(inode);
/*
* If ci is NULL, then the inode doesn't have an encryption key set up
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index ccab27afd3cc..e7de4872d375 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -398,7 +398,7 @@ static int fscrypt_get_policy(struct inode *inode, union fscrypt_policy *policy)
union fscrypt_context ctx;
int ret;
- ci = fscrypt_get_info(inode);
+ ci = fscrypt_get_inode_info(inode);
if (ci) {
/* key available, use the cached policy */
*policy = ci->ci_policy;
@@ -687,7 +687,7 @@ const union fscrypt_policy *fscrypt_policy_to_inherit(struct inode *dir)
err = fscrypt_require_key(dir);
if (err)
return ERR_PTR(err);
- return &fscrypt_get_info(dir)->ci_policy;
+ return &fscrypt_get_inode_info(dir)->ci_policy;
}
return fscrypt_get_dummy_policy(dir->i_sb);
@@ -706,7 +706,7 @@ const union fscrypt_policy *fscrypt_policy_to_inherit(struct inode *dir)
*/
int fscrypt_context_for_new_inode(void *ctx, struct inode *inode)
{
- struct fscrypt_info *ci = fscrypt_get_info(inode);
+ struct fscrypt_info *ci = fscrypt_get_inode_info(inode);
BUILD_BUG_ON(sizeof(union fscrypt_context) !=
FSCRYPT_SET_CONTEXT_MAX_SIZE);
@@ -731,7 +731,7 @@ EXPORT_SYMBOL_GPL(fscrypt_context_for_new_inode);
*/
int fscrypt_set_context(struct inode *inode, void *fs_data)
{
- struct fscrypt_info *ci = fscrypt_get_info(inode);
+ struct fscrypt_info *ci = fscrypt_get_inode_info(inode);
union fscrypt_context ctx;
int ctxsize;
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 03/17] fscrypt: adjust effective lblks based on extents
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 01/17] fscrypt: factor accessing inode->i_crypt_info Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 02/17] fscrypt: separate getting info for a specific block Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 04/17] fscrypt: factor out fscrypt_set_inode_info() Sweet Tea Dorminy
` (14 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
If a filesystem uses extent-based encryption, then the offset within a
file is not a constant which can be used for calculating an IV.
For instance, the same extent could be blocks 0-8 in one file, and
blocks 100-108 in another file. Instead, the block offset within the
extent must be used instead.
Update all uses of logical block offset within the file to use logical
block offset within the extent, if applicable.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/crypto.c | 7 ++++---
fs/crypto/inline_crypt.c | 19 +++++++++++++------
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 41c60c60b74c..93b83dbe82ee 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -107,8 +107,9 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
struct skcipher_request *req = NULL;
DECLARE_CRYPTO_WAIT(wait);
struct scatterlist dst, src;
- struct fscrypt_info *ci = fscrypt_get_lblk_info(inode, lblk_num, NULL,
- NULL);
+ u64 ci_offset = 0;
+ struct fscrypt_info *ci = fscrypt_get_lblk_info(inode, lblk_num,
+ &ci_offset, NULL);
struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
int res = 0;
@@ -117,7 +118,7 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
if (WARN_ON_ONCE(len % FSCRYPT_CONTENTS_ALIGNMENT != 0))
return -EINVAL;
- fscrypt_generate_iv(&iv, lblk_num, ci);
+ fscrypt_generate_iv(&iv, ci_offset, ci);
req = skcipher_request_alloc(tfm, gfp_flags);
if (!req)
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 56d69b231875..0ae220c8afa3 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -271,12 +271,13 @@ void fscrypt_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
{
const struct fscrypt_info *ci;
u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
+ u64 ci_offset = 0;
if (!fscrypt_inode_uses_inline_crypto(inode))
return;
- ci = fscrypt_get_lblk_info(inode, first_lblk, NULL, NULL);
+ ci = fscrypt_get_lblk_info(inode, first_lblk, &ci_offset, NULL);
- fscrypt_generate_dun(ci, first_lblk, dun);
+ fscrypt_generate_dun(ci, ci_offset, dun);
bio_crypt_set_ctx(bio, ci->ci_enc_key.blk_key, dun, gfp_mask);
}
EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx);
@@ -354,13 +355,14 @@ bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
const struct bio_crypt_ctx *bc = bio->bi_crypt_context;
u64 next_dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
struct fscrypt_info *ci;
+ u64 ci_offset = 0;
if (!!bc != fscrypt_inode_uses_inline_crypto(inode))
return false;
if (!bc)
return true;
- ci = fscrypt_get_lblk_info(inode, next_lblk, NULL, NULL);
+ ci = fscrypt_get_lblk_info(inode, next_lblk, &ci_offset, NULL);
/*
* Comparing the key pointers is good enough, as all I/O for each key
* uses the same pointer. I.e., there's currently no need to support
@@ -369,7 +371,7 @@ bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
if (bc->bc_key != ci->ci_enc_key.blk_key)
return false;
- fscrypt_generate_dun(ci, next_lblk, next_dun);
+ fscrypt_generate_dun(ci, ci_offset, next_dun);
return bio_crypt_dun_is_contiguous(bc, bio->bi_iter.bi_size, next_dun);
}
EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio);
@@ -464,6 +466,8 @@ u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks)
{
const struct fscrypt_info *ci;
u32 dun;
+ u64 ci_offset;
+ u64 extent_len;
if (!fscrypt_inode_uses_inline_crypto(inode))
return nr_blocks;
@@ -471,14 +475,17 @@ u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks)
if (nr_blocks <= 1)
return nr_blocks;
- ci = fscrypt_get_lblk_info(inode, lblk, NULL, NULL);
+ ci = fscrypt_get_lblk_info(inode, lblk, &ci_offset, &extent_len);
+
+ /* Spanning an extent boundary will change the DUN */
+ nr_blocks = min_t(u64, nr_blocks, extent_len);
if (!(fscrypt_policy_flags(&ci->ci_policy) &
FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32))
return nr_blocks;
/* With IV_INO_LBLK_32, the DUN can wrap around from U32_MAX to 0. */
- dun = ci->ci_hashed_ino + lblk;
+ dun = ci->ci_hashed_ino + ci_offset;
return min_t(u64, nr_blocks, (u64)U32_MAX + 1 - dun);
}
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 04/17] fscrypt: factor out fscrypt_set_inode_info()
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (2 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 03/17] fscrypt: adjust effective lblks based on extents Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 05/17] fscrypt: use parent dir's info for extent-based encryption Sweet Tea Dorminy
` (13 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
As this function needs to be called in several different places to
short-circuit creating a new fscrypt_info for extent-based encryption,
it makes sense to pull it into its own function for easy calling.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/keysetup.c | 60 +++++++++++++++++++++++++++-----------------
1 file changed, 37 insertions(+), 23 deletions(-)
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 87f28d666602..4d7ff8244c55 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -515,6 +515,38 @@ static void put_crypt_info(struct fscrypt_info *ci)
kmem_cache_free(fscrypt_info_cachep, ci);
}
+static bool fscrypt_set_inode_info(struct inode *inode,
+ struct fscrypt_info *ci,
+ struct fscrypt_master_key *mk)
+{
+ if (ci == NULL) {
+ inode->i_crypt_info = NULL;
+ return true;
+ }
+
+ /*
+ * For existing inodes, multiple tasks may race to set ->i_crypt_info.
+ * So use cmpxchg_release(). This pairs with the smp_load_acquire() in
+ * fscrypt_get_info(). I.e., here we publish ->i_crypt_info with a
+ * RELEASE barrier so that other tasks can ACQUIRE it.
+ */
+ if (cmpxchg_release(&inode->i_crypt_info, NULL, ci) != NULL)
+ return false;
+
+ /*
+ * We won the race and set ->i_crypt_info to our crypt_info.
+ * Now link it into the master key's inode list.
+ */
+ if (mk) {
+ ci->ci_master_key = mk;
+ refcount_inc(&mk->mk_active_refs);
+ spin_lock(&mk->mk_decrypted_inodes_lock);
+ list_add(&ci->ci_master_key_link, &mk->mk_decrypted_inodes);
+ spin_unlock(&mk->mk_decrypted_inodes_lock);
+ }
+ return true;
+}
+
static int
fscrypt_setup_encryption_info(struct inode *inode,
const union fscrypt_policy *policy,
@@ -525,6 +557,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
struct fscrypt_mode *mode;
struct fscrypt_master_key *mk = NULL;
int res;
+ bool set_succeeded;
res = fscrypt_initialize(inode->i_sb->s_cop->flags);
if (res)
@@ -550,34 +583,15 @@ fscrypt_setup_encryption_info(struct inode *inode,
if (res)
goto out;
- /*
- * For existing inodes, multiple tasks may race to set ->i_crypt_info.
- * So use cmpxchg_release(). This pairs with the smp_load_acquire() in
- * fscrypt_get_info(). I.e., here we publish ->i_crypt_info with a
- * RELEASE barrier so that other tasks can ACQUIRE it.
- */
- if (cmpxchg_release(&inode->i_crypt_info, NULL, crypt_info) == NULL) {
- /*
- * We won the race and set ->i_crypt_info to our crypt_info.
- * Now link it into the master key's inode list.
- */
- if (mk) {
- crypt_info->ci_master_key = mk;
- refcount_inc(&mk->mk_active_refs);
- spin_lock(&mk->mk_decrypted_inodes_lock);
- list_add(&crypt_info->ci_master_key_link,
- &mk->mk_decrypted_inodes);
- spin_unlock(&mk->mk_decrypted_inodes_lock);
- }
- crypt_info = NULL;
- }
+ set_succeeded = fscrypt_set_inode_info(inode, crypt_info, mk);
res = 0;
out:
if (mk) {
up_read(&mk->mk_sem);
fscrypt_put_master_key(mk);
}
- put_crypt_info(crypt_info);
+ if (!set_succeeded)
+ put_crypt_info(crypt_info);
return res;
}
@@ -707,7 +721,7 @@ EXPORT_SYMBOL_GPL(fscrypt_prepare_new_inode);
void fscrypt_put_encryption_info(struct inode *inode)
{
put_crypt_info(fscrypt_get_inode_info(inode));
- inode->i_crypt_info = NULL;
+ fscrypt_set_inode_info(inode, NULL, NULL);
}
EXPORT_SYMBOL(fscrypt_put_encryption_info);
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 05/17] fscrypt: use parent dir's info for extent-based encryption.
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (3 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 04/17] fscrypt: factor out fscrypt_set_inode_info() Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 06/17] fscrypt: add a super_block pointer to fscrypt_info Sweet Tea Dorminy
` (12 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
For regular files in filesystems using extent-based encryption, the
corresponding inode does
not need a fscrypt_info structure of its own as for inode-based fscrypt, as they will not be
encrypting anything using it. Any new extents written to the inode will
use a per-extent info structure derived from the inode's parent
directory's info structure. However, it is convenient to cache that
parent directory's info structure in the inode; it makes it easy to
check whether the parents' info exists, so that we don't have to get and
put a reference to the parent inode every time we want to get the inode
info. So do that.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/fscrypt_private.h | 18 ++++++++++++++++++
fs/crypto/keysetup.c | 27 ++++++++++++++++++++++++++-
2 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 2df28c6fe558..e4c9c483114f 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -262,6 +262,24 @@ typedef enum {
FS_ENCRYPT,
} fscrypt_direction_t;
+/**
+ * fscrypt_uses_extent_encryption() -- whether an inode uses per-extent
+ * encryption
+ *
+ * @param inode the inode in question
+ *
+ * Return: true if the inode uses per-extent encryption infos, false otherwise
+ */
+static inline bool fscrypt_uses_extent_encryption(const struct inode *inode)
+{
+ // Non-regular files don't have extents
+ if (!S_ISREG(inode->i_mode))
+ return false;
+
+ // No filesystem currently uses per-extent infos
+ return false;
+}
+
/**
* fscrypt_get_inode_info() - get the fscrypt_info for a particular inode
*
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 4d7ff8244c55..52244e0dd1e4 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -622,6 +622,22 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
if (fscrypt_has_encryption_key(inode))
return 0;
+ if (fscrypt_uses_extent_encryption(inode)) {
+ struct dentry *dentry = d_find_any_alias(inode);
+ struct dentry *parent_dentry = dget_parent(dentry);
+ struct inode *dir = parent_dentry->d_inode;
+ struct fscrypt_info *dir_info = fscrypt_get_inode_info(dir);
+ struct fscrypt_master_key *mk = NULL;
+
+ if (dir_info)
+ mk = dir_info->ci_master_key;
+
+ fscrypt_set_inode_info(inode, dir_info, mk);
+ dput(parent_dentry);
+ dput(dentry);
+ return 0;
+ }
+
res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
if (res < 0) {
if (res == -ERANGE && allow_unsupported)
@@ -704,6 +720,14 @@ int fscrypt_prepare_new_inode(struct inode *dir, struct inode *inode,
*encrypt_ret = true;
+ if (fscrypt_uses_extent_encryption(inode)) {
+ struct fscrypt_info *dir_info = fscrypt_get_inode_info(dir);
+
+ fscrypt_set_inode_info(inode, dir_info,
+ dir_info->ci_master_key);
+ return 0;
+ }
+
get_random_bytes(nonce, FSCRYPT_FILE_NONCE_SIZE);
return fscrypt_setup_encryption_info(inode, policy, nonce,
IS_CASEFOLDED(dir) &&
@@ -720,7 +744,8 @@ EXPORT_SYMBOL_GPL(fscrypt_prepare_new_inode);
*/
void fscrypt_put_encryption_info(struct inode *inode)
{
- put_crypt_info(fscrypt_get_inode_info(inode));
+ if (!fscrypt_uses_extent_encryption(inode))
+ put_crypt_info(fscrypt_get_inode_info(inode));
fscrypt_set_inode_info(inode, NULL, NULL);
}
EXPORT_SYMBOL(fscrypt_put_encryption_info);
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 06/17] fscrypt: add a super_block pointer to fscrypt_info
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (4 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 05/17] fscrypt: use parent dir's info for extent-based encryption Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 07/17] fscrypt: update comments about inodes to include extents Sweet Tea Dorminy
` (11 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
When fscrypt_infos are attached to extents instead of inodes, we can't
go through the inode to get at the filesystems's superblock. Therefore,
add a dedicated superblock pointer to fscrypt_info to keep track of it.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/fscrypt_private.h | 3 +++
fs/crypto/inline_crypt.c | 6 +++---
fs/crypto/keysetup.c | 9 ++++-----
fs/crypto/keysetup_v1.c | 6 +++---
4 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index e4c9c483114f..da4df8642456 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -220,6 +220,9 @@ struct fscrypt_info {
/* Back-pointer to the inode */
struct inode *ci_inode;
+ /* The superblock of the filesystem to which this fscrypt_info pertains */
+ struct super_block *ci_sb;
+
/*
* The master key with which this inode was unlocked (decrypted). This
* will be NULL if the master key was found in a process-subscribed
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 0ae220c8afa3..7e85167d9f37 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -41,7 +41,7 @@ static struct block_device **fscrypt_get_devices(struct super_block *sb,
static unsigned int fscrypt_get_dun_bytes(const struct fscrypt_info *ci)
{
- struct super_block *sb = ci->ci_inode->i_sb;
+ struct super_block *sb = ci->ci_sb;
unsigned int flags = fscrypt_policy_flags(&ci->ci_policy);
int ino_bits = 64, lblk_bits = 64;
@@ -95,7 +95,7 @@ static void fscrypt_log_blk_crypto_impl(struct fscrypt_mode *mode,
int fscrypt_select_encryption_impl(struct fscrypt_info *ci)
{
const struct inode *inode = ci->ci_inode;
- struct super_block *sb = inode->i_sb;
+ struct super_block *sb = ci->ci_sb;
struct blk_crypto_config crypto_cfg;
struct block_device **devs;
unsigned int num_devs;
@@ -158,7 +158,7 @@ int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
const struct fscrypt_info *ci)
{
const struct inode *inode = ci->ci_inode;
- struct super_block *sb = inode->i_sb;
+ struct super_block *sb = ci->ci_sb;
enum blk_crypto_mode_num crypto_mode = ci->ci_mode->blk_crypto_mode;
struct blk_crypto_key *blk_key;
struct block_device **devs;
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 52244e0dd1e4..dacf3c47a24a 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -174,8 +174,7 @@ static int setup_per_mode_enc_key(struct fscrypt_info *ci,
struct fscrypt_prepared_key *keys,
u8 hkdf_context, bool include_fs_uuid)
{
- const struct inode *inode = ci->ci_inode;
- const struct super_block *sb = inode->i_sb;
+ const struct super_block *sb = ci->ci_sb;
struct fscrypt_mode *mode = ci->ci_mode;
const u8 mode_num = mode - fscrypt_modes;
struct fscrypt_prepared_key *prep_key;
@@ -435,7 +434,7 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
if (err)
return err;
- mk = fscrypt_find_master_key(ci->ci_inode->i_sb, &mk_spec);
+ mk = fscrypt_find_master_key(ci->ci_sb, &mk_spec);
if (!mk) {
if (ci->ci_policy.version != FSCRYPT_POLICY_V1)
return -ENOKEY;
@@ -495,8 +494,7 @@ 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_inode->i_sb,
- &ci->ci_enc_key);
+ fscrypt_destroy_prepared_key(ci->ci_sb, &ci->ci_enc_key);
mk = ci->ci_master_key;
if (mk) {
@@ -568,6 +566,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
return -ENOMEM;
crypt_info->ci_inode = inode;
+ crypt_info->ci_sb = inode->i_sb;
crypt_info->ci_policy = *policy;
memcpy(crypt_info->ci_nonce, nonce, FSCRYPT_FILE_NONCE_SIZE);
diff --git a/fs/crypto/keysetup_v1.c b/fs/crypto/keysetup_v1.c
index 75dabd9b27f9..3cbf1480c457 100644
--- a/fs/crypto/keysetup_v1.c
+++ b/fs/crypto/keysetup_v1.c
@@ -232,7 +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;
+ dk->dk_sb = ci->ci_sb;
refcount_set(&dk->dk_refcount, 1);
dk->dk_mode = ci->ci_mode;
err = fscrypt_prepare_key(&dk->dk_key, raw_key, ci);
@@ -306,8 +306,8 @@ int fscrypt_setup_v1_file_key_via_subscribed_keyrings(struct fscrypt_info *ci)
key = find_and_lock_process_key(FSCRYPT_KEY_DESC_PREFIX,
ci->ci_policy.v1.master_key_descriptor,
ci->ci_mode->keysize, &payload);
- if (key == ERR_PTR(-ENOKEY) && ci->ci_inode->i_sb->s_cop->key_prefix) {
- key = find_and_lock_process_key(ci->ci_inode->i_sb->s_cop->key_prefix,
+ if (key == ERR_PTR(-ENOKEY) && ci->ci_sb->s_cop->key_prefix) {
+ key = find_and_lock_process_key(ci->ci_sb->s_cop->key_prefix,
ci->ci_policy.v1.master_key_descriptor,
ci->ci_mode->keysize, &payload);
}
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 07/17] fscrypt: update comments about inodes to include extents
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (5 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 06/17] fscrypt: add a super_block pointer to fscrypt_info Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 08/17] fscrypt: rename mk->mk_decrypted_inodes* Sweet Tea Dorminy
` (10 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
There are a bunch of comments that need updating to refer to any sort of
owning object, be it inode or extent, and they don't really fit anywhere
else. So do it.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/fscrypt_private.h | 57 +++++++++++++++++++++----------------
fs/crypto/inline_crypt.c | 9 ++++--
fs/crypto/keyring.c | 24 +++++++++-------
fs/crypto/keysetup.c | 6 ++--
4 files changed, 55 insertions(+), 41 deletions(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index da4df8642456..71e16d188fe8 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -53,14 +53,15 @@ struct fscrypt_context_v2 {
};
/*
- * fscrypt_context - the encryption context of an inode
+ * fscrypt_context - the encryption context of an object
*
* This is the on-disk equivalent of an fscrypt_policy, stored alongside each
- * encrypted file usually in a hidden extended attribute. It contains the
- * fields from the fscrypt_policy, in order to identify the encryption algorithm
- * and key with which the file is encrypted. It also contains a nonce that was
- * randomly generated by fscrypt itself; this is used as KDF input or as a tweak
- * to cause different files to be encrypted differently.
+ * encrypted file usually in a hidden extended attribute, or along with an extent.
+ * It contains the fields from the fscrypt_policy, in order to identify the
+ * encryption algorithm and key with which the file or extent is encrypted. It
+ * also contains a nonce that was randomly generated by fscrypt itself; this is
+ * used as KDF input or as a tweak to cause different files to be encrypted
+ * differently.
*/
union fscrypt_context {
u8 version;
@@ -189,11 +190,19 @@ struct fscrypt_prepared_key {
};
/*
- * fscrypt_info - the "encryption key" for an inode
+ * fscrypt_info - the "encryption key" for an object (inode or extent)
*
- * When an encrypted file's key is made available, an instance of this struct is
- * allocated and stored in ->i_crypt_info. Once created, it remains until the
- * inode is evicted.
+ * For filesystems using inode-based encryption, or non-regular files for
+ * filesystems with extent-based encryption: when an encrypted file's key is
+ * made available, an instance of this struct is allocated and stored in
+ * ->i_crypt_info. Once created, it remains until the inode is evicted.
+ *
+ * For filesystems using extent-based encryption, for regular files: when
+ * a file is opened, a pointer to the parent directory's fscrypt_info is stored
+ * in the inode's ->i_crypt_info. When an extent's key is made available, an
+ * instance of this struct is allocated and should be stored with the extent.
+ * It is freed when the filesystem decides to drop the extent, or when the key
+ * needs to be removed.
*/
struct fscrypt_info {
@@ -205,33 +214,33 @@ struct fscrypt_info {
#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
/*
- * True if this inode will use inline encryption (blk-crypto) instead of
+ * True if this info will use inline encryption (blk-crypto) instead of
* the traditional filesystem-layer encryption.
*/
bool ci_inlinecrypt;
#endif
/*
- * Encryption mode used for this inode. It corresponds to either the
+ * Encryption mode for this info. It corresponds to either the
* contents or filenames encryption mode, depending on the inode type.
*/
struct fscrypt_mode *ci_mode;
- /* Back-pointer to the inode */
+ /* Back-pointer to the inode, for infos owned by a specific inode */
struct inode *ci_inode;
/* The superblock of the filesystem to which this fscrypt_info pertains */
struct super_block *ci_sb;
/*
- * The master key with which this inode was unlocked (decrypted). This
+ * The master key with which this info was unlocked (decrypted). This
* will be NULL if the master key was found in a process-subscribed
* keyring rather than in the filesystem-level keyring.
*/
struct fscrypt_master_key *ci_master_key;
/*
- * Link in list of inodes that were unlocked with the master key.
+ * Link in list of infos that were unlocked with the master key.
* Only used when ->ci_master_key is set.
*/
struct list_head ci_master_key_link;
@@ -243,20 +252,20 @@ struct fscrypt_info {
struct fscrypt_direct_key *ci_direct_key;
/*
- * This inode's hash key for filenames. This is a 128-bit SipHash-2-4
+ * This infos' hash key for filenames. This is a 128-bit SipHash-2-4
* key. This is only set for directories that use a keyed dirhash over
* the plaintext filenames -- currently just casefolded directories.
*/
siphash_key_t ci_dirhash_key;
bool ci_dirhash_key_initialized;
- /* The encryption policy used by this inode */
+ /* The encryption policy used by this info */
union fscrypt_policy ci_policy;
- /* This inode's nonce, copied from the fscrypt_context */
+ /* This info's nonce, copied from the fscrypt_context */
u8 ci_nonce[FSCRYPT_FILE_NONCE_SIZE];
- /* Hashed inode number. Only set for IV_INO_LBLK_32 */
+ /* Hashed object number. Only set for IV_INO_LBLK_32 */
u32 ci_hashed_ino;
};
@@ -271,7 +280,7 @@ typedef enum {
*
* @param inode the inode in question
*
- * Return: true if the inode uses per-extent encryption infos, false otherwise
+ * Return: true if the inode uses per-extent fscrypt_infos, false otherwise
*/
static inline bool fscrypt_uses_extent_encryption(const struct inode *inode)
{
@@ -350,10 +359,10 @@ fscrypt_msg(const struct inode *inode, const char *level, const char *fmt, ...);
union fscrypt_iv {
struct {
- /* logical block number within the file */
+ /* logical block number within the owning object */
__le64 lblk_num;
- /* per-file nonce; only set in DIRECT_KEY mode */
+ /* per-object nonce; only set in DIRECT_KEY mode */
u8 nonce[FSCRYPT_FILE_NONCE_SIZE];
};
u8 raw[FSCRYPT_MAX_IV_SIZE];
@@ -684,9 +693,9 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported);
/**
* fscrypt_require_key() - require an inode's encryption key
- * @inode: the inode we need the key for
*
- * If the inode is encrypted, set up its encryption key if not already done.
+ * @inode: the inode we need the key for
+ * * If the inode is encrypted, set up its encryption key if not already done.
* Then require that the key be present and return -ENOKEY otherwise.
*
* No locks are needed, and the key will live as long as the struct inode --- so
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 7e85167d9f37..808c8712ebe7 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -344,8 +344,9 @@ EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx_bh);
* fscrypt_set_bio_crypt_ctx() must have already been called on the bio.
*
* This function isn't required in cases where crypto-mergeability is ensured in
- * another way, such as I/O targeting only a single file (and thus a single key)
- * combined with fscrypt_limit_io_blocks() to ensure DUN contiguity.
+ * another way, such as I/O targeting only a single file combined with
+ * fscrypt_limit_io_blocks() to ensure DUN contiguity (which also ensures,
+ * for extent-based encryption, that the I/O doesn't span extents).
*
* Return: true iff the I/O is mergeable
*/
@@ -458,7 +459,9 @@ EXPORT_SYMBOL_GPL(fscrypt_dio_supported);
* DUN to wrap around within logically contiguous blocks, and that wraparound
* will occur. If this happens, a value less than @nr_blocks will be returned
* so that the wraparound doesn't occur in the middle of a bio, which would
- * cause encryption/decryption to produce wrong results.
+ * cause encryption/decryption to produce wrong results. Similarly, the
+ * filesystem could be using extent-based encryption, which will return a value
+ * less than @nr_blocks if needed to prevent spanning multiple extents.
*
* Return: the actual number of blocks that can be submitted
*/
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 2a24b1f0ae68..f825d800867c 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -876,7 +876,7 @@ static void shrink_dcache_inode(struct inode *inode)
d_prune_aliases(inode);
}
-static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
+static void evict_dentries_for_decrypted_objects(struct fscrypt_master_key *mk)
{
struct fscrypt_info *ci;
struct inode *inode;
@@ -970,12 +970,14 @@ static int try_to_lock_encrypted_files(struct super_block *sb,
* Inodes are pinned by their dentries, so we have to evict their
* dentries. shrink_dcache_sb() would suffice, but would be overkill
* and inappropriate for use by unprivileged users. So instead go
- * through the inodes' alias lists and try to evict each dentry.
+ * through the inodes' alias lists and try to evict each dentry. Also,
+ * for extent-based encryption, notify the filesystem that it must free
+ * all the infos for extents using this key.
*/
- evict_dentries_for_decrypted_inodes(mk);
+ evict_dentries_for_decrypted_objects(mk);
/*
- * evict_dentries_for_decrypted_inodes() already iput() each inode in
+ * evict_dentries_for_decrypted_objects() already iput() each inode in
* the list; any inodes for which that dropped the last reference will
* have been evicted due to fscrypt_drop_inode() detecting the key
* removal and telling the VFS to evict the inode. So to finish, we
@@ -995,14 +997,14 @@ static int try_to_lock_encrypted_files(struct super_block *sb,
* key itself.
*
* To "remove the key itself", first we wipe the actual master key secret, so
- * that no more inodes can be unlocked with it. Then we try to evict all cached
- * inodes that had been unlocked with the key.
+ * that nothing else can be unlocked with it. Then we try to evict all cached
+ * inodes and extents that had been unlocked with the key.
*
- * If all inodes were evicted, then we unlink the fscrypt_master_key from the
- * keyring. Otherwise it remains in the keyring in the "incompletely removed"
- * state (without the actual secret key) where it tracks the list of remaining
- * inodes. Userspace can execute the ioctl again later to retry eviction, or
- * alternatively can re-add the secret key again.
+ * If all were evicted, then we unlink the fscrypt_master_key from the keyring.
+ * Otherwise it remains in the keyring in the "incompletely removed" state
+ * (without the actual secret key) where it tracks the list of remaining inodes
+ * and extents. Userspace can execute the ioctl again later to retry eviction,
+ * or alternatively can re-add the secret key again.
*
* For more details, see the "Removing keys" section of
* Documentation/filesystems/fscrypt.rst.
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index dacf3c47a24a..22e3ce5d61dc 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -334,9 +334,9 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64) {
/*
* IV_INO_LBLK_64: encryption keys are derived from (master_key,
- * mode_num, filesystem_uuid), and inode number is included in
- * the IVs. This format is optimized for use with inline
- * encryption hardware compliant with the UFS standard.
+ * mode_num, filesystem_uuid), and inode/extent number is
+ * included in 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,
HKDF_CONTEXT_IV_INO_LBLK_64_KEY,
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 08/17] fscrypt: rename mk->mk_decrypted_inodes*
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (6 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 07/17] fscrypt: update comments about inodes to include extents Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 09/17] fscrypt: make fscrypt_setup_encryption_info generic for extents Sweet Tea Dorminy
` (9 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
With the advent of extent-owned infos, we will need to track both inodes
and extents in each master key, in order to appropriately notify
filesystems that an extent needs to forget its fscrypt_info. Thus, it no
longer makes sense for the list of these objects to be called 'decrypted
inodes'. This change renames mk_decrypted_inodes{,_lock} to
mk_active_infos{,_lock} since the list tracks active fscrypt_info
objects of any provenance.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/fscrypt_private.h | 12 ++++++------
fs/crypto/keyring.c | 30 +++++++++++++++---------------
fs/crypto/keysetup.c | 12 ++++++------
3 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 71e16d188fe8..0c7b785f1d8c 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -537,7 +537,7 @@ struct fscrypt_master_key {
* A structural ref only guarantees that the struct continues to exist.
*
* There is one active ref associated with ->mk_secret being present,
- * and one active ref for each inode in ->mk_decrypted_inodes.
+ * and one active ref for each inode in ->mk_active_infos.
*
* There is one structural ref associated with the active refcount being
* nonzero. Finding a key in the keyring also takes a structural ref,
@@ -551,7 +551,7 @@ struct fscrypt_master_key {
/*
* The secret key material. After FS_IOC_REMOVE_ENCRYPTION_KEY is
* executed, this is wiped and no new inodes can be unlocked with this
- * key; however, there may still be inodes in ->mk_decrypted_inodes
+ * key; however, there may still be inodes in ->mk_active_infos
* which could not be evicted. As long as some inodes still remain,
* FS_IOC_REMOVE_ENCRYPTION_KEY can be retried, or
* FS_IOC_ADD_ENCRYPTION_KEY can add the secret again.
@@ -587,11 +587,11 @@ struct fscrypt_master_key {
struct key *mk_users;
/*
- * List of inodes that were unlocked using this key. This allows the
- * inodes to be evicted efficiently if the key is removed.
+ * List of infos that were unlocked using this key. This allows the
+ * owning objects to be evicted efficiently if the key is removed.
*/
- struct list_head mk_decrypted_inodes;
- spinlock_t mk_decrypted_inodes_lock;
+ struct list_head mk_active_infos;
+ spinlock_t mk_active_infos_lock;
/*
* Per-mode encryption keys for the various types of encryption policies
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index f825d800867c..846f480da081 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -99,10 +99,10 @@ void fscrypt_put_master_key_activeref(struct fscrypt_master_key *mk)
/*
* ->mk_active_refs == 0 implies that ->mk_secret is not present and
- * that ->mk_decrypted_inodes is empty.
+ * that ->mk_active_infos is empty.
*/
WARN_ON(is_master_key_secret_present(&mk->mk_secret));
- WARN_ON(!list_empty(&mk->mk_decrypted_inodes));
+ WARN_ON(!list_empty(&mk->mk_active_infos));
for (i = 0; i <= FSCRYPT_MODE_MAX; i++) {
fscrypt_destroy_prepared_key(
@@ -429,8 +429,8 @@ static int add_new_master_key(struct super_block *sb,
refcount_set(&mk->mk_struct_refs, 1);
mk->mk_spec = *mk_spec;
- INIT_LIST_HEAD(&mk->mk_decrypted_inodes);
- spin_lock_init(&mk->mk_decrypted_inodes_lock);
+ INIT_LIST_HEAD(&mk->mk_active_infos);
+ spin_lock_init(&mk->mk_active_infos_lock);
if (mk_spec->type == FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER) {
err = allocate_master_key_users_keyring(mk);
@@ -882,9 +882,9 @@ static void evict_dentries_for_decrypted_objects(struct fscrypt_master_key *mk)
struct inode *inode;
struct inode *toput_inode = NULL;
- spin_lock(&mk->mk_decrypted_inodes_lock);
+ spin_lock(&mk->mk_active_infos_lock);
- list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) {
+ list_for_each_entry(ci, &mk->mk_active_infos, ci_master_key_link) {
inode = ci->ci_inode;
spin_lock(&inode->i_lock);
if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
@@ -893,16 +893,16 @@ static void evict_dentries_for_decrypted_objects(struct fscrypt_master_key *mk)
}
__iget(inode);
spin_unlock(&inode->i_lock);
- spin_unlock(&mk->mk_decrypted_inodes_lock);
+ spin_unlock(&mk->mk_active_infos_lock);
shrink_dcache_inode(inode);
iput(toput_inode);
toput_inode = inode;
- spin_lock(&mk->mk_decrypted_inodes_lock);
+ spin_lock(&mk->mk_active_infos_lock);
}
- spin_unlock(&mk->mk_decrypted_inodes_lock);
+ spin_unlock(&mk->mk_active_infos_lock);
iput(toput_inode);
}
@@ -914,25 +914,25 @@ static int check_for_busy_inodes(struct super_block *sb,
unsigned long ino;
char ino_str[50] = "";
- spin_lock(&mk->mk_decrypted_inodes_lock);
+ spin_lock(&mk->mk_active_infos_lock);
- list_for_each(pos, &mk->mk_decrypted_inodes)
+ list_for_each(pos, &mk->mk_active_infos)
busy_count++;
if (busy_count == 0) {
- spin_unlock(&mk->mk_decrypted_inodes_lock);
+ spin_unlock(&mk->mk_active_infos_lock);
return 0;
}
{
/* select an example file to show for debugging purposes */
struct inode *inode =
- list_first_entry(&mk->mk_decrypted_inodes,
+ list_first_entry(&mk->mk_active_infos,
struct fscrypt_info,
ci_master_key_link)->ci_inode;
ino = inode->i_ino;
}
- spin_unlock(&mk->mk_decrypted_inodes_lock);
+ spin_unlock(&mk->mk_active_infos_lock);
/* If the inode is currently being created, ino may still be 0. */
if (ino)
@@ -954,7 +954,7 @@ static int try_to_lock_encrypted_files(struct super_block *sb,
/*
* An inode can't be evicted while it is dirty or has dirty pages.
- * Thus, we first have to clean the inodes in ->mk_decrypted_inodes.
+ * Thus, we first have to clean the inodes in ->mk_active_infos.
*
* Just do it the easy way: call sync_filesystem(). It's overkill, but
* it works, and it's more important to minimize the amount of caches we
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 22e3ce5d61dc..cb3ba4e4f816 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -414,7 +414,7 @@ static bool fscrypt_valid_master_key_size(const struct fscrypt_master_key *mk,
*
* If the master key is found in the filesystem-level keyring, then it is
* returned in *mk_ret with its semaphore read-locked. This is needed to ensure
- * that only one task links the fscrypt_info into ->mk_decrypted_inodes (as
+ * that only one task links the fscrypt_info into ->mk_active_infos (as
* multiple tasks may race to create an fscrypt_info for the same inode), and to
* synchronize the master key being removed with a new inode starting to use it.
*/
@@ -504,9 +504,9 @@ static void put_crypt_info(struct fscrypt_info *ci)
* inode from a master key struct that already had its secret
* removed, then complete the full removal of the struct.
*/
- spin_lock(&mk->mk_decrypted_inodes_lock);
+ spin_lock(&mk->mk_active_infos_lock);
list_del(&ci->ci_master_key_link);
- spin_unlock(&mk->mk_decrypted_inodes_lock);
+ spin_unlock(&mk->mk_active_infos_lock);
fscrypt_put_master_key_activeref(mk);
}
memzero_explicit(ci, sizeof(*ci));
@@ -538,9 +538,9 @@ static bool fscrypt_set_inode_info(struct inode *inode,
if (mk) {
ci->ci_master_key = mk;
refcount_inc(&mk->mk_active_refs);
- spin_lock(&mk->mk_decrypted_inodes_lock);
- list_add(&ci->ci_master_key_link, &mk->mk_decrypted_inodes);
- spin_unlock(&mk->mk_decrypted_inodes_lock);
+ spin_lock(&mk->mk_active_infos_lock);
+ list_add(&ci->ci_master_key_link, &mk->mk_active_infos);
+ spin_unlock(&mk->mk_active_infos_lock);
}
return true;
}
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 09/17] fscrypt: make fscrypt_setup_encryption_info generic for extents
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (7 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 08/17] fscrypt: rename mk->mk_decrypted_inodes* Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 10/17] fscrypt: let fscrypt_infos be owned by an extent Sweet Tea Dorminy
` (8 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
fscrypt_setup_encryption_info() will be used for setting up both inode
and extent fscrypt_infos, so it can't just assume that it should set
inode->i_crypt_info to the new fscrypt_info. Instead, this changes it to
take a pointer to the location to store the new fscrypt_info, which
currently is always &inode->i_crypt_info. Correspondingly,
fscrypt_setup_encryption_info() currently calls fscrypt_set_inode_info()
to set the inode's info; this renames it to fscrypt_set_info() and
plumbs through the pointer to the location to store the new
fscrypt_info.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/keysetup.c | 38 +++++++++++++++++++++-----------------
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index cb3ba4e4f816..04f01da900ca 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -513,22 +513,23 @@ static void put_crypt_info(struct fscrypt_info *ci)
kmem_cache_free(fscrypt_info_cachep, ci);
}
-static bool fscrypt_set_inode_info(struct inode *inode,
- struct fscrypt_info *ci,
- struct fscrypt_master_key *mk)
+static bool fscrypt_set_info(struct fscrypt_info **info_ptr,
+ struct fscrypt_info *ci,
+ struct fscrypt_master_key *mk)
{
if (ci == NULL) {
- inode->i_crypt_info = NULL;
+ *info_ptr = NULL;
return true;
}
/*
- * For existing inodes, multiple tasks may race to set ->i_crypt_info.
- * So use cmpxchg_release(). This pairs with the smp_load_acquire() in
- * fscrypt_get_info(). I.e., here we publish ->i_crypt_info with a
- * RELEASE barrier so that other tasks can ACQUIRE it.
+ * For existing objects, multiple tasks may race to set ->i_crypt_info
+ * or the equivalent. So use cmpxchg_release(). This pairs with the
+ * smp_load_acquire() in fscrypt_get_info(). I.e., here we publish
+ * ->i_crypt_info with a RELEASE barrier so that other tasks can
+ * ACQUIRE it.
*/
- if (cmpxchg_release(&inode->i_crypt_info, NULL, ci) != NULL)
+ if (cmpxchg_release(info_ptr, NULL, ci) != NULL)
return false;
/*
@@ -549,7 +550,8 @@ static int
fscrypt_setup_encryption_info(struct inode *inode,
const union fscrypt_policy *policy,
const u8 nonce[FSCRYPT_FILE_NONCE_SIZE],
- bool need_dirhash_key)
+ bool need_dirhash_key,
+ struct fscrypt_info **info_ptr)
{
struct fscrypt_info *crypt_info;
struct fscrypt_mode *mode;
@@ -582,7 +584,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
if (res)
goto out;
- set_succeeded = fscrypt_set_inode_info(inode, crypt_info, mk);
+ set_succeeded = fscrypt_set_info(info_ptr, crypt_info, mk);
res = 0;
out:
if (mk) {
@@ -631,7 +633,7 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
if (dir_info)
mk = dir_info->ci_master_key;
- fscrypt_set_inode_info(inode, dir_info, mk);
+ fscrypt_set_info(&inode->i_crypt_info, dir_info, mk);
dput(parent_dentry);
dput(dentry);
return 0;
@@ -663,7 +665,8 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
res = fscrypt_setup_encryption_info(inode, &policy,
fscrypt_context_nonce(&ctx),
IS_CASEFOLDED(inode) &&
- S_ISDIR(inode->i_mode));
+ S_ISDIR(inode->i_mode),
+ &inode->i_crypt_info);
if (res == -ENOPKG && allow_unsupported) /* Algorithm unavailable? */
res = 0;
@@ -722,15 +725,16 @@ int fscrypt_prepare_new_inode(struct inode *dir, struct inode *inode,
if (fscrypt_uses_extent_encryption(inode)) {
struct fscrypt_info *dir_info = fscrypt_get_inode_info(dir);
- fscrypt_set_inode_info(inode, dir_info,
- dir_info->ci_master_key);
+ fscrypt_set_info(&inode->i_crypt_info, dir_info,
+ dir_info->ci_master_key);
return 0;
}
get_random_bytes(nonce, FSCRYPT_FILE_NONCE_SIZE);
return fscrypt_setup_encryption_info(inode, policy, nonce,
IS_CASEFOLDED(dir) &&
- S_ISDIR(inode->i_mode));
+ S_ISDIR(inode->i_mode),
+ &inode->i_crypt_info);
}
EXPORT_SYMBOL_GPL(fscrypt_prepare_new_inode);
@@ -745,7 +749,7 @@ void fscrypt_put_encryption_info(struct inode *inode)
{
if (!fscrypt_uses_extent_encryption(inode))
put_crypt_info(fscrypt_get_inode_info(inode));
- fscrypt_set_inode_info(inode, NULL, NULL);
+ fscrypt_set_info(&inode->i_crypt_info, NULL, NULL);
}
EXPORT_SYMBOL(fscrypt_put_encryption_info);
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 10/17] fscrypt: let fscrypt_infos be owned by an extent
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (8 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 09/17] fscrypt: make fscrypt_setup_encryption_info generic for extents Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 11/17] fscrypt: update all the *per_file_* function names Sweet Tea Dorminy
` (7 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
In order to notify extents when their info is part of a master key which
is going away, the fscrypt_info must have a backpointer to the extent
somehow. Similarly, if a fscrypt_info is owned by an extent, the info
must not have a pointer to an inode -- multiple inodes may reference a
extent, and the first inode to cause an extent's creation may have a
lifetime much shorter than the extent, so there is no inode pointer
safe to track in an extent-owned info. Therefore, this adds a new
pointer for extent-owned infos to track their extent and updates
fscrypt_setup_encryption_info() accordingly.
Since it's simple to track the piece of extent memory pointing to the
info, and for the extent to then go from such a pointer to the whole
extent via container_of(), we store that. Although some sort of generic
void * or some artificial fscrypt_extent embedded structure would also
work, those would require additional plumbing which doesn't seem
strictly required or clarifying.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/fscrypt_private.h | 6 ++++++
fs/crypto/keysetup.c | 6 +++++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 0c7b785f1d8c..dc45cd35391f 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -229,6 +229,12 @@ struct fscrypt_info {
/* Back-pointer to the inode, for infos owned by a specific inode */
struct inode *ci_inode;
+ /*
+ * Back-pointer to the info pointer in the extent, for infos owned
+ * by an extent
+ */
+ struct fscrypt_info **ci_info_ptr;
+
/* The superblock of the filesystem to which this fscrypt_info pertains */
struct super_block *ci_sb;
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 04f01da900ca..c611e2613aa6 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -567,7 +567,11 @@ fscrypt_setup_encryption_info(struct inode *inode,
if (!crypt_info)
return -ENOMEM;
- crypt_info->ci_inode = inode;
+ if (fscrypt_uses_extent_encryption(inode))
+ crypt_info->ci_info_ptr = info_ptr;
+ else
+ crypt_info->ci_inode = inode;
+
crypt_info->ci_sb = inode->i_sb;
crypt_info->ci_policy = *policy;
memcpy(crypt_info->ci_nonce, nonce, FSCRYPT_FILE_NONCE_SIZE);
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 11/17] fscrypt: update all the *per_file_* function names
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (9 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 10/17] fscrypt: let fscrypt_infos be owned by an extent Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 12/17] fscrypt: notify per-extent infos if master key vanishes Sweet Tea Dorminy
` (6 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
As they are no longer per-file but per-info, whether that info is
per-inode or per-extent, it seems better to rename all the relevant
functions to be per_info instead of per-key.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/fscrypt_private.h | 6 +++---
fs/crypto/keysetup.c | 22 +++++++++++-----------
fs/crypto/keysetup_v1.c | 18 +++++++++---------
3 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index dc45cd35391f..a34d2e525ddf 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -687,7 +687,7 @@ int fscrypt_prepare_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);
+int fscrypt_set_per_info_enc_key(struct fscrypt_info *ci, const u8 *raw_key);
int fscrypt_derive_dirhash_key(struct fscrypt_info *ci,
const struct fscrypt_master_key *mk);
@@ -727,10 +727,10 @@ static inline int fscrypt_require_key(struct inode *inode)
void fscrypt_put_direct_key(struct fscrypt_direct_key *dk);
-int fscrypt_setup_v1_file_key(struct fscrypt_info *ci,
+int fscrypt_setup_v1_info_key(struct fscrypt_info *ci,
const u8 *raw_master_key);
-int fscrypt_setup_v1_file_key_via_subscribed_keyrings(struct fscrypt_info *ci);
+int fscrypt_setup_v1_info_key_via_subscribed_keyrings(struct fscrypt_info *ci);
/* policy.c */
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index c611e2613aa6..1751e3ed9956 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -162,8 +162,8 @@ void fscrypt_destroy_prepared_key(struct super_block *sb,
memzero_explicit(prep_key, sizeof(*prep_key));
}
-/* Given a per-file encryption key, set up the file's crypto transform object */
-int fscrypt_set_per_file_enc_key(struct fscrypt_info *ci, const u8 *raw_key)
+/* Given a fscrypt_info, set up an appropriate crypto transform object */
+int fscrypt_set_per_info_enc_key(struct fscrypt_info *ci, const u8 *raw_key)
{
ci->ci_owns_key = true;
return fscrypt_prepare_key(&ci->ci_enc_key, raw_key, ci);
@@ -313,7 +313,7 @@ static int fscrypt_setup_iv_ino_lblk_32_key(struct fscrypt_info *ci,
return 0;
}
-static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
+static int fscrypt_setup_v2_info_key(struct fscrypt_info *ci,
struct fscrypt_master_key *mk,
bool need_dirhash_key)
{
@@ -321,8 +321,8 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
if (ci->ci_policy.v2.flags & FSCRYPT_POLICY_FLAG_DIRECT_KEY) {
/*
- * DIRECT_KEY: instead of deriving per-file encryption keys, the
- * per-file nonce will be included in all the IVs. But unlike
+ * DIRECT_KEY: instead of deriving per-info encryption keys, the
+ * per-info nonce will be included in all the IVs. But unlike
* v1 policies, for v2 policies in this case we don't encrypt
* with the master key directly but rather derive a per-mode
* encryption key. This ensures that the master key is
@@ -354,7 +354,7 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
if (err)
return err;
- err = fscrypt_set_per_file_enc_key(ci, derived_key);
+ err = fscrypt_set_per_info_enc_key(ci, derived_key);
memzero_explicit(derived_key, ci->ci_mode->keysize);
}
if (err)
@@ -418,7 +418,7 @@ static bool fscrypt_valid_master_key_size(const struct fscrypt_master_key *mk,
* multiple tasks may race to create an fscrypt_info for the same inode), and to
* synchronize the master key being removed with a new inode starting to use it.
*/
-static int setup_file_encryption_key(struct fscrypt_info *ci,
+static int setup_info_encryption_key(struct fscrypt_info *ci,
bool need_dirhash_key,
struct fscrypt_master_key **mk_ret)
{
@@ -445,7 +445,7 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
* to before the search of ->s_master_keys, since users
* shouldn't be able to override filesystem-level keys.
*/
- return fscrypt_setup_v1_file_key_via_subscribed_keyrings(ci);
+ return fscrypt_setup_v1_info_key_via_subscribed_keyrings(ci);
}
down_read(&mk->mk_sem);
@@ -462,10 +462,10 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
switch (ci->ci_policy.version) {
case FSCRYPT_POLICY_V1:
- err = fscrypt_setup_v1_file_key(ci, mk->mk_secret.raw);
+ err = fscrypt_setup_v1_info_key(ci, mk->mk_secret.raw);
break;
case FSCRYPT_POLICY_V2:
- err = fscrypt_setup_v2_file_key(ci, mk, need_dirhash_key);
+ err = fscrypt_setup_v2_info_key(ci, mk, need_dirhash_key);
break;
default:
WARN_ON(1);
@@ -584,7 +584,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
WARN_ON(mode->ivsize > FSCRYPT_MAX_IV_SIZE);
crypt_info->ci_mode = mode;
- res = setup_file_encryption_key(crypt_info, need_dirhash_key, &mk);
+ res = setup_info_encryption_key(crypt_info, need_dirhash_key, &mk);
if (res)
goto out;
diff --git a/fs/crypto/keysetup_v1.c b/fs/crypto/keysetup_v1.c
index 3cbf1480c457..3c3a203c2a94 100644
--- a/fs/crypto/keysetup_v1.c
+++ b/fs/crypto/keysetup_v1.c
@@ -250,7 +250,7 @@ fscrypt_get_direct_key(const struct fscrypt_info *ci, const u8 *raw_key)
}
/* v1 policy, DIRECT_KEY: use the master key directly */
-static int setup_v1_file_key_direct(struct fscrypt_info *ci,
+static int setup_v1_info_key_direct(struct fscrypt_info *ci,
const u8 *raw_master_key)
{
struct fscrypt_direct_key *dk;
@@ -263,8 +263,8 @@ static int setup_v1_file_key_direct(struct fscrypt_info *ci,
return 0;
}
-/* v1 policy, !DIRECT_KEY: derive the file's encryption key */
-static int setup_v1_file_key_derived(struct fscrypt_info *ci,
+/* v1 policy, !DIRECT_KEY: derive the info's encryption key */
+static int setup_v1_info_key_derived(struct fscrypt_info *ci,
const u8 *raw_master_key)
{
u8 *derived_key;
@@ -283,21 +283,21 @@ static int setup_v1_file_key_derived(struct fscrypt_info *ci,
if (err)
goto out;
- err = fscrypt_set_per_file_enc_key(ci, derived_key);
+ err = fscrypt_set_per_info_enc_key(ci, derived_key);
out:
kfree_sensitive(derived_key);
return err;
}
-int fscrypt_setup_v1_file_key(struct fscrypt_info *ci, const u8 *raw_master_key)
+int fscrypt_setup_v1_info_key(struct fscrypt_info *ci, const u8 *raw_master_key)
{
if (ci->ci_policy.v1.flags & FSCRYPT_POLICY_FLAG_DIRECT_KEY)
- return setup_v1_file_key_direct(ci, raw_master_key);
+ return setup_v1_info_key_direct(ci, raw_master_key);
else
- return setup_v1_file_key_derived(ci, raw_master_key);
+ return setup_v1_info_key_derived(ci, raw_master_key);
}
-int fscrypt_setup_v1_file_key_via_subscribed_keyrings(struct fscrypt_info *ci)
+int fscrypt_setup_v1_info_key_via_subscribed_keyrings(struct fscrypt_info *ci)
{
struct key *key;
const struct fscrypt_key *payload;
@@ -314,7 +314,7 @@ int fscrypt_setup_v1_file_key_via_subscribed_keyrings(struct fscrypt_info *ci)
if (IS_ERR(key))
return PTR_ERR(key);
- err = fscrypt_setup_v1_file_key(ci, payload->raw);
+ err = fscrypt_setup_v1_info_key(ci, payload->raw);
up_read(&key->sem);
key_put(key);
return err;
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 12/17] fscrypt: notify per-extent infos if master key vanishes
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (10 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 11/17] fscrypt: update all the *per_file_* function names Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 13/17] fscrypt: use an optional ino equivalent for per-extent infos Sweet Tea Dorminy
` (5 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
When fscrypt_infos can be owned by an extent instead of an inode, a
new method of evicting the per-extent info for a key being removed is
needed. This change adds removal handling to per-extent infos for master
key removal.
This seems racy to me and I'd love to find a better way to do this, but
I can't think of it.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/keyring.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 846f480da081..0c4e917a5281 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -886,6 +886,13 @@ static void evict_dentries_for_decrypted_objects(struct fscrypt_master_key *mk)
list_for_each_entry(ci, &mk->mk_active_infos, ci_master_key_link) {
inode = ci->ci_inode;
+ if (!inode) {
+ spin_unlock(&mk->mk_active_infos_lock);
+ ci->ci_sb->s_cop->forget_extent_info(ci->ci_info_ptr);
+ spin_lock(&mk->mk_active_infos_lock);
+ continue;
+ }
+
spin_lock(&inode->i_lock);
if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
spin_unlock(&inode->i_lock);
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 13/17] fscrypt: use an optional ino equivalent for per-extent infos
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (11 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 12/17] fscrypt: notify per-extent infos if master key vanishes Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 14/17] fscrypt: add creation/usage/freeing of " Sweet Tea Dorminy
` (4 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
Since per-extent infos are not tied to inodes, an ino-based policy
cannot access the inode's i_ino to get the necessary information.
Instead, this adds an optional fscrypt_operation pointer to get the ino
equivalent for an extent, adds a wrapper to get the ino for an info, and
uses this wrapper everywhere where the ci's inode's i_ino is currently
accessed.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/crypto.c | 5 +++--
fs/crypto/fscrypt_private.h | 18 ++++++++++++++++++
fs/crypto/keyring.c | 6 +++---
fs/crypto/keysetup.c | 8 ++++----
include/linux/fscrypt.h | 18 ++++++++++++++++++
5 files changed, 46 insertions(+), 9 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 93b83dbe82ee..4760adc1158f 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -85,9 +85,10 @@ void fscrypt_generate_iv(union fscrypt_iv *iv, u64 lblk_num,
memset(iv, 0, ci->ci_mode->ivsize);
if (flags & FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64) {
+ u64 ino = fscrypt_get_info_ino(ci);
WARN_ON_ONCE(lblk_num > U32_MAX);
- WARN_ON_ONCE(ci->ci_inode->i_ino > U32_MAX);
- lblk_num |= (u64)ci->ci_inode->i_ino << 32;
+ WARN_ON_ONCE(ino > U32_MAX);
+ lblk_num |= (u64)ino << 32;
} else if (flags & FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32) {
WARN_ON_ONCE(lblk_num > U32_MAX);
lblk_num = (u32)(ci->ci_hashed_ino + lblk_num);
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index a34d2e525ddf..d937a320361e 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -344,6 +344,24 @@ fscrypt_get_lblk_info(const struct inode *inode, u64 lblk, u64 *offset,
return fscrypt_get_info(inode);
}
+/**
+ * fscrypt_get_info_ino() - get the ino or ino equivalent for an info
+ *
+ * @ci: the fscrypt_info in question
+ *
+ * For inode-based encryption, this will return the info's inode's ino.
+ * For extent-based encryption, this will return the extent's ino equivalent
+ * or 0 if it is not implemented.
+ */
+static inline u64 fscrypt_get_info_ino(const struct fscrypt_info *ci)
+{
+ if (ci->ci_inode)
+ return ci->ci_inode->i_ino;
+ if (!ci->ci_sb->s_cop->get_extent_ino_equivalent)
+ return 0;
+ return ci->ci_sb->s_cop->get_extent_ino_equivalent(ci->ci_info_ptr);
+}
+
/* crypto.c */
extern struct kmem_cache *fscrypt_info_cachep;
int fscrypt_initialize(unsigned int cop_flags);
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 0c4e917a5281..48e732628d43 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -933,11 +933,11 @@ static int check_for_busy_inodes(struct super_block *sb,
{
/* select an example file to show for debugging purposes */
- struct inode *inode =
+ struct fscrypt_info *ci =
list_first_entry(&mk->mk_active_infos,
struct fscrypt_info,
- ci_master_key_link)->ci_inode;
- ino = inode->i_ino;
+ ci_master_key_link);
+ ino = fscrypt_get_info_ino(ci);
}
spin_unlock(&mk->mk_active_infos_lock);
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 1751e3ed9956..f32b6f0a8336 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -266,11 +266,11 @@ int fscrypt_derive_dirhash_key(struct fscrypt_info *ci,
void fscrypt_hash_inode_number(struct fscrypt_info *ci,
const struct fscrypt_master_key *mk)
{
- WARN_ON(ci->ci_inode->i_ino == 0);
+ u64 ino = fscrypt_get_info_ino(ci);
+ WARN_ON(ino == 0);
WARN_ON(!mk->mk_ino_hash_key_initialized);
- ci->ci_hashed_ino = (u32)siphash_1u64(ci->ci_inode->i_ino,
- &mk->mk_ino_hash_key);
+ ci->ci_hashed_ino = (u32)siphash_1u64(ino, &mk->mk_ino_hash_key);
}
static int fscrypt_setup_iv_ino_lblk_32_key(struct fscrypt_info *ci,
@@ -308,7 +308,7 @@ static int fscrypt_setup_iv_ino_lblk_32_key(struct fscrypt_info *ci,
* New inodes may not have an inode number assigned yet.
* Hashing their inode number is delayed until later.
*/
- if (ci->ci_inode->i_ino)
+ if (fscrypt_get_info_ino(ci))
fscrypt_hash_inode_number(ci, mk);
return 0;
}
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 4f5f8a651213..c05e6ad3e729 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -129,6 +129,15 @@ struct fscrypt_operations {
*/
bool (*empty_dir)(struct inode *inode);
+ /*
+ * Inform the filesystem that a particular extent must forget its
+ * fscrypt_info (for instance, for a key removal).
+ *
+ * @info_ptr: a pointer to the location storing the fscrypt_info pointer
+ * within the opaque extent whose info is to be freed
+ */
+ void (*forget_extent_info)(struct fscrypt_info **info_ptr);
+
/*
* Check whether the filesystem's inode numbers and UUID are stable,
* meaning that they will never be changed even by offline operations
@@ -160,6 +169,15 @@ struct fscrypt_operations {
void (*get_ino_and_lblk_bits)(struct super_block *sb,
int *ino_bits_ret, int *lblk_bits_ret);
+ /*
+ * Get the inode number equivalent for filesystems using per-extent
+ * encryption keys.
+ *
+ * This function only needs to be implemented if support for one of the
+ * FSCRYPT_POLICY_FLAG_IV_INO_* flags is needed.
+ */
+ u64 (*get_extent_ino_equivalent)(struct fscrypt_info **info_ptr);
+
/*
* Return an array of pointers to the block devices to which the
* filesystem may write encrypted file contents, NULL if the filesystem
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 14/17] fscrypt: add creation/usage/freeing of per-extent infos
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (12 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 13/17] fscrypt: use an optional ino equivalent for per-extent infos Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 15/17] fscrypt: allow load/save of extent contexts Sweet Tea Dorminy
` (3 subsequent siblings)
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
This change adds the superblock function pointer to get the info
corresponding to a specific block in an inode for a filesystem using
per-extent infos. It allows creating a info for a new extent and freeing
that info, and uses the extent's info if appropriate in encrypting
blocks of data.
This change does not deal with saving and loading an extent's info, but
introduces the mechanics necessary therefore.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/crypto.c | 6 ++++
fs/crypto/fscrypt_private.h | 14 +++++++--
fs/crypto/keysetup.c | 57 +++++++++++++++++++++++++++++++++++++
include/linux/fscrypt.h | 41 ++++++++++++++++++++++++++
4 files changed, 116 insertions(+), 2 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 4760adc1158f..b2a293e1df29 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -118,6 +118,12 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
return -EINVAL;
if (WARN_ON_ONCE(len % FSCRYPT_CONTENTS_ALIGNMENT != 0))
return -EINVAL;
+ if (!ci) {
+ fscrypt_err(inode,
+ "Error %d getting extent info for block %llu",
+ res, lblk_num);
+ return -EINVAL;
+ }
fscrypt_generate_iv(&iv, ci_offset, ci);
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index d937a320361e..eca911cca8a0 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -294,8 +294,7 @@ static inline bool fscrypt_uses_extent_encryption(const struct inode *inode)
if (!S_ISREG(inode->i_mode))
return false;
- // No filesystem currently uses per-extent infos
- return false;
+ return !!inode->i_sb->s_cop->get_extent_info;
}
/**
@@ -336,6 +335,17 @@ static inline struct fscrypt_info *
fscrypt_get_lblk_info(const struct inode *inode, u64 lblk, u64 *offset,
u64 *extent_len)
{
+ if (fscrypt_uses_extent_encryption(inode)) {
+ struct fscrypt_info *info;
+ int res;
+
+ res = inode->i_sb->s_cop->get_extent_info(inode, lblk, &info,
+ offset, extent_len);
+ if (res == 0)
+ return info;
+ return NULL;
+ }
+
if (offset)
*offset = lblk;
if (extent_len)
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index f32b6f0a8336..136156487e8f 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -742,6 +742,63 @@ int fscrypt_prepare_new_inode(struct inode *dir, struct inode *inode,
}
EXPORT_SYMBOL_GPL(fscrypt_prepare_new_inode);
+/**
+ * fscrypt_prepare_new_extent() - set up the fscrypt_info for a new extent
+ * @inode: the inode to which the extent belongs
+ * @info_ptr: a pointer to return the extent's fscrypt_info into. Should be
+ * a pointer to a member of the extent struct, as it will be passed
+ * back to the filesystem if key removal demands removal of the
+ * info from the extent
+ * @encrypt_ret: (output) set to %true if the new inode will be encrypted
+ *
+ * If the extent is part of an encrypted inode, set up its fscrypt_info in
+ * preparation for encrypting data and set *encrypt_ret=true.
+ *
+ * This isn't %GFP_NOFS-safe, and therefore it should be called before starting
+ * any filesystem transaction to create the inode.
+ *
+ * This doesn't persist the new inode's encryption context. That still needs to
+ * be done later by calling fscrypt_set_context().
+ *
+ * Return: 0 on success, -ENOKEY if the encryption key is missing, or another
+ * -errno code
+ */
+int fscrypt_prepare_new_extent(struct inode *inode,
+ struct fscrypt_info **info_ptr,
+ bool *encrypt_ret)
+{
+ const union fscrypt_policy *policy;
+ u8 nonce[FSCRYPT_FILE_NONCE_SIZE];
+
+ policy = fscrypt_policy_to_inherit(inode);
+ if (policy == NULL)
+ return 0;
+ if (IS_ERR(policy))
+ return PTR_ERR(policy);
+
+ /* Only regular files can have extents. */
+ if (WARN_ON_ONCE(!S_ISREG(inode->i_mode)))
+ return -EINVAL;
+
+ *encrypt_ret = true;
+
+ get_random_bytes(nonce, FSCRYPT_FILE_NONCE_SIZE);
+ return fscrypt_setup_encryption_info(inode, policy, nonce,
+ false, info_ptr);
+}
+EXPORT_SYMBOL_GPL(fscrypt_prepare_new_extent);
+
+/**
+ * fscrypt_free_extent_info() - free an extent's fscrypt_info
+ * @info_ptr: a pointer containing the extent's fscrypt_info pointer.
+ */
+void fscrypt_free_extent_info(struct fscrypt_info **info_ptr)
+{
+ put_crypt_info(*info_ptr);
+ fscrypt_set_info(info_ptr, NULL, NULL);
+}
+EXPORT_SYMBOL_GPL(fscrypt_free_extent_info);
+
/**
* fscrypt_put_encryption_info() - free most of an inode's fscrypt data
* @inode: an inode being evicted
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index c05e6ad3e729..6afdcb27f8a2 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -113,6 +113,29 @@ struct fscrypt_operations {
int (*set_context)(struct inode *inode, const void *ctx, size_t len,
void *fs_data);
+ /*
+ * Get the fscrypt_info for the given inode at the given block, for
+ * extent-based encryption only.
+ *
+ * @inode: the inode in question
+ * @lblk: the logical block number in question
+ * @ci: a pointer to return the fscrypt_info
+ * @offset: a pointer to return the offset of @lblk into the extent,
+ * in blocks (may be NULL)
+ * @extent_len: a pointer to return the number of blocks in this extent
+ * starting at this point (may be NULL)
+ *
+ * May cause the filesystem to allocate memory, which the filesystem
+ * must do with %GFP_NOFS, including calls into fscrypt to create or
+ * load an fscrypt_info.
+ *
+ * Return: 0 if an extent is found with an info, -ENODATA if the key is
+ * unavailable, or another -errno.
+ */
+ int (*get_extent_info)(const struct inode *inode, u64 lblk,
+ struct fscrypt_info **ci, u64 *offset,
+ u64 *extent_len);
+
/*
* Get the dummy fscrypt policy in use on the filesystem (if any).
*
@@ -340,6 +363,11 @@ void fscrypt_put_encryption_info(struct inode *inode);
void fscrypt_free_inode(struct inode *inode);
int fscrypt_drop_inode(struct inode *inode);
+int fscrypt_prepare_new_extent(struct inode *inode,
+ struct fscrypt_info **info_ptr,
+ bool *encrypt_ret);
+void fscrypt_free_extent_info(struct fscrypt_info **info_ptr);
+
/* fname.c */
int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
u8 *out, unsigned int olen);
@@ -597,6 +625,19 @@ static inline int fscrypt_drop_inode(struct inode *inode)
return 0;
}
+static inline int fscrypt_prepare_new_extent(struct inode *inode,
+ struct fscrypt_info **info_ptr,
+ bool *encrypt_ret)
+{
+ if (IS_ENCRYPTED(inode))
+ return -EOPNOTSUPP;
+ return 0;
+}
+
+static inline void fscrypt_free_extent_info(struct fscrypt_info **info_ptr)
+{
+}
+
/* fname.c */
static inline int fscrypt_setup_filename(struct inode *dir,
const struct qstr *iname,
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 15/17] fscrypt: allow load/save of extent contexts
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (13 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 14/17] fscrypt: add creation/usage/freeing of " Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-02 21:47 ` Eric Biggers
2023-01-01 5:06 ` [RFC PATCH 16/17] fscrypt: disable inline encryption for extent-based encryption Sweet Tea Dorminy
` (2 subsequent siblings)
17 siblings, 1 reply; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
The other half of using per-extent infos is saving and loading them from
disk.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/keysetup.c | 50 +++++++++++++++++++++++++++++++++++++++++
fs/crypto/policy.c | 20 +++++++++++++++++
include/linux/fscrypt.h | 17 ++++++++++++++
3 files changed, 87 insertions(+)
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 136156487e8f..82439fb73dd9 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -799,6 +799,56 @@ void fscrypt_free_extent_info(struct fscrypt_info **info_ptr)
}
EXPORT_SYMBOL_GPL(fscrypt_free_extent_info);
+/**
+ * fscrypt_load_extent_info() - set up a preexisting extent's fscrypt_info
+ * @inode: the inode to which the extent belongs. Must be encrypted.
+ * @buf: a buffer containing the extent's stored context
+ * @len: the length of the @ctx buffer
+ * @info_ptr: a pointer to return the extent's fscrypt_info into. Should be
+ * a pointer to a member of the extent struct, as it will be passed
+ * back to the filesystem if key removal demands removal of the
+ * info from the extent
+ *
+ * This is not %GFP_NOFS safe, so the caller is expected to call
+ * memalloc_nofs_save/restore() if appropriate.
+ *
+ * Return: 0 if successful, or -errno if it fails.
+ */
+int fscrypt_load_extent_info(struct inode *inode, void *buf, size_t len,
+ struct fscrypt_info **info_ptr)
+{
+ int res;
+ union fscrypt_context ctx;
+ union fscrypt_policy policy;
+
+ if (!fscrypt_has_encryption_key(inode))
+ return -EINVAL;
+
+ memcpy(&ctx, buf, len);
+
+ res = fscrypt_policy_from_context(&policy, &ctx, res);
+ if (res) {
+ fscrypt_warn(inode,
+ "Unrecognized or corrupt encryption context");
+ return res;
+ }
+
+ if (!fscrypt_supported_policy(&policy, inode)) {
+ return -EINVAL;
+ }
+
+ res = fscrypt_setup_encryption_info(inode, &policy,
+ fscrypt_context_nonce(&ctx),
+ IS_CASEFOLDED(inode) &&
+ S_ISDIR(inode->i_mode),
+ info_ptr);
+
+ if (res == -ENOPKG) /* Algorithm unavailable? */
+ res = 0;
+ return res;
+}
+EXPORT_SYMBOL_GPL(fscrypt_load_extent_info);
+
/**
* fscrypt_put_encryption_info() - free most of an inode's fscrypt data
* @inode: an inode being evicted
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index e7de4872d375..aab5edc1155e 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -751,6 +751,26 @@ int fscrypt_set_context(struct inode *inode, void *fs_data)
}
EXPORT_SYMBOL_GPL(fscrypt_set_context);
+/**
+ * fscrypt_set_extent_context() - Set the fscrypt context for an extent
+ * @ci: info from which to fetch policy and nonce
+ * @ctx: where context should be written
+ * @len: the size of ctx
+ *
+ * Given an fscrypt_info belonging to an extent (generated via
+ * fscrypt_prepare_new_extent()), generate a new context and write it to @ctx.
+ * len is checked to be at least FSCRYPT_SET_CONTEXT_MAX_SIZE bytes.
+ *
+ * Return: size of the resulting context or a negative error code.
+ */
+int fscrypt_set_extent_context(struct fscrypt_info *ci, void *ctx, size_t len)
+{
+ if (len < FSCRYPT_SET_CONTEXT_MAX_SIZE)
+ return -EINVAL;
+ return fscrypt_new_context(ctx, &ci->ci_policy, ci->ci_nonce);
+}
+EXPORT_SYMBOL_GPL(fscrypt_set_extent_context);
+
/**
* fscrypt_parse_test_dummy_encryption() - parse the test_dummy_encryption mount option
* @param: the mount option
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 6afdcb27f8a2..47c2df1061c7 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -324,6 +324,8 @@ int fscrypt_ioctl_get_nonce(struct file *filp, void __user *arg);
int fscrypt_has_permitted_context(struct inode *parent, struct inode *child);
int fscrypt_context_for_new_inode(void *ctx, struct inode *inode);
int fscrypt_set_context(struct inode *inode, void *fs_data);
+int fscrypt_set_extent_context(struct fscrypt_info *info, void *ctx,
+ size_t len);
struct fscrypt_dummy_policy {
const union fscrypt_policy *policy;
@@ -367,6 +369,8 @@ int fscrypt_prepare_new_extent(struct inode *inode,
struct fscrypt_info **info_ptr,
bool *encrypt_ret);
void fscrypt_free_extent_info(struct fscrypt_info **info_ptr);
+int fscrypt_load_extent_info(struct inode *inode, void *buf, size_t len,
+ struct fscrypt_info **info_ptr);
/* fname.c */
int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
@@ -532,6 +536,12 @@ static inline int fscrypt_set_context(struct inode *inode, void *fs_data)
return -EOPNOTSUPP;
}
+static inline int fscrypt_set_extent_context(struct fscrypt_info *info,
+ void *ctx, size_t len)
+{
+ return -EOPNOTSUPP;
+}
+
struct fscrypt_dummy_policy {
};
@@ -638,6 +648,13 @@ static inline void fscrypt_free_extent_info(struct fscrypt_info **info_ptr)
{
}
+static inline int fscrypt_load_extent_info(struct inode *inode, void *buf,
+ size_t len,
+ struct fscrypt_info **info_ptr)
+{
+ return -EOPNOTSUPP;
+}
+
/* fname.c */
static inline int fscrypt_setup_filename(struct inode *dir,
const struct qstr *iname,
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [RFC PATCH 15/17] fscrypt: allow load/save of extent contexts
2023-01-01 5:06 ` [RFC PATCH 15/17] fscrypt: allow load/save of extent contexts Sweet Tea Dorminy
@ 2023-01-02 21:47 ` Eric Biggers
2023-01-02 22:31 ` Sweet Tea Dorminy
0 siblings, 1 reply; 28+ messages in thread
From: Eric Biggers @ 2023-01-02 21:47 UTC (permalink / raw)
To: Sweet Tea Dorminy; +Cc: linux-fscrypt, paulcrowley, linux-btrfs, kernel-team
On Sun, Jan 01, 2023 at 12:06:19AM -0500, Sweet Tea Dorminy wrote:
> The other half of using per-extent infos is saving and loading them from
> disk.
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
> fs/crypto/keysetup.c | 50 +++++++++++++++++++++++++++++++++++++++++
> fs/crypto/policy.c | 20 +++++++++++++++++
> include/linux/fscrypt.h | 17 ++++++++++++++
> 3 files changed, 87 insertions(+)
>
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index 136156487e8f..82439fb73dd9 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -799,6 +799,56 @@ void fscrypt_free_extent_info(struct fscrypt_info **info_ptr)
> }
> EXPORT_SYMBOL_GPL(fscrypt_free_extent_info);
>
> +/**
> + * fscrypt_load_extent_info() - set up a preexisting extent's fscrypt_info
> + * @inode: the inode to which the extent belongs. Must be encrypted.
> + * @buf: a buffer containing the extent's stored context
> + * @len: the length of the @ctx buffer
> + * @info_ptr: a pointer to return the extent's fscrypt_info into. Should be
> + * a pointer to a member of the extent struct, as it will be passed
> + * back to the filesystem if key removal demands removal of the
> + * info from the extent
> + *
> + * This is not %GFP_NOFS safe, so the caller is expected to call
> + * memalloc_nofs_save/restore() if appropriate.
> + *
> + * Return: 0 if successful, or -errno if it fails.
> + */
> +int fscrypt_load_extent_info(struct inode *inode, void *buf, size_t len,
> + struct fscrypt_info **info_ptr)
> +{
When is the filesystem going to call fscrypt_load_extent_info()?
My concern (which we've discussed, but probably I didn't explain clearly enough)
is that the two "naive" solutions don't really work:
Option 1: Set up during the I/O to the extent. I think this is not feasible
because the full fscrypt_setup_encryption_info() is not safe to do doing I/O.
For example, it allocates memory under GFP_KERNEL, and it uses
crypto_alloc_skcipher() which can involve loading kernel modules.
Option 2: Set up for *all* extents when the file is opened. I expect that this
would not be feasible either, since a file can have a huge number of extents.
This patchset seems to be assuming one of those options. (It's not clear
whether it's Option 1 or Option 2, since the caller of
fscrypt_load_extent_info() is not included in the patchset.)
That leaves the option I suggested, which probably I didn't explain clearly
enough: split up key setup so that part can be done when opening the file, and
part can be done during I/O. Specifically, when opening the file, preallocate
some number of crypto_skcipher objects. This would be limited to a fixed
number, like 128, even if the file has thousands of extents. Then, when doing
I/O to an extent, temporarily take a crypto_skcipher from the cache, derive the
extent's key using fscrypt_hkdf_expand(), and set it in the crypto_skcipher
using crypto_skcipher_setkey().
That way, during I/O only fscrypt_hkdf_expand() and crypto_skcipher_setkey()
would be needed. Those are fairly safe to call during I/O, in contrast to
crypto_alloc_skcipher() which is really problematic to call during I/O.
Of course, it will still be somewhat expensive to derive and set a key. So it
might also make sense to maintain a map that maps (master key, extent nonce,
encryption mode) to the corresponding cached crypto_skcipher, if any, so that an
already-prepared one can be used when possible.
By the way, blk-crypto-fallback (block/blk-crypto-fallback.c) does something
similar, as it had to solve a similar problem. The way it was solved is to
require that blk_crypto_fallback_start_using_mode() be called to preallocate the
crypto_skcipher objects for a given encryption mode.
You actually could just use blk-crypto-fallback to do the en/decryption for you,
if you wanted to. I.e., you could use the blk-crypto API for en/decryption,
instead of going directly through crypto_skcipher. (Note that currently
blk-crypto is opt-in via the "inlinecrypt" mount option; it's not used by
default. But it doesn't *have* to be that way; it could just be always used.
It would be necessary to ensure that CONFIG_BLK_INLINE_ENCRYPTION_FALLBACK gets
selected.) That would save having to reimplement the caching of crypto_skcipher
objects. However, key derivation would still need to be done at the filesystem
level, so it probably would still make sense to cache derived keys.
- Eric
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [RFC PATCH 15/17] fscrypt: allow load/save of extent contexts
2023-01-02 21:47 ` Eric Biggers
@ 2023-01-02 22:31 ` Sweet Tea Dorminy
2023-01-02 22:51 ` Eric Biggers
0 siblings, 1 reply; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-02 22:31 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-fscrypt, paulcrowley, linux-btrfs, kernel-team
(in which I fail to reply-all the first time)
>
> When is the filesystem going to call fscrypt_load_extent_info()?
>
> My concern (which we've discussed, but probably I didn't explain clearly enough)
> is that the two "naive" solutions don't really work:
>
> Option 1: Set up during the I/O to the extent. I think this is not feasible
> because the full fscrypt_setup_encryption_info() is not safe to do doing I/O.
> For example, it allocates memory under GFP_KERNEL, and it uses
> crypto_alloc_skcipher() which can involve loading kernel modules.
>
memalloc_nofs_save()/memalloc_nofs_restore() could do the job of making
sure allocations use GFP_NOFS, I think? I guess those calls should be in
fscrypt_load_extent_info() instead of just in the doc...
>
> That leaves the option I suggested, which probably I didn't explain clearly
> enough: split up key setup so that part can be done when opening the file, and
> part can be done during I/O. Specifically, when opening the file, preallocate
> some number of crypto_skcipher objects. This would be limited to a fixed
> number, like 128, even if the file has thousands of extents. Then, when doing
> I/O to an extent, temporarily take a crypto_skcipher from the cache, derive the
> extent's key using fscrypt_hkdf_expand(), and set it in the crypto_skcipher
> using crypto_skcipher_setkey().
I didn't elaborate why it wasn't here and should have. With just this
patchset, I thought a file only ever has extents with the same
encryption mode, since an inode can't change encryption mode/key past
creation at present . So loading the parent dir's fscrypt_info should be
enough to ensure the module is loaded for the mode of all the extents. I
suppose I'd need to ensure that reflinks also only reflink extents
sharing the same encryption mode.
In the future patchset which allows changing the key being used for new
extents (naming that is hard), I had envisioned requiring the filesystem
to provide a list of enc modes used by an inode when opening, and then
fscrypt_file_open() could make sure all the necessary modules are loaded
for those modes.
>
> That way, during I/O only fscrypt_hkdf_expand() and crypto_skcipher_setkey()
> would be needed. Those are fairly safe to call during I/O, in contrast to
> crypto_alloc_skcipher() which is really problematic to call during I/O.
Could we use a mempool of skciphers for all of fscrypt, or should it
only be for extents and be on a per-file basis?
>
> Of course, it will still be somewhat expensive to derive and set a key. So it
> might also make sense to maintain a map that maps (master key, extent nonce,
> encryption mode) to the corresponding cached crypto_skcipher, if any, so that an
> already-prepared one can be used when possible.
Same question: just for extent infos, or can this be generalized to all
infos?
>
> By the way, blk-crypto-fallback (block/blk-crypto-fallback.c) does something
> similar, as it had to solve a similar problem. The way it was solved is to
> require that blk_crypto_fallback_start_using_mode() be called to preallocate the
> crypto_skcipher objects for a given encryption mode.
>
> You actually could just use blk-crypto-fallback to do the en/decryption for you,
> if you wanted to. I.e., you could use the blk-crypto API for en/decryption,
> instead of going directly through crypto_skcipher. (Note that currently
> blk-crypto is opt-in via the "inlinecrypt" mount option; it's not used by
> default. But it doesn't *have* to be that way; it could just be always used.
> It would be necessary to ensure that CONFIG_BLK_INLINE_ENCRYPTION_FALLBACK gets
> selected.) That would save having to reimplement the caching of crypto_skcipher
> objects. However, key derivation would still need to be done at the filesystem
> level, so it probably would still make sense to cache derived keys.
>
> - Eric
Interesting. I will have to study that more, thanks for the pointer.
Thank you!
Sweet Tea
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 15/17] fscrypt: allow load/save of extent contexts
2023-01-02 22:31 ` Sweet Tea Dorminy
@ 2023-01-02 22:51 ` Eric Biggers
2023-01-03 0:33 ` Sweet Tea Dorminy
0 siblings, 1 reply; 28+ messages in thread
From: Eric Biggers @ 2023-01-02 22:51 UTC (permalink / raw)
To: Sweet Tea Dorminy; +Cc: linux-fscrypt, paulcrowley, linux-btrfs, kernel-team
On Mon, Jan 02, 2023 at 05:31:02PM -0500, Sweet Tea Dorminy wrote:
> (in which I fail to reply-all the first time)
>
> >
> > When is the filesystem going to call fscrypt_load_extent_info()?
> >
> > My concern (which we've discussed, but probably I didn't explain clearly enough)
> > is that the two "naive" solutions don't really work:
> >
> > Option 1: Set up during the I/O to the extent. I think this is not feasible
> > because the full fscrypt_setup_encryption_info() is not safe to do doing I/O.
> > For example, it allocates memory under GFP_KERNEL, and it uses
> > crypto_alloc_skcipher() which can involve loading kernel modules.
> >
> memalloc_nofs_save()/memalloc_nofs_restore() could do the job of making sure
> allocations use GFP_NOFS, I think? I guess those calls should be in
> fscrypt_load_extent_info() instead of just in the doc...
> >
> > That leaves the option I suggested, which probably I didn't explain clearly
> > enough: split up key setup so that part can be done when opening the file, and
> > part can be done during I/O. Specifically, when opening the file, preallocate
> > some number of crypto_skcipher objects. This would be limited to a fixed
> > number, like 128, even if the file has thousands of extents. Then, when doing
> > I/O to an extent, temporarily take a crypto_skcipher from the cache, derive the
> > extent's key using fscrypt_hkdf_expand(), and set it in the crypto_skcipher
> > using crypto_skcipher_setkey().
>
> I didn't elaborate why it wasn't here and should have. With just this
> patchset, I thought a file only ever has extents with the same encryption
> mode, since an inode can't change encryption mode/key past creation at
> present . So loading the parent dir's fscrypt_info should be enough to
> ensure the module is loaded for the mode of all the extents. I suppose I'd
> need to ensure that reflinks also only reflink extents sharing the same
> encryption mode.
>
> In the future patchset which allows changing the key being used for new
> extents (naming that is hard), I had envisioned requiring the filesystem to
> provide a list of enc modes used by an inode when opening, and then
> fscrypt_file_open() could make sure all the necessary modules are loaded for
> those modes.
"Loading the parent dir's fscrypt_info" isn't relevant here, since the filenames
encryption mode can be different from the contents encryption mode. It's also
possible for an encrypted file to be located in an unencrypted directory. Maybe
you meant to be talking about the file itself being opened?
Anyway, crypto_alloc_skcipher() takes a lock (crypto_alg_sem) under which memory
is allocated with GFP_KERNEL. So neither preloading kernel modules nor
memalloc_nofs_save() helps for it; it's still not GFP_NOFS-safe.
I don't think we should allow files to have extents with different encryption
modes. Encryption policies will still be a property of files.
> > That way, during I/O only fscrypt_hkdf_expand() and crypto_skcipher_setkey()
> > would be needed. Those are fairly safe to call during I/O, in contrast to
> > crypto_alloc_skcipher() which is really problematic to call during I/O.
>
> Could we use a mempool of skciphers for all of fscrypt, or should it only be
> for extents and be on a per-file basis?
It probably should be global, or at least per-master-key, as it would be a
massive overhead to allocate (e.g.) 128 crypto_skcipher objects for every file.
blk-crypto-fallback uses a global cache.
It does introduce a bottleneck and memory that can't be reclaimed, though. I'd
appreciate any thoughts about other solutions. Maybe the number of objects in
the cache could be scaled up and down as the number of in-core inodes changes.
> > Of course, it will still be somewhat expensive to derive and set a key. So it
> > might also make sense to maintain a map that maps (master key, extent nonce,
> > encryption mode) to the corresponding cached crypto_skcipher, if any, so that an
> > already-prepared one can be used when possible.
> Same question: just for extent infos, or can this be generalized to all
> infos?
I think that we should definitely still cache the per-file key in
inode::i_crypt_info and not in some global cache.
- Eric
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 15/17] fscrypt: allow load/save of extent contexts
2023-01-02 22:51 ` Eric Biggers
@ 2023-01-03 0:33 ` Sweet Tea Dorminy
2023-01-03 0:47 ` Eric Biggers
0 siblings, 1 reply; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-03 0:33 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-fscrypt, paulcrowley, linux-btrfs, kernel-team
>
> Anyway, crypto_alloc_skcipher() takes a lock (crypto_alg_sem) under which memory
> is allocated with GFP_KERNEL. So neither preloading kernel modules nor
> memalloc_nofs_save() helps for it; it's still not GFP_NOFS-safe.
I'm still confused. My understanding is that memalloc_nofs_save() means
all allocations on that thread until memalloc_nofs_restore() is called
effectively gets GFP_NOFS appended to the allocation flags. So since
crypto_alloc_skcipher()'s allocation appears to be on the same thread as
we'd be calling memalloc_nofs_save/restore(), it would presumably get
allocated as though it had flags GFP_KERNEL | GFP_NOFS, even though the
call is kzalloc(..., GFP_KERNEL, ...).
I don't understand how the lock would make a difference. Can you elaborate?
Sorry for my confusion...
Sweet Tea
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 15/17] fscrypt: allow load/save of extent contexts
2023-01-03 0:33 ` Sweet Tea Dorminy
@ 2023-01-03 0:47 ` Eric Biggers
2023-01-03 1:23 ` Sweet Tea Dorminy
0 siblings, 1 reply; 28+ messages in thread
From: Eric Biggers @ 2023-01-03 0:47 UTC (permalink / raw)
To: Sweet Tea Dorminy; +Cc: linux-fscrypt, paulcrowley, linux-btrfs, kernel-team
On Mon, Jan 02, 2023 at 07:33:15PM -0500, Sweet Tea Dorminy wrote:
>
> >
> > Anyway, crypto_alloc_skcipher() takes a lock (crypto_alg_sem) under which memory
> > is allocated with GFP_KERNEL. So neither preloading kernel modules nor
> > memalloc_nofs_save() helps for it; it's still not GFP_NOFS-safe.
>
> I'm still confused. My understanding is that memalloc_nofs_save() means all
> allocations on that thread until memalloc_nofs_restore() is called
> effectively gets GFP_NOFS appended to the allocation flags. So since
> crypto_alloc_skcipher()'s allocation appears to be on the same thread as
> we'd be calling memalloc_nofs_save/restore(), it would presumably get
> allocated as though it had flags GFP_KERNEL | GFP_NOFS, even though the call
> is kzalloc(..., GFP_KERNEL, ...).
>
> I don't understand how the lock would make a difference. Can you elaborate?
>
> Sorry for my confusion...
Other tasks (using the crypto API for another purpose, perhaps totally unrelated
to fs/crypto/) can take crypto_alg_sem without taking the same precaution. So,
when your task that is running in fs-reclaim context and has used
memalloc_nofs_save() tries to take the same lock, it might be that the lock is
already held by another thread that is waiting for fs-reclaim to complete in
order to satisfy a GFP_KERNEL allocation.
That's a deadlock.
Locks are only GFP_NOFS-safe when everyone agrees to use them that way.
- Eric
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 15/17] fscrypt: allow load/save of extent contexts
2023-01-03 0:47 ` Eric Biggers
@ 2023-01-03 1:23 ` Sweet Tea Dorminy
0 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-03 1:23 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-fscrypt, paulcrowley, linux-btrfs, kernel-team
On 1/2/23 19:47, Eric Biggers wrote:
> On Mon, Jan 02, 2023 at 07:33:15PM -0500, Sweet Tea Dorminy wrote:
>>
>>>
>>> Anyway, crypto_alloc_skcipher() takes a lock (crypto_alg_sem) under which memory
>>> is allocated with GFP_KERNEL. So neither preloading kernel modules nor
>>> memalloc_nofs_save() helps for it; it's still not GFP_NOFS-safe.
>>
>> I'm still confused. My understanding is that memalloc_nofs_save() means all
>> allocations on that thread until memalloc_nofs_restore() is called
>> effectively gets GFP_NOFS appended to the allocation flags. So since
>> crypto_alloc_skcipher()'s allocation appears to be on the same thread as
>> we'd be calling memalloc_nofs_save/restore(), it would presumably get
>> allocated as though it had flags GFP_KERNEL | GFP_NOFS, even though the call
>> is kzalloc(..., GFP_KERNEL, ...).
>>
>> I don't understand how the lock would make a difference. Can you elaborate?
>>
>> Sorry for my confusion...
>
> Other tasks (using the crypto API for another purpose, perhaps totally unrelated
> to fs/crypto/) can take crypto_alg_sem without taking the same precaution. So,
> when your task that is running in fs-reclaim context and has used
> memalloc_nofs_save() tries to take the same lock, it might be that the lock is
> already held by another thread that is waiting for fs-reclaim to complete in
> order to satisfy a GFP_KERNEL allocation.
>
> That's a deadlock.
>
> Locks are only GFP_NOFS-safe when everyone agrees to use them that way.
>
> - Eric
Ah that is definitely what I was missing, I've never had to think about
that interaction. Thank you for explaining!
^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH 16/17] fscrypt: disable inline encryption for extent-based encryption
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (14 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 15/17] fscrypt: allow load/save of extent contexts Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-01-01 5:06 ` [RFC PATCH 17/17] fscrypt: update documentation to mention per-extent keys Sweet Tea Dorminy
2023-02-22 11:52 ` [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Neal Gompa
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
I haven't worked out how to do inline-suitable fscrypt_infos for
extent-based encryption, so disable the combo for now.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/crypto/inline_crypt.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 808c8712ebe7..43fb0d933ff4 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -113,6 +113,10 @@ int fscrypt_select_encryption_impl(struct fscrypt_info *ci)
if (!(sb->s_flags & SB_INLINECRYPT))
return 0;
+ /* The filesystem must not use per-extent infos */
+ if (fscrypt_uses_extent_encryption(inode))
+ return 0;
+
/*
* When a page contains multiple logically contiguous filesystem blocks,
* some filesystem code only calls fscrypt_mergeable_bio() for the first
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH 17/17] fscrypt: update documentation to mention per-extent keys.
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (15 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 16/17] fscrypt: disable inline encryption for extent-based encryption Sweet Tea Dorminy
@ 2023-01-01 5:06 ` Sweet Tea Dorminy
2023-02-22 11:52 ` [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Neal Gompa
17 siblings, 0 replies; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-01-01 5:06 UTC (permalink / raw)
To: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
Cc: Sweet Tea Dorminy
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
Documentation/filesystems/fscrypt.rst | 38 ++++++++++++++++++++++-----
1 file changed, 32 insertions(+), 6 deletions(-)
diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 5ba5817c17c2..0ff8abf89422 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -31,7 +31,7 @@ However, except for filenames, fscrypt does not encrypt filesystem
metadata.
Unlike eCryptfs, which is a stacked filesystem, fscrypt is integrated
-directly into supported filesystems --- currently ext4, F2FS, and
+directly into supported filesystems --- currently btrfs, ext4, F2FS, and
UBIFS. This allows encrypted files to be read and written without
caching both the decrypted and encrypted pages in the pagecache,
thereby nearly halving the memory used and bringing it in line with
@@ -280,6 +280,11 @@ included in the IV. Moreover:
key derived using the KDF. Users may use the same master key for
other v2 encryption policies.
+For filesystems with extent-based content encryption (e.g. btrfs),
+this is the only choice. Data shared among multiple inodes must share
+the exact same key, therefore necessitating inodes using the same key
+for contents encryption.
+
IV_INO_LBLK_64 policies
-----------------------
@@ -374,12 +379,13 @@ to individual filesystems. However, authenticated encryption (AE)
modes are not currently supported because of the difficulty of dealing
with ciphertext expansion.
-Contents encryption
--------------------
+Inode-based contents encryption
+-------------------------------
-For file contents, each filesystem block is encrypted independently.
-Starting from Linux kernel 5.5, encryption of filesystems with block
-size less than system's page size is supported.
+Most filesystems use the previously discussed per-file keys. For these
+filesystems, for file contents, each filesystem block is encrypted
+independently. Starting from Linux kernel 5.5, encryption of filesystems
+with block size less than system's page size is supported.
Each block's IV is set to the logical block number within the file as
a little endian number, except that:
@@ -403,6 +409,26 @@ Note that because file logical block numbers are included in the IVs,
filesystems must enforce that blocks are never shifted around within
encrypted files, e.g. via "collapse range" or "insert range".
+Extent-based contents encryption
+--------------------------------
+
+For certain filesystems (currently only btrfs), data is encrypted on a
+per-extent basis, for whatever the filesystem's notion of an extent is. The
+scheme is exactly as with inode-based contents encryption, except that the
+'inode number' for an extent is requested from the filesystem instead of from
+the file's inode, and the 'logical block number' refers to an offset within the
+extent.
+
+Because the encryption material is per-extent instead of per-inode, as long
+as the extent's encryption context does not change, the filesystem may shift
+around the position of the extent, and may have multiple files referring to
+the same encrypted extent.
+
+Not all extents within a file are decrypted simultaneously, so it is possible
+for a file read to fail partway through the file if it crosses into an extent
+whose key is unavailable. However, all writes will succeed, unless the key is
+removed mid-write.
+
Filenames encryption
--------------------
--
2.38.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [RFC PATCH 00/17] fscrypt: add per-extent encryption keys
2023-01-01 5:06 [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Sweet Tea Dorminy
` (16 preceding siblings ...)
2023-01-01 5:06 ` [RFC PATCH 17/17] fscrypt: update documentation to mention per-extent keys Sweet Tea Dorminy
@ 2023-02-22 11:52 ` Neal Gompa
2023-02-22 14:13 ` Sweet Tea Dorminy
17 siblings, 1 reply; 28+ messages in thread
From: Neal Gompa @ 2023-02-22 11:52 UTC (permalink / raw)
To: Sweet Tea Dorminy
Cc: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
On Sun, Jan 1, 2023 at 12:08 AM Sweet Tea Dorminy
<sweettea-kernel@dorminy.me> wrote:
>
> Last month, after a discussion of using fscrypt in btrfs, several
> potential areas for expansion of fscrypt functionality were identified:
> specifically, per-extent keys, authenticated encryption, and 'rekeying'
> a directory tree [1]. These additions will permit btrfs to have better
> cryptographic characteristics than previous attempts at expanding btrfs
> to use fscrypt.
>
> This attempts to implement the first of these, per-extent keys (in
> analogy to the current per-inode keys) in fscrypt. For a filesystem
> using per-extent keys, the idea is that each regular file inode is
> linked to its parent directory's fscrypt_info, while each extent in
> the filesystem -- opaque to fscrypt -- stores a fscrypt_info providing
> the key for the data in that extent. For non-regular files, the inode
> has its own fscrypt_info as in current ("inode-based") fscrypt.
>
> IV generation methods using logical block numbers use the logical block
> number within the extent, and for IV generation methods using inode
> numbers, such filesystems may optionally implement a method providing an
> equivalent on a per-extent basis.
>
> Known limitations: change 12 ("fscrypt: notify per-extent infos if
> master key vanishes") does not sufficiently argue that there cannot be a
> race between freeing a master key and using it for some pending extent IO.
> Change 16 ("fscrypt: disable inline encryption for extent-based
> encryption") merely disables inline encryption, when it should implement
> generating appropriate inline encryption info for extent infos.
>
> This has not been thoroughly tested against a btrfs implementation of
> the interfaces -- I've thrown out everything here and tried something
> new several times, and while I think this interface is a decent one, I
> would like to get input on it in parallel with finishing the btrfs side
> of this part, and the other elements of the design mentioned in [1]
>
> [1] https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing
>
> *** BLURB HERE ***
>
> Sweet Tea Dorminy (17):
> fscrypt: factor accessing inode->i_crypt_info
> fscrypt: separate getting info for a specific block
> fscrypt: adjust effective lblks based on extents
> fscrypt: factor out fscrypt_set_inode_info()
> fscrypt: use parent dir's info for extent-based encryption.
> fscrypt: add a super_block pointer to fscrypt_info
> fscrypt: update comments about inodes to include extents
> fscrypt: rename mk->mk_decrypted_inodes*
> fscrypt: make fscrypt_setup_encryption_info generic for extents
> fscrypt: let fscrypt_infos be owned by an extent
> fscrypt: update all the *per_file_* function names
> fscrypt: notify per-extent infos if master key vanishes
> fscrypt: use an optional ino equivalent for per-extent infos
> fscrypt: add creation/usage/freeing of per-extent infos
> fscrypt: allow load/save of extent contexts
> fscrypt: disable inline encryption for extent-based encryption
> fscrypt: update documentation to mention per-extent keys.
>
> Documentation/filesystems/fscrypt.rst | 38 +++-
> fs/crypto/crypto.c | 17 +-
> fs/crypto/fname.c | 9 +-
> fs/crypto/fscrypt_private.h | 174 +++++++++++++----
> fs/crypto/hooks.c | 2 +-
> fs/crypto/inline_crypt.c | 42 ++--
> fs/crypto/keyring.c | 67 ++++---
> fs/crypto/keysetup.c | 263 ++++++++++++++++++++------
> fs/crypto/keysetup_v1.c | 24 +--
> fs/crypto/policy.c | 28 ++-
> include/linux/fscrypt.h | 76 ++++++++
> 11 files changed, 580 insertions(+), 160 deletions(-)
>
>
> base-commit: b7af0635c87ff78d6bd523298ab7471f9ffd3ce5
> --
> 2.38.1
>
I'm surprised that this submission generated no discussion across a
timeframe of over a month. Is this normal for RFC patch sets?
--
真実はいつも一つ！/ Always, there's only one truth!
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [RFC PATCH 00/17] fscrypt: add per-extent encryption keys
2023-02-22 11:52 ` [RFC PATCH 00/17] fscrypt: add per-extent encryption keys Neal Gompa
@ 2023-02-22 14:13 ` Sweet Tea Dorminy
2023-02-22 20:53 ` Eric Biggers
0 siblings, 1 reply; 28+ messages in thread
From: Sweet Tea Dorminy @ 2023-02-22 14:13 UTC (permalink / raw)
To: Neal Gompa; +Cc: linux-fscrypt, ebiggers, paulcrowley, linux-btrfs, kernel-team
On 2/22/23 06:52, Neal Gompa wrote:
> On Sun, Jan 1, 2023 at 12:08 AM Sweet Tea Dorminy
> <sweettea-kernel@dorminy.me> wrote:
>>
>> Last month, after a discussion of using fscrypt in btrfs, several
>> potential areas for expansion of fscrypt functionality were identified:
>> specifically, per-extent keys, authenticated encryption, and 'rekeying'
>> a directory tree [1]. These additions will permit btrfs to have better
>> cryptographic characteristics than previous attempts at expanding btrfs
>> to use fscrypt.
>>
>> This attempts to implement the first of these, per-extent keys (in
>> analogy to the current per-inode keys) in fscrypt. For a filesystem
>> using per-extent keys, the idea is that each regular file inode is
>> linked to its parent directory's fscrypt_info, while each extent in
>> the filesystem -- opaque to fscrypt -- stores a fscrypt_info providing
>> the key for the data in that extent. For non-regular files, the inode
>> has its own fscrypt_info as in current ("inode-based") fscrypt.
>>
>> IV generation methods using logical block numbers use the logical block
>> number within the extent, and for IV generation methods using inode
>> numbers, such filesystems may optionally implement a method providing an
>> equivalent on a per-extent basis.
>>
>> Known limitations: change 12 ("fscrypt: notify per-extent infos if
>> master key vanishes") does not sufficiently argue that there cannot be a
>> race between freeing a master key and using it for some pending extent IO.
>> Change 16 ("fscrypt: disable inline encryption for extent-based
>> encryption") merely disables inline encryption, when it should implement
>> generating appropriate inline encryption info for extent infos.
>>
>> This has not been thoroughly tested against a btrfs implementation of
>> the interfaces -- I've thrown out everything here and tried something
>> new several times, and while I think this interface is a decent one, I
>> would like to get input on it in parallel with finishing the btrfs side
>> of this part, and the other elements of the design mentioned in [1]
>>
>> [1] https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing
>>
>> *** BLURB HERE ***
>>
>> Sweet Tea Dorminy (17):
>> fscrypt: factor accessing inode->i_crypt_info
>> fscrypt: separate getting info for a specific block
>> fscrypt: adjust effective lblks based on extents
>> fscrypt: factor out fscrypt_set_inode_info()
>> fscrypt: use parent dir's info for extent-based encryption.
>> fscrypt: add a super_block pointer to fscrypt_info
>> fscrypt: update comments about inodes to include extents
>> fscrypt: rename mk->mk_decrypted_inodes*
>> fscrypt: make fscrypt_setup_encryption_info generic for extents
>> fscrypt: let fscrypt_infos be owned by an extent
>> fscrypt: update all the *per_file_* function names
>> fscrypt: notify per-extent infos if master key vanishes
>> fscrypt: use an optional ino equivalent for per-extent infos
>> fscrypt: add creation/usage/freeing of per-extent infos
>> fscrypt: allow load/save of extent contexts
>> fscrypt: disable inline encryption for extent-based encryption
>> fscrypt: update documentation to mention per-extent keys.
>>
>> Documentation/filesystems/fscrypt.rst | 38 +++-
>> fs/crypto/crypto.c | 17 +-
>> fs/crypto/fname.c | 9 +-
>> fs/crypto/fscrypt_private.h | 174 +++++++++++++----
>> fs/crypto/hooks.c | 2 +-
>> fs/crypto/inline_crypt.c | 42 ++--
>> fs/crypto/keyring.c | 67 ++++---
>> fs/crypto/keysetup.c | 263 ++++++++++++++++++++------
>> fs/crypto/keysetup_v1.c | 24 +--
>> fs/crypto/policy.c | 28 ++-
>> include/linux/fscrypt.h | 76 ++++++++
>> 11 files changed, 580 insertions(+), 160 deletions(-)
>>
>>
>> base-commit: b7af0635c87ff78d6bd523298ab7471f9ffd3ce5
>> --
>> 2.38.1
>>
>
> I'm surprised that this submission generated no discussion across a
> timeframe of over a month. Is this normal for RFC patch sets?
Eric pointed out some issues with patches 1 and 15 on 1/2. I've been on
parental leave and have been busier with new little one than expected,
and haven't sent out a new version yet. But I'm back to work in a week
and this is my primary priority.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [RFC PATCH 00/17] fscrypt: add per-extent encryption keys
2023-02-22 14:13 ` Sweet Tea Dorminy
@ 2023-02-22 20:53 ` Eric Biggers
0 siblings, 0 replies; 28+ messages in thread
From: Eric Biggers @ 2023-02-22 20:53 UTC (permalink / raw)
To: Sweet Tea Dorminy
Cc: Neal Gompa, linux-fscrypt, paulcrowley, linux-btrfs, kernel-team
On Wed, Feb 22, 2023 at 09:13:47AM -0500, Sweet Tea Dorminy wrote:
> >
> > I'm surprised that this submission generated no discussion across a
> > timeframe of over a month. Is this normal for RFC patch sets?
>
> Eric pointed out some issues with patches 1 and 15 on 1/2. I've been on
> parental leave and have been busier with new little one than expected, and
> haven't sent out a new version yet. But I'm back to work in a week and this
> is my primary priority.
IMO, most of the patchset will change as a result of addressing my feedback. So
I haven't had much motivation to review the current version in detail. I'm
looking forward to the next version; I'm glad you're still working on it!
- Eric
^ permalink raw reply [flat|nested] 28+ messages in thread