[PATCH v2 00/14] fscrypt: add extent encryption

[PATCH v2 00/14] fscrypt: add extent encryption

[Sweet Tea Dorminy]

This changeset adds extent-based data encryption to fscrypt.
Some filesystems need to encrypt data based on extents, rather than on
inodes, due to features incompatible with inode-based encryption. For
instance, btrfs can have multiple inodes referencing a single block of
data, and moves logical data blocks to different physical locations on
disk in the background. 

As per discussion last year in [1] and later in [2], we would like to
allow the use of fscrypt with btrfs, with authenticated encryption. This
is the first step of that work, adding extent-based encryption to
fscrypt; authenticated encryption is the next step. Extent-based
encryption should be usable by other filesystems which wish to support
snapshotting or background data rearrangement also, but btrfs is the
first user. 

This changeset requires extent encryption to use inlinecrypt, as
discussed previously. There are two questionable parts: the
forget_extent_info hook is not yet in use by btrfs, as I haven't yet
written a test exercising a race where it would be relevant; and saving
the session key credentials just to enable v1 session-based policies is
perhaps less good than 

This applies atop [3], which itself is based on kdave/misc-next. It
passes encryption fstests with suitable changes to btrfs-progs and tests.

[1] https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing
[2] https://lore.kernel.org/linux-fscrypt/80496cfe-161d-fb0d-8230-93818b966b1b@dorminy.me/T/#t
[3]
https://lore.kernel.org/linux-fscrypt/cover.1688927423.git.sweettea-kernel@dorminy.me/

Changelog:
v2:
- Change to soft-delete keys in use by inodes using extent encryption at
  key removal time (new changes 9 and 10), as per feedback.
- Add a missing error check in fscrypt_mergeable_bio (change 3)
v1:
- https://lore.kernel.org/linux-fscrypt/248eac32-96cc-eb2e-85da-422a8d75a376@dorminy.me/T/#t

Sweet Tea Dorminy (14):
  fscrypt: factor helper for locking master key
  fscrypt: factor getting info for a specific block
  fscrypt: adjust effective lblks based on extents
  fscrypt: add a super_block pointer to fscrypt_info
  fscrypt: setup leaf inodes for extent encryption
  fscrypt: allow infos to be owned by extents
  fscrypt: notify per-extent infos if master key vanishes
  fscrypt: use an optional ino equivalent for per-extent infos
  fscrypt: move function call warning of busy inodes
  fscrypt: revamp key removal for extent encryption
  fscrypt: add creation/usage/freeing of per-extent infos
  fscrypt: allow load/save of extent contexts
  fscrypt: save session key credentials for extent infos
  fscrypt: update documentation for per-extent keys

 Documentation/filesystems/fscrypt.rst |  43 +++-
 fs/crypto/crypto.c                    |   6 +-
 fs/crypto/fscrypt_private.h           | 128 +++++++++++-
 fs/crypto/inline_crypt.c              |  28 ++-
 fs/crypto/keyring.c                   |  79 +++++---
 fs/crypto/keysetup.c                  | 274 ++++++++++++++++++++++----
 fs/crypto/keysetup_v1.c               |   7 +-
 fs/crypto/policy.c                    |  20 ++
 include/linux/fscrypt.h               |  65 ++++++
 9 files changed, 562 insertions(+), 88 deletions(-)


base-commit: e0144e6be2ec62c8d874076a6292a8d83b00ee32
prerequisite-patch-id: ab342a72cf967dadfb8bec1320c5906fd3c6800f
prerequisite-patch-id: ced2a9dab36539f55c14cd74a28950087c475ff2
prerequisite-patch-id: d4f1a64c994c2fa0d2d4cab83f9ddff52f0622e9
prerequisite-patch-id: 1af0fc98277159b31c26bc5751663efc0d322d75
prerequisite-patch-id: 3b21b62208587486cf9b31618f7c3bc875362f1a
prerequisite-patch-id: c43d693f5b7c498a876d9ffcfc49c11a8ca93d80
prerequisite-patch-id: f120bde1cf47fbef1d9f8fd09cdcccc1408c3ff4
prerequisite-patch-id: c6a1f087d4a67b928b9c6af04e00310bfa74ace1
-- 
2.40.1

[PATCH v2 01/14] fscrypt: factor helper for locking master key

[Sweet Tea Dorminy]

When we are making extent infos, we'll need to lock the master key in
more places, so go on and factor out a helper.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/keysetup.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index a19650f954e2..c3d3da5da449 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -106,7 +106,18 @@ select_encryption_mode(const union fscrypt_policy *policy,
 	return ERR_PTR(-EINVAL);
 }
 
-/* Create a symmetric cipher object for the given encryption mode and key */
+static int lock_master_key(struct fscrypt_master_key *mk)
+{
+	down_read(&mk->mk_sem);
+
+	/* Has the secret been removed (via FS_IOC_REMOVE_ENCRYPTION_KEY)? */
+	if (!is_master_key_secret_present(&mk->mk_secret))
+		return -ENOKEY;
+
+	return 0;
+}
+
+/* Create a symmetric cipher object for the given encryption mode */
 static struct crypto_skcipher *
 fscrypt_allocate_skcipher(struct fscrypt_mode *mode, const u8 *raw_key,
 			  const struct inode *inode)
@@ -556,13 +567,10 @@ static int find_and_lock_master_key(const struct fscrypt_info *ci,
 		*mk_ret = NULL;
 		return 0;
 	}
-	down_read(&mk->mk_sem);
 
-	/* Has the secret been removed (via FS_IOC_REMOVE_ENCRYPTION_KEY)? */
-	if (!is_master_key_secret_present(&mk->mk_secret)) {
-		err = -ENOKEY;
+	err = lock_master_key(mk);
+	if (err)
 		goto out_release_key;
-	}
 
 	if (!fscrypt_valid_master_key_size(mk, ci)) {
 		err = -ENOKEY;
-- 
2.40.1

[PATCH v2 02/14] fscrypt: factor getting info for a specific block

[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 grabbing
inode->i_crypt_info will be insufficient; the caller must specifically
request the inode info or the info for a specific block.

Add fscrypt_get_lblk_info() to get info for a specific block, and update
all relevant callsites.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/crypto.c          |  3 ++-
 fs/crypto/fscrypt_private.h | 29 +++++++++++++++++++++++++++++
 fs/crypto/inline_crypt.c    | 10 ++++++----
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 9f3bda18c797..1b7e375b1c6b 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 = inode->i_crypt_info;
+	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/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index e726a1fb9f7e..4d1e67bc1e62 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -277,6 +277,35 @@ typedef enum {
 	FS_ENCRYPT,
 } fscrypt_direction_t;
 
+/**
+ * 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 inode->i_crypt_info;
+}
+
+
 /* crypto.c */
 extern struct kmem_cache *fscrypt_info_cachep;
 int fscrypt_initialize(struct super_block *sb);
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 2063f7941ce6..885a2ec3d711 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -270,7 +270,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_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);
@@ -349,21 +349,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 != inode->i_crypt_info->ci_enc_key->blk_key)
+	if (bc->bc_key != ci->ci_enc_key->blk_key)
 		return false;
 
-	fscrypt_generate_dun(inode->i_crypt_info, 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);
@@ -465,7 +467,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_lblk_info(inode, lblk, NULL, NULL);
 	if (!(fscrypt_policy_flags(&ci->ci_policy) &
 	      FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32))
 		return nr_blocks;
-- 
2.40.1

[PATCH v2 03/14] fscrypt: adjust effective lblks based on extents

[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       |  3 ++-
 fs/crypto/inline_crypt.c | 24 +++++++++++++++++-------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 1b7e375b1c6b..d75f1b3f5795 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;
+	u64 ci_offset = 0;
 	struct fscrypt_info *ci =
-		fscrypt_get_lblk_info(inode, lblk_num, NULL, NULL);
+		fscrypt_get_lblk_info(inode, lblk_num, &ci_offset, NULL);
 	struct crypto_skcipher *tfm = ci->ci_enc_key->tfm;
 	int res = 0;
 
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 885a2ec3d711..b3e7a5291d22 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -267,12 +267,15 @@ 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);
+	if (!ci)
+		return;
 
-	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);
@@ -350,22 +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;
+	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
 	 * merging requests where the keys are the same but the pointers differ.
 	 */
-	if (bc->bc_key != ci->ci_enc_key->blk_key)
+	if (!ci || 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);
@@ -460,6 +464,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 = 0;
+	u64 extent_len = 0;
 
 	if (!fscrypt_inode_uses_inline_crypto(inode))
 		return nr_blocks;
@@ -467,14 +473,18 @@ 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.40.1

[PATCH v2 04/14] fscrypt: add a super_block pointer to fscrypt_info

[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    |  4 ++--
 fs/crypto/keysetup.c        | 10 +++++-----
 fs/crypto/keysetup_v1.c     |  6 +++---
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 4d1e67bc1e62..c04454c289fd 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -241,6 +241,9 @@ struct fscrypt_info {
 	/* Back-pointer to the inode */
 	struct inode *ci_inode;
 
+	/* The superblock of the filesystem to which this 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 b3e7a5291d22..b8b4e59000d7 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;
 
@@ -155,7 +155,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 c3d3da5da449..c5f68cf65a6f 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -250,8 +250,7 @@ static int setup_new_mode_prepared_key(struct fscrypt_master_key *mk,
 				       struct fscrypt_prepared_key *prep_key,
 				       const struct fscrypt_info *ci)
 {
-	const struct inode *inode = ci->ci_inode;
-	const struct super_block *sb = inode->i_sb;
+	const struct super_block *sb = ci->ci_sb;
 	unsigned int policy_flags = fscrypt_policy_flags(&ci->ci_policy);
 	struct fscrypt_mode *mode = ci->ci_mode;
 	const u8 mode_num = mode - fscrypt_modes;
@@ -525,7 +524,7 @@ static bool fscrypt_valid_master_key_size(const struct fscrypt_master_key *mk,
 static int find_and_lock_master_key(const struct fscrypt_info *ci,
 				    struct fscrypt_master_key **mk_ret)
 {
-	struct super_block *sb = ci->ci_inode->i_sb;
+	struct super_block *sb = ci->ci_sb;
 	struct fscrypt_key_specifier mk_spec;
 	struct fscrypt_master_key *mk;
 	int err;
@@ -599,7 +598,7 @@ static void put_crypt_info(struct fscrypt_info *ci)
 		if (type == FSCRYPT_KEY_DIRECT_V1)
 			fscrypt_put_direct_key(ci->ci_enc_key);
 		if (type == FSCRYPT_KEY_PER_INFO) {
-			fscrypt_destroy_prepared_key(ci->ci_inode->i_sb,
+			fscrypt_destroy_prepared_key(ci->ci_sb,
 						     ci->ci_enc_key);
 			kfree_sensitive(ci->ci_enc_key);
 		}
@@ -616,7 +615,7 @@ static void put_crypt_info(struct fscrypt_info *ci)
 		spin_lock(&mk->mk_decrypted_inodes_lock);
 		list_del(&ci->ci_master_key_link);
 		spin_unlock(&mk->mk_decrypted_inodes_lock);
-		fscrypt_put_master_key_activeref(ci->ci_inode->i_sb, mk);
+		fscrypt_put_master_key_activeref(ci->ci_sb, mk);
 	}
 	memzero_explicit(ci, sizeof(*ci));
 	kmem_cache_free(fscrypt_info_cachep, ci);
@@ -642,6 +641,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 1e785cedead0..41d317f08aeb 100644
--- a/fs/crypto/keysetup_v1.c
+++ b/fs/crypto/keysetup_v1.c
@@ -235,7 +235,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;
 	dk->dk_key.type = FSCRYPT_KEY_DIRECT_V1;
@@ -309,8 +309,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.40.1

[PATCH v2 05/14] fscrypt: setup leaf inodes for extent encryption

[Sweet Tea Dorminy]

For extent-based encryption, leaf/regular file inodes are special: it's
useful to set their i_crypt_info field so that it's easy to inherit
their encryption policy for a new extent, but they never need to do any
encyption themselves. Additionally, since encryption can only be set up
on a directory, not a single file, their encryption policy can always
duplicate their parent inode's policy.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/fscrypt_private.h | 17 +++++++++++++
 fs/crypto/keysetup.c        | 49 ++++++++++++++++++++++++++-----------
 2 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index c04454c289fd..260635e8b558 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -308,6 +308,23 @@ fscrypt_get_lblk_info(const struct inode *inode, u64 lblk, u64 *offset,
 	return inode->i_crypt_info;
 }
 
+/**
+ * fscrypt_uses_extent_encryption() -- whether an inode uses per-extent
+ *				       encryption
+ *
+ * @inode: the inode in question
+ *
+ * Return: true if the inode uses per-extent fscrypt_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 filesystems currently use per-extent infos
+	return false;
+}
 
 /* crypto.c */
 extern struct kmem_cache *fscrypt_info_cachep;
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index c5f68cf65a6f..7469b2d8ac87 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -747,27 +747,48 @@ fscrypt_setup_encryption_info(struct inode *inode,
 int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 {
 	int res;
-	union fscrypt_context ctx;
+	union fscrypt_context ctx = { 0 };
 	union fscrypt_policy policy;
 
 	if (fscrypt_has_encryption_key(inode))
 		return 0;
 
-	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
-	if (res < 0) {
-		if (res == -ERANGE && allow_unsupported)
-			return 0;
-		fscrypt_warn(inode, "Error %d getting encryption context", res);
-		return res;
-	}
+	if (fscrypt_uses_extent_encryption(inode)) {
+		/*
+		 * Nothing will be encrypted with this info, so we can borrow
+		 * the parent (dir) inode's policy and use a zero nonce.
+		 */
+		struct dentry *dentry = d_find_any_alias(inode);
+		struct dentry *parent_dentry = dget_parent(dentry);
+		struct inode *dir = parent_dentry->d_inode;
+		bool found = false;
 
-	res = fscrypt_policy_from_context(&policy, &ctx, res);
-	if (res) {
-		if (allow_unsupported)
+		if (dir->i_crypt_info) {
+			found = true;
+			policy = dir->i_crypt_info->ci_policy;
+			nonce = dir->i_crypt_info->ci_nonce;
+		}
+		dput(parent_dentry);
+		dput(dentry);
+		if (!found)
 			return 0;
-		fscrypt_warn(inode,
-			     "Unrecognized or corrupt encryption context");
-		return res;
+	} else {
+		res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
+		if (res < 0) {
+			if (res == -ERANGE && allow_unsupported)
+				return 0;
+			fscrypt_warn(inode, "Error %d getting encryption context", res);
+			return res;
+		}
+
+		res = fscrypt_policy_from_context(&policy, &ctx, res);
+		if (res) {
+			if (allow_unsupported)
+				return 0;
+			fscrypt_warn(inode,
+				     "Unrecognized or corrupt encryption context");
+			return res;
+		}
 	}
 
 	if (!fscrypt_supported_policy(&policy, inode)) {
-- 
2.40.1

[PATCH v2 06/14] fscrypt: allow infos to be owned by extents

[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        | 49 ++++++++++++++++++++++++++++---------
 2 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 260635e8b558..1674e66e72e3 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -241,6 +241,12 @@ struct fscrypt_info {
 	/* Back-pointer to the 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 info pertains */
 	struct super_block *ci_sb;
 
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 7469b2d8ac87..29565338d9c0 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -625,12 +625,17 @@ 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;
 	struct fscrypt_master_key *mk = NULL;
 	int res;
+	bool info_for_extent = !!info_ptr;
+
+	if (!info_ptr)
+		info_ptr = &inode->i_crypt_info;
 
 	res = fscrypt_initialize(inode->i_sb);
 	if (res)
@@ -640,7 +645,11 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	if (!crypt_info)
 		return -ENOMEM;
 
-	crypt_info->ci_inode = inode;
+	if (fscrypt_uses_extent_encryption(inode) && info_for_extent)
+		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);
@@ -656,6 +665,12 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	res = fscrypt_select_encryption_impl(crypt_info);
 	if (res)
 		goto out;
+	if (info_for_extent && !fscrypt_using_inline_encryption(crypt_info)) {
+		fscrypt_warn(inode,
+			     "extent encryption requires inlinecrypt mount option");
+		res = -EINVAL;
+		goto out;
+	}
 
 	res = find_and_lock_master_key(crypt_info, &mk);
 	if (res)
@@ -701,7 +716,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	 * 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) {
+	if (cmpxchg_release(info_ptr, 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.
@@ -735,7 +750,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
  *		       %false unless the operation being performed is needed in
  *		       order for files (or directories) to be deleted.
  *
- * Set up ->i_crypt_info, if it hasn't already been done.
+ * Set up inode->i_crypt_info, if it hasn't already been done.
  *
  * Note: unless ->i_crypt_info is already set, this isn't %GFP_NOFS-safe.  So
  * generally this shouldn't be called from within a filesystem transaction.
@@ -747,8 +762,9 @@ fscrypt_setup_encryption_info(struct inode *inode,
 int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 {
 	int res;
-	union fscrypt_context ctx = { 0 };
+	union fscrypt_context ctx;
 	union fscrypt_policy policy;
+	const u8 *nonce;
 
 	if (fscrypt_has_encryption_key(inode))
 		return 0;
@@ -756,7 +772,7 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 	if (fscrypt_uses_extent_encryption(inode)) {
 		/*
 		 * Nothing will be encrypted with this info, so we can borrow
-		 * the parent (dir) inode's policy and use a zero nonce.
+		 * the parent (dir) inode's policy and nonce.
 		 */
 		struct dentry *dentry = d_find_any_alias(inode);
 		struct dentry *parent_dentry = dget_parent(dentry);
@@ -789,6 +805,7 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 				     "Unrecognized or corrupt encryption context");
 			return res;
 		}
+		nonce = fscrypt_context_nonce(&ctx);
 	}
 
 	if (!fscrypt_supported_policy(&policy, inode)) {
@@ -797,10 +814,10 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 		return -EINVAL;
 	}
 
-	res = fscrypt_setup_encryption_info(inode, &policy,
-					    fscrypt_context_nonce(&ctx),
+	res = fscrypt_setup_encryption_info(inode, &policy, nonce,
 					    IS_CASEFOLDED(inode) &&
-					    S_ISDIR(inode->i_mode));
+					    S_ISDIR(inode->i_mode),
+					    NULL);
 
 	if (res == -ENOPKG && allow_unsupported) /* Algorithm unavailable? */
 		res = 0;
@@ -834,7 +851,8 @@ int fscrypt_prepare_new_inode(struct inode *dir, struct inode *inode,
 			      bool *encrypt_ret)
 {
 	const union fscrypt_policy *policy;
-	u8 nonce[FSCRYPT_FILE_NONCE_SIZE];
+	u8 nonce_bytes[FSCRYPT_FILE_NONCE_SIZE];
+	const u8 *nonce;
 
 	policy = fscrypt_policy_to_inherit(dir);
 	if (policy == NULL)
@@ -856,10 +874,17 @@ int fscrypt_prepare_new_inode(struct inode *dir, struct inode *inode,
 
 	*encrypt_ret = true;
 
-	get_random_bytes(nonce, FSCRYPT_FILE_NONCE_SIZE);
+	if (fscrypt_uses_extent_encryption(inode)) {
+		nonce = dir->i_crypt_info->ci_nonce;
+	} else {
+		get_random_bytes(nonce_bytes, FSCRYPT_FILE_NONCE_SIZE);
+		nonce = nonce_bytes;
+	}
+
 	return fscrypt_setup_encryption_info(inode, policy, nonce,
 					     IS_CASEFOLDED(dir) &&
-					     S_ISDIR(inode->i_mode));
+					     S_ISDIR(inode->i_mode),
+					     NULL);
 }
 EXPORT_SYMBOL_GPL(fscrypt_prepare_new_inode);
 
-- 
2.40.1

[PATCH v2 07/14] fscrypt: notify per-extent infos if master key vanishes

[Sweet Tea Dorminy]

When fscrypt_infos can be owned by an extent, we need some way to
attempt to evict the extent infos in the same way we evict inodes.
This change adds a function pointer to allow a filesystem to optionally
provide a way to evict extents; locking if needed must be handled by the
filesystem.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/keyring.c     | 11 ++++++++++-
 include/linux/fscrypt.h |  9 +++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 7cbb1fd872ac..0aad825087c1 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -875,6 +875,16 @@ static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
 
 	list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) {
 		inode = ci->ci_inode;
+		if (!inode) {
+			if (!ci->ci_sb->s_cop->forget_extent_info)
+				continue;
+
+			spin_unlock(&mk->mk_decrypted_inodes_lock);
+			ci->ci_sb->s_cop->forget_extent_info(ci->ci_info_ptr);
+			spin_lock(&mk->mk_decrypted_inodes_lock);
+			continue;
+		}
+
 		spin_lock(&inode->i_lock);
 		if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
 			spin_unlock(&inode->i_lock);
@@ -887,7 +897,6 @@ static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
 		shrink_dcache_inode(inode);
 		iput(toput_inode);
 		toput_inode = inode;
-
 		spin_lock(&mk->mk_decrypted_inodes_lock);
 	}
 
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index c895b12737a1..378a1f41c62f 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
-- 
2.40.1

[PATCH v2 08/14] fscrypt: use an optional ino equivalent for per-extent infos

[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/fscrypt_private.h | 18 ++++++++++++++++++
 fs/crypto/keyring.c         |  8 ++++----
 fs/crypto/keysetup.c        |  6 +++---
 include/linux/fscrypt.h     |  9 +++++++++
 4 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 1674e66e72e3..8bf27ceeecd1 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -332,6 +332,24 @@ static inline bool fscrypt_uses_extent_encryption(const struct inode *inode)
 	return false;
 }
 
+/**
+ * fscrypt_get_info_ino() - get the ino or ino equivalent for an info
+ *
+ * @ci: the fscrypt_info in question
+ *
+ * Return: 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(struct super_block *sb);
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 0aad825087c1..bfcd2ecbe481 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -923,12 +923,12 @@ static int check_for_busy_inodes(struct super_block *sb,
 	}
 
 	{
-		/* select an example file to show for debugging purposes */
-		struct inode *inode =
+		/* select an example info to show for debugging purposes */
+		struct fscrypt_info *ci =
 			list_first_entry(&mk->mk_decrypted_inodes,
 					 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_decrypted_inodes_lock);
 
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 29565338d9c0..d20cee61e24f 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -380,10 +380,10 @@ 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_ONCE(ci->ci_inode->i_ino == 0);
+	WARN_ON_ONCE(fscrypt_get_info_ino(ci) == 0);
 	WARN_ON_ONCE(!mk->mk_ino_hash_key_initialized);
 
-	ci->ci_hashed_ino = (u32)siphash_1u64(ci->ci_inode->i_ino,
+	ci->ci_hashed_ino = (u32)siphash_1u64(fscrypt_get_info_ino(ci),
 					      &mk->mk_ino_hash_key);
 }
 
@@ -706,7 +706,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
 		if (res)
 			goto out;
 
-		if (inode->i_ino)
+		if (fscrypt_get_info_ino(crypt_info))
 			fscrypt_hash_inode_number(crypt_info, mk);
 	}
 
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 378a1f41c62f..22affbb15706 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -169,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.40.1

[PATCH v2 09/14] fscrypt: move function call warning of busy inodes

[Sweet Tea Dorminy]

Extent encryption will want to attempt to evict inodes, and not warn of
busy ones, before removing the key instead of after as it is at present.
Therefore pull the call for check_for_busy_inodes() out of
try_to_lock_encrypted_files() into its only callsite.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/keyring.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index bfcd2ecbe481..c4499248b6cc 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -947,8 +947,7 @@ static int check_for_busy_inodes(struct super_block *sb,
 static int try_to_lock_encrypted_files(struct super_block *sb,
 				       struct fscrypt_master_key *mk)
 {
-	int err1;
-	int err2;
+	int err;
 
 	/*
 	 * An inode can't be evicted while it is dirty or has dirty pages.
@@ -960,7 +959,7 @@ static int try_to_lock_encrypted_files(struct super_block *sb,
 	 * already call sync_filesystem() via sys_syncfs() or sys_sync().
 	 */
 	down_read(&sb->s_umount);
-	err1 = sync_filesystem(sb);
+	err = sync_filesystem(sb);
 	up_read(&sb->s_umount);
 	/* If a sync error occurs, still try to evict as much as possible. */
 
@@ -972,16 +971,7 @@ static int try_to_lock_encrypted_files(struct super_block *sb,
 	 */
 	evict_dentries_for_decrypted_inodes(mk);
 
-	/*
-	 * evict_dentries_for_decrypted_inodes() 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
-	 * just need to check whether any inodes couldn't be evicted.
-	 */
-	err2 = check_for_busy_inodes(sb, mk);
-
-	return err1 ?: err2;
+	return err;
 }
 
 /*
@@ -1073,14 +1063,24 @@ static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users)
 	up_write(&mk->mk_sem);
 
 	if (inodes_remain) {
+		int err2;
 		/* Some inodes still reference this key; try to evict them. */
 		err = try_to_lock_encrypted_files(sb, mk);
+		/* We already tried to iput() each inode referencing this key
+		 * which would cause the inode to be evicted if that was the
+		 * last reference (since fscrypt_drop_inode() would see the
+		 * key removal). So the only remaining inodes referencing this
+		 * key are still busy and couldn't be evicted; check for them.
+		 */
+		err2 = check_for_busy_inodes(sb, mk);
+		err = err ?: err2;
 		if (err == -EBUSY) {
 			status_flags |=
 				FSCRYPT_KEY_REMOVAL_STATUS_FLAG_FILES_BUSY;
 			err = 0;
 		}
 	}
+
 	/*
 	 * We return 0 if we successfully did something: removed a claim to the
 	 * key, wiped the secret, or tried locking the files again.  Users need
-- 
2.40.1

[PATCH v2 10/14] fscrypt: revamp key removal for extent encryption

[Sweet Tea Dorminy]

Currently, for inode encryption, once an inode is open IO will not fail
due to lack of key until the inode is closed. Even if the key is
removed, open inodes will continue to use the key.

For extent encryption, it's a little harder, since the extent may not be
createdi/loaded until well after the REMOVE_KEY ioctl is called. To be
as similar to inode based encryption as plausible, this changes key
removal to be 'soft' for extent-based encryption, allowing new extents
to use keys which were in use by open inodes at the time of removal;
this hopefully follows the discussion at [1].

[1] https://lore.kernel.org/linux-fscrypt/248eac32-96cc-eb2e-85da-422a8d75a376@dorminy.me/T/#m48f43837cf98e0212de2e70aa6435320e3532d6e

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/fscrypt_private.h | 37 ++++++++++++++++++++++++++++-----
 fs/crypto/keyring.c         | 41 ++++++++++++++++++++++++++++---------
 fs/crypto/keysetup.c        | 32 ++++++++++++++++++++++++++++-
 3 files changed, 94 insertions(+), 16 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 8bf27ceeecd1..926531597e7b 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -332,6 +332,21 @@ static inline bool fscrypt_uses_extent_encryption(const struct inode *inode)
 	return false;
 }
 
+/**
+ * fscrypt_fs_uses_extent_encryption() -- whether a filesystem uses per-extent
+ *				          encryption
+ *
+ * @sb: the superblock of the filesystem in question
+ *
+ * Return: true if the fs uses per-extent fscrypt_infos, false otherwise
+ */
+static inline bool
+fscrypt_fs_uses_extent_encryption(const struct super_block *sb)
+{
+	// No filesystems currently use per-extent infos
+	return false;
+}
+
 /**
  * fscrypt_get_info_ino() - get the ino or ino equivalent for an info
  *
@@ -556,11 +571,14 @@ 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
-	 * 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.
+	 * executed, no new inodes can be unlocked with this key; however,
+	 * there may still be inodes in ->mk_decrypted_inodes which could not
+	 * be evicted. For inode-based encryption, the secret is wiped; for
+	 * extent-based encryption, the secret is preserved while inodes still
+	 * reference it, as they may need to create new extents using the
+	 * secret to service IO; @soft_deleted is set to true then. 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.
 	 *
 	 * While ->mk_secret is present, one ref in ->mk_active_refs is held.
 	 *
@@ -599,6 +617,13 @@ struct fscrypt_master_key {
 	struct list_head	mk_decrypted_inodes;
 	spinlock_t		mk_decrypted_inodes_lock;
 
+	/*
+	 * Whether the key is unavailable to new inodes, but still available
+	 * to new extents within decrypted inodes. Protected by
+	 * ->mk_decrypted_inodes_lock.
+	 */
+	bool			mk_soft_deleted;
+
 	/*
 	 * Per-mode encryption keys for the various types of encryption policies
 	 * that use them.  Allocated and derived on-demand.
@@ -626,6 +651,8 @@ is_master_key_secret_present(const struct fscrypt_master_key_secret *secret)
 	return READ_ONCE(secret->size) != 0;
 }
 
+void fscrypt_wipe_master_key_secret(struct fscrypt_master_key_secret *secret);
+
 static inline const char *master_key_spec_type(
 				const struct fscrypt_key_specifier *spec)
 {
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index c4499248b6cc..59748d333b89 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -38,7 +38,7 @@ struct fscrypt_keyring {
 	struct hlist_head key_hashtable[128];
 };
 
-static void wipe_master_key_secret(struct fscrypt_master_key_secret *secret)
+void fscrypt_wipe_master_key_secret(struct fscrypt_master_key_secret *secret)
 {
 	fscrypt_destroy_hkdf(&secret->hkdf);
 	memzero_explicit(secret, sizeof(*secret));
@@ -239,8 +239,9 @@ void fscrypt_destroy_keyring(struct super_block *sb)
 			 */
 			WARN_ON_ONCE(refcount_read(&mk->mk_active_refs) != 1);
 			WARN_ON_ONCE(refcount_read(&mk->mk_struct_refs) != 1);
-			WARN_ON_ONCE(!is_master_key_secret_present(&mk->mk_secret));
-			wipe_master_key_secret(&mk->mk_secret);
+			WARN_ON_ONCE(!mk->mk_soft_deleted &&
+				     !is_master_key_secret_present(&mk->mk_secret));
+			fscrypt_wipe_master_key_secret(&mk->mk_secret);
 			fscrypt_put_master_key_activeref(sb, mk);
 		}
 	}
@@ -485,6 +486,10 @@ static int add_existing_master_key(struct fscrypt_master_key *mk,
 		move_master_key_secret(&mk->mk_secret, secret);
 	}
 
+	spin_lock(&mk->mk_decrypted_inodes_lock);
+	mk->mk_soft_deleted = false;
+	spin_unlock(&mk->mk_decrypted_inodes_lock);
+
 	return 0;
 }
 
@@ -738,7 +743,7 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
 		goto out_wipe_secret;
 	err = 0;
 out_wipe_secret:
-	wipe_master_key_secret(&secret);
+	fscrypt_wipe_master_key_secret(&secret);
 	return err;
 }
 EXPORT_SYMBOL_GPL(fscrypt_ioctl_add_key);
@@ -770,7 +775,7 @@ int fscrypt_get_test_dummy_key_identifier(
 				  NULL, 0, key_identifier,
 				  FSCRYPT_KEY_IDENTIFIER_SIZE);
 out:
-	wipe_master_key_secret(&secret);
+	fscrypt_wipe_master_key_secret(&secret);
 	return err;
 }
 
@@ -794,7 +799,7 @@ int fscrypt_add_test_dummy_key(struct super_block *sb,
 
 	fscrypt_get_test_dummy_secret(&secret);
 	err = add_master_key(sb, &secret, key_spec);
-	wipe_master_key_secret(&secret);
+	fscrypt_wipe_master_key_secret(&secret);
 	return err;
 }
 
@@ -1026,6 +1031,12 @@ static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users)
 	mk = fscrypt_find_master_key(sb, &arg.key_spec);
 	if (!mk)
 		return -ENOKEY;
+
+	if (fscrypt_fs_uses_extent_encryption(sb)) {
+		/* Keep going even if this has an error. */
+		try_to_lock_encrypted_files(sb, mk);
+	}
+
 	down_write(&mk->mk_sem);
 
 	/* If relevant, remove current user's (or all users) claim to the key */
@@ -1052,13 +1063,23 @@ static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users)
 		}
 	}
 
-	/* No user claims remaining.  Go ahead and wipe the secret. */
+	/* No user claims remaining. */
 	err = -ENOKEY;
-	if (is_master_key_secret_present(&mk->mk_secret)) {
-		wipe_master_key_secret(&mk->mk_secret);
+	if (fscrypt_fs_uses_extent_encryption(sb) && refcount_read(&mk->mk_active_refs) > 1) {
+		mk->mk_soft_deleted = true;
+		err = 0;
+	} else if (is_master_key_secret_present(&mk->mk_secret)) {
+		fscrypt_wipe_master_key_secret(&mk->mk_secret);
 		fscrypt_put_master_key_activeref(sb, mk);
 		err = 0;
+	} else if (mk->mk_soft_deleted) {
+		/*
+		 * Was soft deleted, but all inodes have stopped using it, and
+		 * the secret was wiped by the last one.
+		 */
+		err = 0;
 	}
+
 	inodes_remain = refcount_read(&mk->mk_active_refs) > 0;
 	up_write(&mk->mk_sem);
 
@@ -1159,7 +1180,7 @@ int fscrypt_ioctl_get_key_status(struct file *filp, void __user *uarg)
 	}
 	down_read(&mk->mk_sem);
 
-	if (!is_master_key_secret_present(&mk->mk_secret)) {
+	if (mk->mk_soft_deleted || !is_master_key_secret_present(&mk->mk_secret)) {
 		arg.status = refcount_read(&mk->mk_active_refs) > 0 ?
 			FSCRYPT_KEY_STATUS_INCOMPLETELY_REMOVED :
 			FSCRYPT_KEY_STATUS_ABSENT /* raced with full removal */;
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index d20cee61e24f..2b4fca6814a7 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -576,6 +576,15 @@ static int find_and_lock_master_key(const struct fscrypt_info *ci,
 		goto out_release_key;
 	}
 
+	if (!ci->ci_info_ptr && mk->mk_soft_deleted) {
+		/*
+		 * This is an inode info, and only extent infos can use keys
+		 * that have been soft deleted
+		 */
+		err = -ENOKEY;
+		goto out_release_key;
+	}
+
 	*mk_ret = mk;
 	return 0;
 
@@ -606,6 +615,8 @@ static void put_crypt_info(struct fscrypt_info *ci)
 
 	mk = ci->ci_master_key;
 	if (mk) {
+		bool wipe_secret = false;
+
 		/*
 		 * Remove this inode from the list of inodes that were unlocked
 		 * with the master key.  In addition, if we're removing the last
@@ -614,7 +625,25 @@ static void put_crypt_info(struct fscrypt_info *ci)
 		 */
 		spin_lock(&mk->mk_decrypted_inodes_lock);
 		list_del(&ci->ci_master_key_link);
+		wipe_secret = (list_empty(&mk->mk_decrypted_inodes) &&
+				mk->mk_soft_deleted);
 		spin_unlock(&mk->mk_decrypted_inodes_lock);
+		if (wipe_secret) {
+			/* Someone tried to remove this key, but there were
+			 * still inodes open that could need new extents, which
+			 * needed to be able to access the key secret. But now
+			 * this was the last reference. So we can delete the
+			 * key secret now. (We don't need to check for new
+			 * inodes on the decrypted_inode list because once
+			 * ->mk_soft_deleted is set, no new inode can join the
+			 * list.
+			 */
+			down_write(&mk->mk_sem);
+			fscrypt_wipe_master_key_secret(&mk->mk_secret);
+			up_write(&mk->mk_sem);
+			fscrypt_put_master_key_activeref(ci->ci_sb, mk);
+		}
+
 		fscrypt_put_master_key_activeref(ci->ci_sb, mk);
 	}
 	memzero_explicit(ci, sizeof(*ci));
@@ -958,6 +987,7 @@ int fscrypt_drop_inode(struct inode *inode)
 	 * then the thread removing the key will either evict the inode itself
 	 * or will correctly detect that it wasn't evicted due to the race.
 	 */
-	return !is_master_key_secret_present(&ci->ci_master_key->mk_secret);
+	return READ_ONCE(ci->ci_master_key->mk_soft_deleted) ||
+		!is_master_key_secret_present(&ci->ci_master_key->mk_secret);
 }
 EXPORT_SYMBOL_GPL(fscrypt_drop_inode);
-- 
2.40.1

[PATCH v2 11/14] fscrypt: add creation/usage/freeing of per-extent infos

[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          |  2 +
 fs/crypto/fscrypt_private.h | 76 +++++++++++++++++++++----------------
 fs/crypto/keyring.c         |  9 +----
 fs/crypto/keysetup.c        | 58 +++++++++++++++++++++++++++-
 include/linux/fscrypt.h     | 48 ++++++++++++++++++-----
 5 files changed, 141 insertions(+), 52 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index d75f1b3f5795..0f0c721e40fe 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -113,6 +113,8 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
 	struct crypto_skcipher *tfm = ci->ci_enc_key->tfm;
 	int res = 0;
 
+	if (!ci)
+		return -EINVAL;
 	if (WARN_ON_ONCE(len <= 0))
 		return -EINVAL;
 	if (WARN_ON_ONCE(len % FSCRYPT_CONTENTS_ALIGNMENT != 0))
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 926531597e7b..6e6020f7746c 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -286,6 +286,38 @@ typedef enum {
 	FS_ENCRYPT,
 } fscrypt_direction_t;
 
+/**
+ * fscrypt_fs_uses_extent_encryption() -- whether a filesystem uses per-extent
+ *				          encryption
+ *
+ * @sb: the superblock of the filesystem in question
+ *
+ * Return: true if the fs uses per-extent fscrypt_infos, false otherwise
+ */
+static inline bool
+fscrypt_fs_uses_extent_encryption(const struct super_block *sb)
+{
+	return !!sb->s_cop->get_extent_info;
+}
+
+/**
+ * fscrypt_uses_extent_encryption() -- whether an inode uses per-extent
+ *				       encryption
+ *
+ * @inode: the inode in question
+ *
+ * Return: true if the inode uses per-extent fscrypt_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;
+
+	return fscrypt_fs_uses_extent_encryption(inode->i_sb);
+}
+
+
 /**
  * fscrypt_get_lblk_info() - get the fscrypt_info to crypt a particular block
  *
@@ -306,6 +338,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)
@@ -314,39 +357,6 @@ fscrypt_get_lblk_info(const struct inode *inode, u64 lblk, u64 *offset,
 	return inode->i_crypt_info;
 }
 
-/**
- * fscrypt_uses_extent_encryption() -- whether an inode uses per-extent
- *				       encryption
- *
- * @inode: the inode in question
- *
- * Return: true if the inode uses per-extent fscrypt_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 filesystems currently use per-extent infos
-	return false;
-}
-
-/**
- * fscrypt_fs_uses_extent_encryption() -- whether a filesystem uses per-extent
- *				          encryption
- *
- * @sb: the superblock of the filesystem in question
- *
- * Return: true if the fs uses per-extent fscrypt_infos, false otherwise
- */
-static inline bool
-fscrypt_fs_uses_extent_encryption(const struct super_block *sb)
-{
-	// No filesystems currently use per-extent infos
-	return false;
-}
-
 /**
  * fscrypt_get_info_ino() - get the ino or ino equivalent for an info
  *
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 59748d333b89..8e4065d1e422 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -880,15 +880,8 @@ static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
 
 	list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) {
 		inode = ci->ci_inode;
-		if (!inode) {
-			if (!ci->ci_sb->s_cop->forget_extent_info)
-				continue;
-
-			spin_unlock(&mk->mk_decrypted_inodes_lock);
-			ci->ci_sb->s_cop->forget_extent_info(ci->ci_info_ptr);
-			spin_lock(&mk->mk_decrypted_inodes_lock);
+		if (ci->ci_info_ptr)
 			continue;
-		}
 
 		spin_lock(&inode->i_lock);
 		if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 2b4fca6814a7..c8cdcd4fe835 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -676,8 +676,8 @@ fscrypt_setup_encryption_info(struct inode *inode,
 
 	if (fscrypt_uses_extent_encryption(inode) && info_for_extent)
 		crypt_info->ci_info_ptr = info_ptr;
-	else
-		crypt_info->ci_inode = inode;
+
+	crypt_info->ci_inode = inode;
 
 	crypt_info->ci_sb = inode->i_sb;
 	crypt_info->ci_policy = *policy;
@@ -917,6 +917,60 @@ 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)
+{
+	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;
+
+	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);
+	*info_ptr = 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 22affbb15706..e39165fbed41 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).
 	 *
@@ -129,15 +152,6 @@ 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
@@ -348,6 +362,10 @@ 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);
+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);
@@ -609,6 +627,18 @@ 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)
+{
+	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.40.1

[PATCH v2 12/14] fscrypt: allow load/save of extent contexts

[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, 86 insertions(+), 1 deletion(-)

diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index c8cdcd4fe835..3b80e7061039 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -847,7 +847,6 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 					    IS_CASEFOLDED(inode) &&
 					    S_ISDIR(inode->i_mode),
 					    NULL);
-
 	if (res == -ENOPKG && allow_unsupported) /* Algorithm unavailable? */
 		res = 0;
 	if (res == -ENOKEY)
@@ -971,6 +970,55 @@ 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, len);
+	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 f4456ecb3f87..9ecb01e49d33 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -762,6 +762,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 e39165fbed41..4ba624beea91 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -325,6 +325,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;
@@ -365,6 +367,8 @@ int fscrypt_drop_inode(struct inode *inode);
 int fscrypt_prepare_new_extent(struct inode *inode,
 			       struct fscrypt_info **info_ptr);
 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,
@@ -541,6 +545,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 {
 };
 
@@ -639,6 +649,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.40.1

[PATCH v2 13/14] fscrypt: save session key credentials for extent infos

[Sweet Tea Dorminy]

For v1 encryption policies using per-session keys, the thread which
opens the inode and therefore initializes the encryption info is part of
the session, so it can get the key from the session keyring. However,
for extent encryption, the extent infos are likely loaded from a
different thread, which does not have access to the session keyring.
This change saves the credentials of the inode opening thread and reuses
those credentials temporarily when dealing with extent infos, allowing
finding the encryption key correctly.

v1 encryption policies using per-session keys should probably not exist
for new usages such as extent encryption, but this makes more tests
work without change; maybe the right answer is to disallow v1 session
keys plus extent encryption and deal with editing tests to not use v1
session encryption so much.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/fscrypt_private.h |  8 ++++++++
 fs/crypto/keysetup.c        | 14 ++++++++++++++
 fs/crypto/keysetup_v1.c     |  1 +
 3 files changed, 23 insertions(+)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 6e6020f7746c..a1c484511ba3 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -231,6 +231,14 @@ struct fscrypt_info {
 	 */
 	bool ci_inlinecrypt;
 #endif
+	/* Credential struct from the thread which created this info. This is
+	 * only used in v1 session keyrings with extent encryption; it allows
+	 * the thread creating extents for an inode to join the session
+	 * keyring temporarily, since otherwise the thread is usually part of
+	 * kernel writeback and therefore unrelated to the thread with the
+	 * right session key.
+	 */
+	struct cred *ci_session_creds;
 
 	/*
 	 * Encryption mode used for this inode.  It corresponds to either the
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 3b80e7061039..9c56ef8d2eb6 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -646,6 +646,8 @@ static void put_crypt_info(struct fscrypt_info *ci)
 
 		fscrypt_put_master_key_activeref(ci->ci_sb, mk);
 	}
+	if (ci->ci_session_creds)
+		abort_creds(ci->ci_session_creds);
 	memzero_explicit(ci, sizeof(*ci));
 	kmem_cache_free(fscrypt_info_cachep, ci);
 }
@@ -662,6 +664,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	struct fscrypt_master_key *mk = NULL;
 	int res;
 	bool info_for_extent = !!info_ptr;
+	const struct cred *creds = NULL;
 
 	if (!info_ptr)
 		info_ptr = &inode->i_crypt_info;
@@ -705,7 +708,18 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	if (res)
 		goto out;
 
+	if (info_for_extent && inode->i_crypt_info->ci_session_creds) {
+		 creds = override_creds(inode->i_crypt_info->ci_session_creds);
+		/*
+		 * The inode this is being created for is using a session key,
+		 * so we have to join this thread to that session temporarily
+		 * in order to be able to find the right key...
+		 */
+	}
+
 	res = fscrypt_setup_file_key(crypt_info, mk);
+	if (creds)
+		revert_creds(creds);
 	if (res)
 		goto out;
 
diff --git a/fs/crypto/keysetup_v1.c b/fs/crypto/keysetup_v1.c
index 41d317f08aeb..4f2be2377dfa 100644
--- a/fs/crypto/keysetup_v1.c
+++ b/fs/crypto/keysetup_v1.c
@@ -318,6 +318,7 @@ int fscrypt_setup_v1_file_key_via_subscribed_keyrings(struct fscrypt_info *ci)
 		return PTR_ERR(key);
 
 	err = fscrypt_setup_v1_file_key(ci, payload->raw);
+	ci->ci_session_creds = prepare_creds();
 	up_read(&key->sem);
 	key_put(key);
 	return err;
-- 
2.40.1

[PATCH v2 14/14] fscrypt: update documentation for per-extent keys

[Sweet Tea Dorminy]

Add some documentation of how extent-based encryption works, hopefully
enough for future filesystem users.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 Documentation/filesystems/fscrypt.rst | 43 +++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index eccd327e6df5..e862d59bd5b5 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
@@ -125,6 +125,11 @@ However, these ioctls have some limitations:
   well as kill any processes whose working directory is in an affected
   encrypted directory.
 
+- If the filesystem is using extent-based encryption, the master
+  encryption key will *not* be wiped from kernel memory until all
+  inodes using the key have been evicted (requiring that all files
+  using the key are closed).
+
 - The kernel cannot magically wipe copies of the master key(s) that
   userspace might have as well.  Therefore, userspace must wipe all
   copies of the master key(s) it makes as well; normally this should
@@ -280,6 +285,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
 -----------------------
 
@@ -381,12 +391,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:
@@ -410,6 +421,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.40.1

Re: [PATCH v2 03/14] fscrypt: adjust effective lblks based on extents

[Josef Bacik]

On Sun, Jul 09, 2023 at 02:53:36PM -0400, Sweet Tea Dorminy wrote:
> 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       |  3 ++-
>  fs/crypto/inline_crypt.c | 24 +++++++++++++++++-------
>  2 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> index 1b7e375b1c6b..d75f1b3f5795 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;
> +	u64 ci_offset = 0;
>  	struct fscrypt_info *ci =
> -		fscrypt_get_lblk_info(inode, lblk_num, NULL, NULL);
> +		fscrypt_get_lblk_info(inode, lblk_num, &ci_offset, NULL);
>  	struct crypto_skcipher *tfm = ci->ci_enc_key->tfm;
>  	int res = 0;
>  
> diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
> index 885a2ec3d711..b3e7a5291d22 100644
> --- a/fs/crypto/inline_crypt.c
> +++ b/fs/crypto/inline_crypt.c
> @@ -267,12 +267,15 @@ 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);
> +	if (!ci)
> +		return;
>  
> -	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);
> @@ -350,22 +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;
> +	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
>  	 * merging requests where the keys are the same but the pointers differ.
>  	 */
> -	if (bc->bc_key != ci->ci_enc_key->blk_key)
> +	if (!ci || bc->bc_key != ci->ci_enc_key->blk_key)
>  		return false;
>  

This seems like an unrelated change, we weren't checking !ci before and the
behavior hasn't changed with the new code.  Thanks,

Josef

Re: [PATCH v2 05/14] fscrypt: setup leaf inodes for extent encryption

[Josef Bacik]

On Sun, Jul 09, 2023 at 02:53:38PM -0400, Sweet Tea Dorminy wrote:
> For extent-based encryption, leaf/regular file inodes are special: it's
> useful to set their i_crypt_info field so that it's easy to inherit
> their encryption policy for a new extent, but they never need to do any
> encyption themselves. Additionally, since encryption can only be set up
> on a directory, not a single file, their encryption policy can always
> duplicate their parent inode's policy.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/crypto/fscrypt_private.h | 17 +++++++++++++
>  fs/crypto/keysetup.c        | 49 ++++++++++++++++++++++++++-----------
>  2 files changed, 52 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index c04454c289fd..260635e8b558 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -308,6 +308,23 @@ fscrypt_get_lblk_info(const struct inode *inode, u64 lblk, u64 *offset,
>  	return inode->i_crypt_info;
>  }
>  
> +/**
> + * fscrypt_uses_extent_encryption() -- whether an inode uses per-extent
> + *				       encryption
> + *
> + * @inode: the inode in question
> + *
> + * Return: true if the inode uses per-extent fscrypt_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 filesystems currently use per-extent infos
> +	return false;

Wrong comment format.

> +}
>  
>  /* crypto.c */
>  extern struct kmem_cache *fscrypt_info_cachep;
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index c5f68cf65a6f..7469b2d8ac87 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -747,27 +747,48 @@ fscrypt_setup_encryption_info(struct inode *inode,
>  int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
>  {
>  	int res;
> -	union fscrypt_context ctx;
> +	union fscrypt_context ctx = { 0 };
>  	union fscrypt_policy policy;
>  
>  	if (fscrypt_has_encryption_key(inode))
>  		return 0;
>  
> -	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
> -	if (res < 0) {
> -		if (res == -ERANGE && allow_unsupported)
> -			return 0;
> -		fscrypt_warn(inode, "Error %d getting encryption context", res);
> -		return res;
> -	}
> +	if (fscrypt_uses_extent_encryption(inode)) {
> +		/*
> +		 * Nothing will be encrypted with this info, so we can borrow
> +		 * the parent (dir) inode's policy and use a zero nonce.
> +		 */
> +		struct dentry *dentry = d_find_any_alias(inode);
> +		struct dentry *parent_dentry = dget_parent(dentry);
> +		struct inode *dir = parent_dentry->d_inode;
> +		bool found = false;
>  

Can this be extracted to a helper to keep this function cleaner?  Thanks,

Josef

Re: [PATCH v2 13/14] fscrypt: save session key credentials for extent infos

[Josef Bacik]

On Sun, Jul 09, 2023 at 02:53:46PM -0400, Sweet Tea Dorminy wrote:
> For v1 encryption policies using per-session keys, the thread which
> opens the inode and therefore initializes the encryption info is part of
> the session, so it can get the key from the session keyring. However,
> for extent encryption, the extent infos are likely loaded from a
> different thread, which does not have access to the session keyring.
> This change saves the credentials of the inode opening thread and reuses
> those credentials temporarily when dealing with extent infos, allowing
> finding the encryption key correctly.
> 
> v1 encryption policies using per-session keys should probably not exist
> for new usages such as extent encryption, but this makes more tests
> work without change; maybe the right answer is to disallow v1 session
> keys plus extent encryption and deal with editing tests to not use v1
> session encryption so much.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/crypto/fscrypt_private.h |  8 ++++++++
>  fs/crypto/keysetup.c        | 14 ++++++++++++++
>  fs/crypto/keysetup_v1.c     |  1 +
>  3 files changed, 23 insertions(+)
> 
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index 6e6020f7746c..a1c484511ba3 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -231,6 +231,14 @@ struct fscrypt_info {
>  	 */
>  	bool ci_inlinecrypt;
>  #endif
> +	/* Credential struct from the thread which created this info. This is
> +	 * only used in v1 session keyrings with extent encryption; it allows
> +	 * the thread creating extents for an inode to join the session
> +	 * keyring temporarily, since otherwise the thread is usually part of
> +	 * kernel writeback and therefore unrelated to the thread with the
> +	 * right session key.
> +	 */
> +	struct cred *ci_session_creds;
>  
>  	/*
>  	 * Encryption mode used for this inode.  It corresponds to either the
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index 3b80e7061039..9c56ef8d2eb6 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -646,6 +646,8 @@ static void put_crypt_info(struct fscrypt_info *ci)
>  
>  		fscrypt_put_master_key_activeref(ci->ci_sb, mk);
>  	}
> +	if (ci->ci_session_creds)
> +		abort_creds(ci->ci_session_creds);
>  	memzero_explicit(ci, sizeof(*ci));
>  	kmem_cache_free(fscrypt_info_cachep, ci);
>  }
> @@ -662,6 +664,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
>  	struct fscrypt_master_key *mk = NULL;
>  	int res;
>  	bool info_for_extent = !!info_ptr;
> +	const struct cred *creds = NULL;
>  
>  	if (!info_ptr)
>  		info_ptr = &inode->i_crypt_info;
> @@ -705,7 +708,18 @@ fscrypt_setup_encryption_info(struct inode *inode,
>  	if (res)
>  		goto out;
>  
> +	if (info_for_extent && inode->i_crypt_info->ci_session_creds) {
> +		 creds = override_creds(inode->i_crypt_info->ci_session_creds);

Whitespace.  Thanks,

Josef

Re: [PATCH v2 07/14] fscrypt: notify per-extent infos if master key vanishes

[Josef Bacik]

On Sun, Jul 09, 2023 at 02:53:40PM -0400, Sweet Tea Dorminy wrote:
> When fscrypt_infos can be owned by an extent, we need some way to
> attempt to evict the extent infos in the same way we evict inodes.
> This change adds a function pointer to allow a filesystem to optionally
> provide a way to evict extents; locking if needed must be handled by the
> filesystem.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/crypto/keyring.c     | 11 ++++++++++-
>  include/linux/fscrypt.h |  9 +++++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
> index 7cbb1fd872ac..0aad825087c1 100644
> --- a/fs/crypto/keyring.c
> +++ b/fs/crypto/keyring.c
> @@ -875,6 +875,16 @@ static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
>  
>  	list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) {
>  		inode = ci->ci_inode;
> +		if (!inode) {
> +			if (!ci->ci_sb->s_cop->forget_extent_info)
> +				continue;
> +
> +			spin_unlock(&mk->mk_decrypted_inodes_lock);
> +			ci->ci_sb->s_cop->forget_extent_info(ci->ci_info_ptr);
> +			spin_lock(&mk->mk_decrypted_inodes_lock);
> +			continue;
> +		}
> +
>  		spin_lock(&inode->i_lock);
>  		if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
>  			spin_unlock(&inode->i_lock);
> @@ -887,7 +897,6 @@ static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
>  		shrink_dcache_inode(inode);
>  		iput(toput_inode);
>  		toput_inode = inode;
> -

Errant whitespace change.  Thanks,

Josef

Re: [PATCH v2 09/14] fscrypt: move function call warning of busy inodes

[Josef Bacik]

On Sun, Jul 09, 2023 at 02:53:42PM -0400, Sweet Tea Dorminy wrote:
> Extent encryption will want to attempt to evict inodes, and not warn of
> busy ones, before removing the key instead of after as it is at present.
> Therefore pull the call for check_for_busy_inodes() out of
> try_to_lock_encrypted_files() into its only callsite.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/crypto/keyring.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
> index bfcd2ecbe481..c4499248b6cc 100644
> --- a/fs/crypto/keyring.c
> +++ b/fs/crypto/keyring.c
> @@ -947,8 +947,7 @@ static int check_for_busy_inodes(struct super_block *sb,
>  static int try_to_lock_encrypted_files(struct super_block *sb,
>  				       struct fscrypt_master_key *mk)
>  {
> -	int err1;
> -	int err2;
> +	int err;
>  
>  	/*
>  	 * An inode can't be evicted while it is dirty or has dirty pages.
> @@ -960,7 +959,7 @@ static int try_to_lock_encrypted_files(struct super_block *sb,
>  	 * already call sync_filesystem() via sys_syncfs() or sys_sync().
>  	 */
>  	down_read(&sb->s_umount);
> -	err1 = sync_filesystem(sb);
> +	err = sync_filesystem(sb);
>  	up_read(&sb->s_umount);
>  	/* If a sync error occurs, still try to evict as much as possible. */
>  
> @@ -972,16 +971,7 @@ static int try_to_lock_encrypted_files(struct super_block *sb,
>  	 */
>  	evict_dentries_for_decrypted_inodes(mk);
>  
> -	/*
> -	 * evict_dentries_for_decrypted_inodes() 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
> -	 * just need to check whether any inodes couldn't be evicted.
> -	 */
> -	err2 = check_for_busy_inodes(sb, mk);
> -
> -	return err1 ?: err2;
> +	return err;
>  }
>  
>  /*
> @@ -1073,14 +1063,24 @@ static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users)
>  	up_write(&mk->mk_sem);
>  
>  	if (inodes_remain) {
> +		int err2;
>  		/* Some inodes still reference this key; try to evict them. */
>  		err = try_to_lock_encrypted_files(sb, mk);
> +		/* We already tried to iput() each inode referencing this key
> +		 * which would cause the inode to be evicted if that was the
> +		 * last reference (since fscrypt_drop_inode() would see the
> +		 * key removal). So the only remaining inodes referencing this
> +		 * key are still busy and couldn't be evicted; check for them.
> +		 */
> +		err2 = check_for_busy_inodes(sb, mk);
> +		err = err ?: err2;
>  		if (err == -EBUSY) {
>  			status_flags |=
>  				FSCRYPT_KEY_REMOVAL_STATUS_FLAG_FILES_BUSY;
>  			err = 0;
>  		}
>  	}
> +

Extra whitespace.  Thanks,

Josef

Re: [PATCH v2 10/14] fscrypt: revamp key removal for extent encryption

[Josef Bacik]

On Sun, Jul 09, 2023 at 02:53:43PM -0400, Sweet Tea Dorminy wrote:
> Currently, for inode encryption, once an inode is open IO will not fail
> due to lack of key until the inode is closed. Even if the key is
> removed, open inodes will continue to use the key.
> 
> For extent encryption, it's a little harder, since the extent may not be
> createdi/loaded until well after the REMOVE_KEY ioctl is called. To be
> as similar to inode based encryption as plausible, this changes key
> removal to be 'soft' for extent-based encryption, allowing new extents
> to use keys which were in use by open inodes at the time of removal;
> this hopefully follows the discussion at [1].
> 
> [1] https://lore.kernel.org/linux-fscrypt/248eac32-96cc-eb2e-85da-422a8d75a376@dorminy.me/T/#m48f43837cf98e0212de2e70aa6435320e3532d6e
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/crypto/fscrypt_private.h | 37 ++++++++++++++++++++++++++++-----
>  fs/crypto/keyring.c         | 41 ++++++++++++++++++++++++++++---------
>  fs/crypto/keysetup.c        | 32 ++++++++++++++++++++++++++++-
>  3 files changed, 94 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index 8bf27ceeecd1..926531597e7b 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -332,6 +332,21 @@ static inline bool fscrypt_uses_extent_encryption(const struct inode *inode)
>  	return false;
>  }
>  
> +/**
> + * fscrypt_fs_uses_extent_encryption() -- whether a filesystem uses per-extent
> + *				          encryption
> + *
> + * @sb: the superblock of the filesystem in question
> + *
> + * Return: true if the fs uses per-extent fscrypt_infos, false otherwise
> + */
> +static inline bool
> +fscrypt_fs_uses_extent_encryption(const struct super_block *sb)
> +{
> +	// No filesystems currently use per-extent infos
> +	return false;

Wrong comment format.

> +}
> +
>  /**
>   * fscrypt_get_info_ino() - get the ino or ino equivalent for an info
>   *
> @@ -556,11 +571,14 @@ 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
> -	 * 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.
> +	 * executed, no new inodes can be unlocked with this key; however,
> +	 * there may still be inodes in ->mk_decrypted_inodes which could not
> +	 * be evicted. For inode-based encryption, the secret is wiped; for
> +	 * extent-based encryption, the secret is preserved while inodes still
> +	 * reference it, as they may need to create new extents using the
> +	 * secret to service IO; @soft_deleted is set to true then. 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.
>  	 *
>  	 * While ->mk_secret is present, one ref in ->mk_active_refs is held.
>  	 *
> @@ -599,6 +617,13 @@ struct fscrypt_master_key {
>  	struct list_head	mk_decrypted_inodes;
>  	spinlock_t		mk_decrypted_inodes_lock;
>  
> +	/*
> +	 * Whether the key is unavailable to new inodes, but still available
> +	 * to new extents within decrypted inodes. Protected by
> +	 * ->mk_decrypted_inodes_lock.
> +	 */
> +	bool			mk_soft_deleted;
> +

You say this is protected by mk_decrypted_inodes_lock, but you only ever take it
when you set it to false.  It looks like it's more used to gate behavior, so the
locking is sort of inconsistent and doesn't appear to be needed?  Can you just
do READ_ONCE()/WRITE_ONCE() and it's fine?  If not you need to wrap it in the
lock where you're accessing it.  Thanks,

Josef

Re: [PATCH v2 11/14] fscrypt: add creation/usage/freeing of per-extent infos

[Josef Bacik]

On Sun, Jul 09, 2023 at 02:53:44PM -0400, Sweet Tea Dorminy wrote:
> 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          |  2 +
>  fs/crypto/fscrypt_private.h | 76 +++++++++++++++++++++----------------
>  fs/crypto/keyring.c         |  9 +----
>  fs/crypto/keysetup.c        | 58 +++++++++++++++++++++++++++-
>  include/linux/fscrypt.h     | 48 ++++++++++++++++++-----
>  5 files changed, 141 insertions(+), 52 deletions(-)
> 
> diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> index d75f1b3f5795..0f0c721e40fe 100644
> --- a/fs/crypto/crypto.c
> +++ b/fs/crypto/crypto.c
> @@ -113,6 +113,8 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
>  	struct crypto_skcipher *tfm = ci->ci_enc_key->tfm;
>  	int res = 0;
>  
> +	if (!ci)
> +		return -EINVAL;
>  	if (WARN_ON_ONCE(len <= 0))
>  		return -EINVAL;
>  	if (WARN_ON_ONCE(len % FSCRYPT_CONTENTS_ALIGNMENT != 0))
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index 926531597e7b..6e6020f7746c 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -286,6 +286,38 @@ typedef enum {
>  	FS_ENCRYPT,
>  } fscrypt_direction_t;
>  
> +/**
> + * fscrypt_fs_uses_extent_encryption() -- whether a filesystem uses per-extent
> + *				          encryption
> + *
> + * @sb: the superblock of the filesystem in question
> + *
> + * Return: true if the fs uses per-extent fscrypt_infos, false otherwise
> + */
> +static inline bool
> +fscrypt_fs_uses_extent_encryption(const struct super_block *sb)
> +{
> +	return !!sb->s_cop->get_extent_info;
> +}
> +
> +/**
> + * fscrypt_uses_extent_encryption() -- whether an inode uses per-extent
> + *				       encryption
> + *
> + * @inode: the inode in question
> + *
> + * Return: true if the inode uses per-extent fscrypt_infos, false otherwise
> + */
> +static inline bool fscrypt_uses_extent_encryption(const struct inode *inode)
> +{
> +	// Non-regular files don't have extents

Wrong comment format.

> +	if (!S_ISREG(inode->i_mode))
> +		return false;
> +
> +	return fscrypt_fs_uses_extent_encryption(inode->i_sb);
> +}
> +
> +
>  /**
>   * fscrypt_get_lblk_info() - get the fscrypt_info to crypt a particular block
>   *
> @@ -306,6 +338,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)
> @@ -314,39 +357,6 @@ fscrypt_get_lblk_info(const struct inode *inode, u64 lblk, u64 *offset,
>  	return inode->i_crypt_info;
>  }
>  
> -/**
> - * fscrypt_uses_extent_encryption() -- whether an inode uses per-extent
> - *				       encryption
> - *
> - * @inode: the inode in question
> - *
> - * Return: true if the inode uses per-extent fscrypt_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 filesystems currently use per-extent infos
> -	return false;
> -}
> -
> -/**
> - * fscrypt_fs_uses_extent_encryption() -- whether a filesystem uses per-extent
> - *				          encryption
> - *
> - * @sb: the superblock of the filesystem in question
> - *
> - * Return: true if the fs uses per-extent fscrypt_infos, false otherwise
> - */
> -static inline bool
> -fscrypt_fs_uses_extent_encryption(const struct super_block *sb)
> -{
> -	// No filesystems currently use per-extent infos
> -	return false;
> -}
> -
>  /**
>   * fscrypt_get_info_ino() - get the ino or ino equivalent for an info
>   *
> diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
> index 59748d333b89..8e4065d1e422 100644
> --- a/fs/crypto/keyring.c
> +++ b/fs/crypto/keyring.c
> @@ -880,15 +880,8 @@ static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
>  
>  	list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) {
>  		inode = ci->ci_inode;
> -		if (!inode) {
> -			if (!ci->ci_sb->s_cop->forget_extent_info)
> -				continue;
> -
> -			spin_unlock(&mk->mk_decrypted_inodes_lock);
> -			ci->ci_sb->s_cop->forget_extent_info(ci->ci_info_ptr);
> -			spin_lock(&mk->mk_decrypted_inodes_lock);
> +		if (ci->ci_info_ptr)
>  			continue;
> -		}
>  
>  		spin_lock(&inode->i_lock);
>  		if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index 2b4fca6814a7..c8cdcd4fe835 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -676,8 +676,8 @@ fscrypt_setup_encryption_info(struct inode *inode,
>  
>  	if (fscrypt_uses_extent_encryption(inode) && info_for_extent)
>  		crypt_info->ci_info_ptr = info_ptr;
> -	else
> -		crypt_info->ci_inode = inode;
> +
> +	crypt_info->ci_inode = inode;

You changed this and now you're changing it back, go back to the original patch
and leave this the way it was.

>  
>  	crypt_info->ci_sb = inode->i_sb;
>  	crypt_info->ci_policy = *policy;
> @@ -917,6 +917,60 @@ 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)
> +{
> +	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;
> +
> +	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);
> +	*info_ptr = 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 22affbb15706..e39165fbed41 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).
>  	 *
> @@ -129,15 +152,6 @@ 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);
> -

You've done this again, backed out a change you did before.  Rework the series
to simply not need this thing in the first place.  Thanks,

Josef

Re: [PATCH v2 12/14] fscrypt: allow load/save of extent contexts

[Josef Bacik]

On Sun, Jul 09, 2023 at 02:53:45PM -0400, 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, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index c8cdcd4fe835..3b80e7061039 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -847,7 +847,6 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
>  					    IS_CASEFOLDED(inode) &&
>  					    S_ISDIR(inode->i_mode),
>  					    NULL);
> -

Errant change.  Thanks,

Josef