[PATCH v5 00/13] Move fscrypt and fsverity info out of struct inode

[PATCH v5 00/13] Move fscrypt and fsverity info out of struct inode

[Eric Biggers]

This is a cleaned-up implementation of moving the i_crypt_info and
i_verity_info pointers out of 'struct inode' and into the fs-specific
part of the inode, as proposed previously by Christian at
https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/

The high-level concept is still the same: fs/crypto/ and fs/verity/
locate the pointer by adding an offset to the address of struct inode.
The offset is retrieved from fscrypt_operations or fsverity_operations.

I've cleaned up a lot of the details, including:
- Grouped changes into patches differently
- Rewrote commit messages and comments to be clearer
- Adjusted code formatting to be consistent with existing code
- Removed unneeded #ifdefs
- Improved choice and location of VFS_WARN_ON_ONCE() statements
- Added missing kerneldoc for ubifs_inode::i_crypt_info
- Moved field initialization to init_once functions when they exist
- Improved ceph offset calculation and removed unneeded static_asserts
- fsverity_get_info() now checks IS_VERITY() instead of v_ops
- fscrypt_put_encryption_info() no longer checks IS_ENCRYPTED(), since I
  no longer think it's actually correct there.
- verity_data_blocks() now keeps doing a raw dereference
- Dropped fscrypt_set_inode_info() 
- Renamed some functions
- Do offset calculation using int, so we don't rely on unsigned overflow
- And more.

For v4 and earlier, see
https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/

I'd like to take this series through the fscrypt tree for 6.18.
(fsverity normally has a separate tree, but by choosing just one tree
for this, we'll avoid conflicts in some places.)

Eric Biggers (13):
  fscrypt: replace raw loads of info pointer with helper function
  fscrypt: add support for info in fs-specific part of inode
  ext4: move crypt info pointer to fs-specific part of inode
  f2fs: move crypt info pointer to fs-specific part of inode
  ubifs: move crypt info pointer to fs-specific part of inode
  ceph: move crypt info pointer to fs-specific part of inode
  fs: remove inode::i_crypt_info
  fsverity: add support for info in fs-specific part of inode
  ext4: move verity info pointer to fs-specific part of inode
  f2fs: move verity info pointer to fs-specific part of inode
  btrfs: move verity info pointer to fs-specific part of inode
  fs: remove inode::i_verity_info
  fsverity: check IS_VERITY() in fsverity_cleanup_inode()

 fs/btrfs/btrfs_inode.h       |  5 ++++
 fs/btrfs/inode.c             |  3 ++
 fs/btrfs/verity.c            |  2 ++
 fs/ceph/crypto.c             |  2 ++
 fs/ceph/inode.c              |  1 +
 fs/ceph/super.h              |  1 +
 fs/crypto/bio.c              |  2 +-
 fs/crypto/crypto.c           | 14 +++++----
 fs/crypto/fname.c            | 11 +++----
 fs/crypto/fscrypt_private.h  |  4 +--
 fs/crypto/hooks.c            |  2 +-
 fs/crypto/inline_crypt.c     | 12 ++++----
 fs/crypto/keysetup.c         | 43 ++++++++++++++++-----------
 fs/crypto/policy.c           |  7 +++--
 fs/ext4/crypto.c             |  2 ++
 fs/ext4/ext4.h               |  8 +++++
 fs/ext4/super.c              |  6 ++++
 fs/ext4/verity.c             |  2 ++
 fs/f2fs/f2fs.h               |  6 ++++
 fs/f2fs/super.c              | 10 ++++++-
 fs/f2fs/verity.c             |  2 ++
 fs/ubifs/crypto.c            |  2 ++
 fs/ubifs/ubifs.h             |  4 +++
 fs/verity/enable.c           |  6 ++--
 fs/verity/fsverity_private.h |  9 +++---
 fs/verity/open.c             | 23 ++++++++-------
 fs/verity/verify.c           |  2 +-
 include/linux/fs.h           | 10 -------
 include/linux/fscrypt.h      | 40 +++++++++++++++++++++++--
 include/linux/fsverity.h     | 57 +++++++++++++++++++++++++++++-------
 30 files changed, 215 insertions(+), 83 deletions(-)


base-commit: 561c80369df0733ba0574882a1635287b20f9de2
-- 
2.50.1

[PATCH v5 01/13] fscrypt: replace raw loads of info pointer with helper function

[Eric Biggers]

Add and use a helper function fscrypt_get_inode_info_raw().  It loads an
inode's fscrypt info pointer using a raw dereference, which is
appropriate when the caller knows the key setup already happened.

This eliminates most occurrences of inode::i_crypt_info in the source,
in preparation for replacing that with a filesystem-specific field.

Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 fs/crypto/bio.c          |  2 +-
 fs/crypto/crypto.c       | 14 ++++++++------
 fs/crypto/fname.c        | 11 ++++++-----
 fs/crypto/hooks.c        |  2 +-
 fs/crypto/inline_crypt.c | 12 +++++++-----
 fs/crypto/policy.c       |  7 ++++---
 include/linux/fscrypt.h  | 16 ++++++++++++++++
 7 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index 486fcb2ecf13e..0d746de4cd103 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -111,11 +111,11 @@ static int fscrypt_zeroout_range_inline_crypt(const struct inode *inode,
  * Return: 0 on success; -errno on failure.
  */
 int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
 			  sector_t pblk, unsigned int len)
 {
-	const struct fscrypt_inode_info *ci = inode->i_crypt_info;
+	const struct fscrypt_inode_info *ci = fscrypt_get_inode_info_raw(inode);
 	const unsigned int du_bits = ci->ci_data_unit_bits;
 	const unsigned int du_size = 1U << du_bits;
 	const unsigned int du_per_page_bits = PAGE_SHIFT - du_bits;
 	const unsigned int du_per_page = 1U << du_per_page_bits;
 	u64 du_index = (u64)lblk << (inode->i_blkbits - du_bits);
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index b6ccab524fdef..07f9cbfe3ea41 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -171,11 +171,11 @@ int fscrypt_crypt_data_unit(const struct fscrypt_inode_info *ci,
  */
 struct page *fscrypt_encrypt_pagecache_blocks(struct folio *folio,
 		size_t len, size_t offs, gfp_t gfp_flags)
 {
 	const struct inode *inode = folio->mapping->host;
-	const struct fscrypt_inode_info *ci = inode->i_crypt_info;
+	const struct fscrypt_inode_info *ci = fscrypt_get_inode_info_raw(inode);
 	const unsigned int du_bits = ci->ci_data_unit_bits;
 	const unsigned int du_size = 1U << du_bits;
 	struct page *ciphertext_page;
 	u64 index = ((u64)folio->index << (PAGE_SHIFT - du_bits)) +
 		    (offs >> du_bits);
@@ -230,12 +230,13 @@ int fscrypt_encrypt_block_inplace(const struct inode *inode, struct page *page,
 				  unsigned int len, unsigned int offs,
 				  u64 lblk_num)
 {
 	if (WARN_ON_ONCE(inode->i_sb->s_cop->supports_subblock_data_units))
 		return -EOPNOTSUPP;
-	return fscrypt_crypt_data_unit(inode->i_crypt_info, FS_ENCRYPT,
-				       lblk_num, page, page, len, offs);
+	return fscrypt_crypt_data_unit(fscrypt_get_inode_info_raw(inode),
+				       FS_ENCRYPT, lblk_num, page, page, len,
+				       offs);
 }
 EXPORT_SYMBOL(fscrypt_encrypt_block_inplace);
 
 /**
  * fscrypt_decrypt_pagecache_blocks() - Decrypt data from a pagecache folio
@@ -253,11 +254,11 @@ EXPORT_SYMBOL(fscrypt_encrypt_block_inplace);
  */
 int fscrypt_decrypt_pagecache_blocks(struct folio *folio, size_t len,
 				     size_t offs)
 {
 	const struct inode *inode = folio->mapping->host;
-	const struct fscrypt_inode_info *ci = inode->i_crypt_info;
+	const struct fscrypt_inode_info *ci = fscrypt_get_inode_info_raw(inode);
 	const unsigned int du_bits = ci->ci_data_unit_bits;
 	const unsigned int du_size = 1U << du_bits;
 	u64 index = ((u64)folio->index << (PAGE_SHIFT - du_bits)) +
 		    (offs >> du_bits);
 	size_t i;
@@ -303,12 +304,13 @@ int fscrypt_decrypt_block_inplace(const struct inode *inode, struct page *page,
 				  unsigned int len, unsigned int offs,
 				  u64 lblk_num)
 {
 	if (WARN_ON_ONCE(inode->i_sb->s_cop->supports_subblock_data_units))
 		return -EOPNOTSUPP;
-	return fscrypt_crypt_data_unit(inode->i_crypt_info, FS_DECRYPT,
-				       lblk_num, page, page, len, offs);
+	return fscrypt_crypt_data_unit(fscrypt_get_inode_info_raw(inode),
+				       FS_DECRYPT, lblk_num, page, page, len,
+				       offs);
 }
 EXPORT_SYMBOL(fscrypt_decrypt_block_inplace);
 
 /**
  * fscrypt_initialize() - allocate major buffers for fs encryption.
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index f9f6713e144f7..fb77ad1ca74a2 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -92,11 +92,11 @@ static inline bool fscrypt_is_dot_dotdot(const struct qstr *str)
  * Return: 0 on success, -errno on failure
  */
 int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
 			  u8 *out, unsigned int olen)
 {
-	const struct fscrypt_inode_info *ci = inode->i_crypt_info;
+	const struct fscrypt_inode_info *ci = fscrypt_get_inode_info_raw(inode);
 	struct crypto_sync_skcipher *tfm = ci->ci_enc_key.tfm;
 	SYNC_SKCIPHER_REQUEST_ON_STACK(req, tfm);
 	union fscrypt_iv iv;
 	struct scatterlist sg;
 	int err;
@@ -136,11 +136,11 @@ EXPORT_SYMBOL_GPL(fscrypt_fname_encrypt);
  */
 static int fname_decrypt(const struct inode *inode,
 			 const struct fscrypt_str *iname,
 			 struct fscrypt_str *oname)
 {
-	const struct fscrypt_inode_info *ci = inode->i_crypt_info;
+	const struct fscrypt_inode_info *ci = fscrypt_get_inode_info_raw(inode);
 	struct crypto_sync_skcipher *tfm = ci->ci_enc_key.tfm;
 	SYNC_SKCIPHER_REQUEST_ON_STACK(req, tfm);
 	union fscrypt_iv iv;
 	struct scatterlist src_sg, dst_sg;
 	int err;
@@ -272,12 +272,13 @@ bool __fscrypt_fname_encrypted_size(const union fscrypt_policy *policy,
  *	   fill out encrypted_len_ret with the length (up to max_len).
  */
 bool fscrypt_fname_encrypted_size(const struct inode *inode, u32 orig_len,
 				  u32 max_len, u32 *encrypted_len_ret)
 {
-	return __fscrypt_fname_encrypted_size(&inode->i_crypt_info->ci_policy,
-					      orig_len, max_len,
+	const struct fscrypt_inode_info *ci = fscrypt_get_inode_info_raw(inode);
+
+	return __fscrypt_fname_encrypted_size(&ci->ci_policy, orig_len, max_len,
 					      encrypted_len_ret);
 }
 EXPORT_SYMBOL_GPL(fscrypt_fname_encrypted_size);
 
 /**
@@ -541,11 +542,11 @@ EXPORT_SYMBOL_GPL(fscrypt_match_name);
  *
  * Return: the SipHash of @name using the hash key of @dir
  */
 u64 fscrypt_fname_siphash(const struct inode *dir, const struct qstr *name)
 {
-	const struct fscrypt_inode_info *ci = dir->i_crypt_info;
+	const struct fscrypt_inode_info *ci = fscrypt_get_inode_info_raw(dir);
 
 	WARN_ON_ONCE(!ci->ci_dirhash_key_initialized);
 
 	return siphash(name->name, name->len, &ci->ci_dirhash_key);
 }
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index e0b32ac841f76..7a5d4c168c49e 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -197,11 +197,11 @@ int fscrypt_prepare_setflags(struct inode *inode,
 	 */
 	if (IS_ENCRYPTED(inode) && (flags & ~oldflags & FS_CASEFOLD_FL)) {
 		err = fscrypt_require_key(inode);
 		if (err)
 			return err;
-		ci = inode->i_crypt_info;
+		ci = fscrypt_get_inode_info_raw(inode);
 		if (ci->ci_policy.version != FSCRYPT_POLICY_V2)
 			return -EINVAL;
 		mk = ci->ci_master_key;
 		down_read(&mk->mk_sem);
 		if (mk->mk_present)
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index caaff809765b2..5dee7c498bc8c 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -261,11 +261,11 @@ int fscrypt_derive_sw_secret(struct super_block *sb,
 	return err;
 }
 
 bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode)
 {
-	return inode->i_crypt_info->ci_inlinecrypt;
+	return fscrypt_get_inode_info_raw(inode)->ci_inlinecrypt;
 }
 EXPORT_SYMBOL_GPL(__fscrypt_inode_uses_inline_crypto);
 
 static void fscrypt_generate_dun(const struct fscrypt_inode_info *ci,
 				 u64 lblk_num,
@@ -305,11 +305,11 @@ void fscrypt_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
 	const struct fscrypt_inode_info *ci;
 	u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
 
 	if (!fscrypt_inode_uses_inline_crypto(inode))
 		return;
-	ci = inode->i_crypt_info;
+	ci = fscrypt_get_inode_info_raw(inode);
 
 	fscrypt_generate_dun(ci, first_lblk, dun);
 	bio_crypt_set_ctx(bio, ci->ci_enc_key.blk_key, dun, gfp_mask);
 }
 EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx);
@@ -383,26 +383,28 @@ EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx_bh);
  */
 bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
 			   u64 next_lblk)
 {
 	const struct bio_crypt_ctx *bc = bio->bi_crypt_context;
+	const struct fscrypt_inode_info *ci;
 	u64 next_dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
 
 	if (!!bc != fscrypt_inode_uses_inline_crypto(inode))
 		return false;
 	if (!bc)
 		return true;
+	ci = fscrypt_get_inode_info_raw(inode);
 
 	/*
 	 * 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);
 
 /**
@@ -500,11 +502,11 @@ u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks)
 		return nr_blocks;
 
 	if (nr_blocks <= 1)
 		return nr_blocks;
 
-	ci = inode->i_crypt_info;
+	ci = fscrypt_get_inode_info_raw(inode);
 	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. */
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 6ad30ae07c065..9d51f3500de37 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -725,11 +725,11 @@ const union fscrypt_policy *fscrypt_policy_to_inherit(struct inode *dir)
 
 	if (IS_ENCRYPTED(dir)) {
 		err = fscrypt_require_key(dir);
 		if (err)
 			return ERR_PTR(err);
-		return &dir->i_crypt_info->ci_policy;
+		return &fscrypt_get_inode_info_raw(dir)->ci_policy;
 	}
 
 	return fscrypt_get_dummy_policy(dir->i_sb);
 }
 
@@ -744,11 +744,11 @@ const union fscrypt_policy *fscrypt_policy_to_inherit(struct inode *dir)
  *
  * Return: size of the resulting context or a negative error code.
  */
 int fscrypt_context_for_new_inode(void *ctx, struct inode *inode)
 {
-	struct fscrypt_inode_info *ci = inode->i_crypt_info;
+	struct fscrypt_inode_info *ci = fscrypt_get_inode_info_raw(inode);
 
 	BUILD_BUG_ON(sizeof(union fscrypt_context) !=
 			FSCRYPT_SET_CONTEXT_MAX_SIZE);
 
 	/* fscrypt_prepare_new_inode() should have set up the key already. */
@@ -769,11 +769,11 @@ EXPORT_SYMBOL_GPL(fscrypt_context_for_new_inode);
  *
  * Return: 0 on success, -errno on failure
  */
 int fscrypt_set_context(struct inode *inode, void *fs_data)
 {
-	struct fscrypt_inode_info *ci = inode->i_crypt_info;
+	struct fscrypt_inode_info *ci;
 	union fscrypt_context ctx;
 	int ctxsize;
 
 	ctxsize = fscrypt_context_for_new_inode(&ctx, inode);
 	if (ctxsize < 0)
@@ -781,10 +781,11 @@ int fscrypt_set_context(struct inode *inode, void *fs_data)
 
 	/*
 	 * This may be the first time the inode number is available, so do any
 	 * delayed key setup that requires the inode number.
 	 */
+	ci = fscrypt_get_inode_info_raw(inode);
 	if (ci->ci_policy.version == FSCRYPT_POLICY_V2 &&
 	    (ci->ci_policy.v2.flags & FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32))
 		fscrypt_hash_inode_number(ci, ci->ci_master_key);
 
 	return inode->i_sb->s_cop->set_context(inode, &ctx, ctxsize, fs_data);
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 10dd161690a28..23c5198612d1a 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -193,10 +193,26 @@ struct fscrypt_operations {
 };
 
 int fscrypt_d_revalidate(struct inode *dir, const struct qstr *name,
 			 struct dentry *dentry, unsigned int flags);
 
+/*
+ * Load the inode's fscrypt info pointer, using a raw dereference.  Since this
+ * uses a raw dereference with no memory barrier, it is appropriate to use only
+ * when the caller knows the inode's key setup already happened, resulting in
+ * non-NULL fscrypt info.  E.g., the file contents en/decryption functions use
+ * this, since fscrypt_file_open() set up the key.
+ */
+static inline struct fscrypt_inode_info *
+fscrypt_get_inode_info_raw(const struct inode *inode)
+{
+	struct fscrypt_inode_info *ci = inode->i_crypt_info;
+
+	VFS_WARN_ON_ONCE(ci == NULL);
+	return ci;
+}
+
 static inline struct fscrypt_inode_info *
 fscrypt_get_inode_info(const struct inode *inode)
 {
 	/*
 	 * Pairs with the cmpxchg_release() in fscrypt_setup_encryption_info().
-- 
2.50.1

[PATCH v5 02/13] fscrypt: add support for info in fs-specific part of inode

[Eric Biggers]

Add an inode_info_offs field to struct fscrypt_operations, and update
fs/crypto/ to support it.  When set to a nonzero value, it specifies the
offset to the fscrypt_inode_info pointer within the filesystem-specific
part of the inode structure, to be used instead of inode::i_crypt_info.

Since this makes inode::i_crypt_info no longer necessarily used, update
comments that mentioned it.

This is a prerequisite for a later commit that removes
inode::i_crypt_info, saving memory and improving cache efficiency with
filesystems that don't support fscrypt.

Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 fs/crypto/fscrypt_private.h |  4 ++--
 fs/crypto/keysetup.c        | 43 ++++++++++++++++++++++---------------
 include/linux/fscrypt.h     | 22 +++++++++++++++----
 3 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index d8b485b9881c5..245e6b84aa174 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -247,12 +247,12 @@ struct fscrypt_prepared_key {
 
 /*
  * fscrypt_inode_info - the "encryption key" for an inode
  *
  * When an encrypted file's key is made available, an instance of this struct is
- * allocated and stored in ->i_crypt_info.  Once created, it remains until the
- * inode is evicted.
+ * allocated and a pointer to it is stored in the file's in-memory inode.  Once
+ * created, it remains until the inode is evicted.
  */
 struct fscrypt_inode_info {
 
 	/* The key in a form prepared for actual encryption/decryption */
 	struct fscrypt_prepared_key ci_enc_key;
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 4f3b9ecbfe4e6..c1f85715c2760 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -640,19 +640,20 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	res = setup_file_encryption_key(crypt_info, need_dirhash_key, &mk);
 	if (res)
 		goto out;
 
 	/*
-	 * For existing inodes, multiple tasks may race to set ->i_crypt_info.
-	 * So use cmpxchg_release().  This pairs with the smp_load_acquire() in
-	 * fscrypt_get_inode_info().  I.e., here we publish ->i_crypt_info with
-	 * a RELEASE barrier so that other tasks can ACQUIRE it.
+	 * For existing inodes, multiple tasks may race to set the inode's
+	 * fscrypt info pointer.  So use cmpxchg_release().  This pairs with the
+	 * smp_load_acquire() in fscrypt_get_inode_info().  I.e., publish the
+	 * pointer 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(fscrypt_inode_info_addr(inode), 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.
+		 * We won the race and set the inode's fscrypt info to our
+		 * crypt_info.  Now link it into the master key's inode list.
 		 */
 		if (mk) {
 			crypt_info->ci_master_key = mk;
 			refcount_inc(&mk->mk_active_refs);
 			spin_lock(&mk->mk_decrypted_inodes_lock);
@@ -679,17 +680,17 @@ fscrypt_setup_encryption_info(struct inode *inode,
  *		       unrecognized encryption context) the same way as the key
  *		       being unavailable, instead of returning an error.  Use
  *		       %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 the inode's encryption key, if it hasn't already been done.
  *
- * Note: unless ->i_crypt_info is already set, this isn't %GFP_NOFS-safe.  So
+ * Note: unless the key setup was already done, this isn't %GFP_NOFS-safe.  So
  * generally this shouldn't be called from within a filesystem transaction.
  *
- * Return: 0 if ->i_crypt_info was set or was already set, *or* if the
- *	   encryption key is unavailable.  (Use fscrypt_has_encryption_key() to
+ * Return: 0 if the key is now set up, *or* if it couldn't be set up because the
+ *	   needed master key is absent.  (Use fscrypt_has_encryption_key() to
  *	   distinguish these cases.)  Also can return another -errno code.
  */
 int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 {
 	int res;
@@ -739,23 +740,23 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
  * @dir: a possibly-encrypted directory
  * @inode: the new inode.  ->i_mode and ->i_blkbits must be set already.
  *	   ->i_ino doesn't need to be set yet.
  * @encrypt_ret: (output) set to %true if the new inode will be encrypted
  *
- * If the directory is encrypted, set up its ->i_crypt_info in preparation for
+ * If the directory is encrypted, set up its encryption key in preparation for
  * encrypting the name of the new file.  Also, if the new inode will be
- * encrypted, set up its ->i_crypt_info and set *encrypt_ret=true.
+ * encrypted, set up its encryption key too 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.  For this reason, ->i_ino
  * isn't required to be set yet, as the filesystem may not have set it yet.
  *
  * 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
+ * Return: 0 on success, -ENOKEY if a key needs to be set up for @dir or @inode
+ *	   but the needed master key is absent, or another -errno code
  */
 int fscrypt_prepare_new_inode(struct inode *dir, struct inode *inode,
 			      bool *encrypt_ret)
 {
 	const union fscrypt_policy *policy;
@@ -798,12 +799,20 @@ EXPORT_SYMBOL_GPL(fscrypt_prepare_new_inode);
  * Free the inode's fscrypt_inode_info.  Filesystems must call this when the
  * inode is being evicted.  An RCU grace period need not have elapsed yet.
  */
 void fscrypt_put_encryption_info(struct inode *inode)
 {
-	put_crypt_info(inode->i_crypt_info);
-	inode->i_crypt_info = NULL;
+	/*
+	 * Ideally we'd start with a lightweight IS_ENCRYPTED() check here
+	 * before proceeding to retrieve and check the pointer.  However, during
+	 * inode creation, the fscrypt_inode_info is set before S_ENCRYPTED.  If
+	 * an error occurs, it needs to be cleaned up regardless.
+	 */
+	struct fscrypt_inode_info **ci_addr = fscrypt_inode_info_addr(inode);
+
+	put_crypt_info(*ci_addr);
+	*ci_addr = NULL;
 }
 EXPORT_SYMBOL(fscrypt_put_encryption_info);
 
 /**
  * fscrypt_free_inode() - free an inode's fscrypt data requiring RCU delay
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 23c5198612d1a..d7ff53accbfef 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -59,10 +59,16 @@ struct fscrypt_name {
 
 #ifdef CONFIG_FS_ENCRYPTION
 
 /* Crypto operations for filesystems */
 struct fscrypt_operations {
+	/*
+	 * The offset of the pointer to struct fscrypt_inode_info in the
+	 * filesystem-specific part of the inode, relative to the beginning of
+	 * the common part of the inode (the 'struct inode').
+	 */
+	ptrdiff_t inode_info_offs;
 
 	/*
 	 * If set, then fs/crypto/ will allocate a global bounce page pool the
 	 * first time an encryption key is set up for a file.  The bounce page
 	 * pool is required by the following functions:
@@ -193,36 +199,44 @@ struct fscrypt_operations {
 };
 
 int fscrypt_d_revalidate(struct inode *dir, const struct qstr *name,
 			 struct dentry *dentry, unsigned int flags);
 
+static inline struct fscrypt_inode_info **
+fscrypt_inode_info_addr(const struct inode *inode)
+{
+	if (inode->i_sb->s_cop->inode_info_offs == 0)
+		return (struct fscrypt_inode_info **)&inode->i_crypt_info;
+	return (void *)inode + inode->i_sb->s_cop->inode_info_offs;
+}
+
 /*
  * Load the inode's fscrypt info pointer, using a raw dereference.  Since this
  * uses a raw dereference with no memory barrier, it is appropriate to use only
  * when the caller knows the inode's key setup already happened, resulting in
  * non-NULL fscrypt info.  E.g., the file contents en/decryption functions use
  * this, since fscrypt_file_open() set up the key.
  */
 static inline struct fscrypt_inode_info *
 fscrypt_get_inode_info_raw(const struct inode *inode)
 {
-	struct fscrypt_inode_info *ci = inode->i_crypt_info;
+	struct fscrypt_inode_info *ci = *fscrypt_inode_info_addr(inode);
 
 	VFS_WARN_ON_ONCE(ci == NULL);
 	return ci;
 }
 
 static inline struct fscrypt_inode_info *
 fscrypt_get_inode_info(const struct inode *inode)
 {
 	/*
 	 * Pairs with the cmpxchg_release() in fscrypt_setup_encryption_info().
-	 * I.e., another task may publish ->i_crypt_info concurrently, executing
-	 * a RELEASE barrier.  We need to use smp_load_acquire() here to safely
+	 * I.e., another task may publish the fscrypt info concurrently,
+	 * executing a RELEASE barrier.  Use smp_load_acquire() here to safely
 	 * ACQUIRE the memory the other task published.
 	 */
-	return smp_load_acquire(&inode->i_crypt_info);
+	return smp_load_acquire(fscrypt_inode_info_addr(inode));
 }
 
 /**
  * fscrypt_needs_contents_encryption() - check whether an inode needs
  *					 contents encryption
-- 
2.50.1

[PATCH v5 03/13] ext4: move crypt info pointer to fs-specific part of inode

[Eric Biggers]

Move the fscrypt_inode_info pointer into the filesystem-specific part of
the inode by adding the field ext4_inode_info::i_crypt_info and
configuring fscrypt_operations::inode_info_offs accordingly.

This is a prerequisite for a later commit that removes
inode::i_crypt_info, saving memory and improving cache efficiency with
filesystems that don't support fscrypt.

Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 fs/ext4/crypto.c | 2 ++
 fs/ext4/ext4.h   | 4 ++++
 fs/ext4/super.c  | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index 0a056d97e6402..cf0a0970c0956 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c
@@ -225,10 +225,12 @@ static bool ext4_has_stable_inodes(struct super_block *sb)
 {
 	return ext4_has_feature_stable_inodes(sb);
 }
 
 const struct fscrypt_operations ext4_cryptops = {
+	.inode_info_offs	= (int)offsetof(struct ext4_inode_info, i_crypt_info) -
+				  (int)offsetof(struct ext4_inode_info, vfs_inode),
 	.needs_bounce_pages	= 1,
 	.has_32bit_inodes	= 1,
 	.supports_subblock_data_units = 1,
 	.legacy_key_prefix	= "ext4:",
 	.get_context		= ext4_get_context,
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 01a6e2de7fc3e..c897109dadb15 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1180,10 +1180,14 @@ struct ext4_inode_info {
 
 	/* Precomputed uuid+inum+igen checksum for seeding inode checksums */
 	__u32 i_csum_seed;
 
 	kprojid_t i_projid;
+
+#ifdef CONFIG_FS_ENCRYPTION
+	struct fscrypt_inode_info *i_crypt_info;
+#endif
 };
 
 /*
  * File system states
  */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index c7d39da7e733b..0c3059ecce37c 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1468,10 +1468,13 @@ static void init_once(void *foo)
 	INIT_LIST_HEAD(&ei->i_orphan);
 	init_rwsem(&ei->xattr_sem);
 	init_rwsem(&ei->i_data_sem);
 	inode_init_once(&ei->vfs_inode);
 	ext4_fc_init_inode(&ei->vfs_inode);
+#ifdef CONFIG_FS_ENCRYPTION
+	ei->i_crypt_info = NULL;
+#endif
 }
 
 static int __init init_inodecache(void)
 {
 	ext4_inode_cachep = kmem_cache_create_usercopy("ext4_inode_cache",
-- 
2.50.1

[PATCH v5 04/13] f2fs: move crypt info pointer to fs-specific part of inode

[Eric Biggers]

Move the fscrypt_inode_info pointer into the filesystem-specific part of
the inode by adding the field f2fs_inode_info::i_crypt_info and
configuring fscrypt_operations::inode_info_offs accordingly.

This is a prerequisite for a later commit that removes
inode::i_crypt_info, saving memory and improving cache efficiency with
filesystems that don't support fscrypt.

Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 fs/f2fs/f2fs.h  | 3 +++
 fs/f2fs/super.c | 7 ++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 46be7560548ce..2f5c30c069c3c 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -905,10 +905,13 @@ struct f2fs_inode_info {
 	unsigned char i_compress_flag;		/* compress flag */
 	unsigned int i_cluster_size;		/* cluster size */
 
 	unsigned int atomic_write_cnt;
 	loff_t original_i_size;		/* original i_size before atomic write */
+#ifdef CONFIG_FS_ENCRYPTION
+	struct fscrypt_inode_info *i_crypt_info; /* filesystem encryption info */
+#endif
 };
 
 static inline void get_read_extent_info(struct extent_info *ext,
 					struct f2fs_extent *i_ext)
 {
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index e16c4e2830c29..b42b55280d9e3 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -478,10 +478,13 @@ static inline void adjust_unusable_cap_perc(struct f2fs_sb_info *sbi)
 static void init_once(void *foo)
 {
 	struct f2fs_inode_info *fi = (struct f2fs_inode_info *) foo;
 
 	inode_init_once(&fi->vfs_inode);
+#ifdef CONFIG_FS_ENCRYPTION
+	fi->i_crypt_info = NULL;
+#endif
 }
 
 #ifdef CONFIG_QUOTA
 static const char * const quotatypes[] = INITQFNAMES;
 #define QTYPE2NAME(t) (quotatypes[t])
@@ -3568,10 +3571,12 @@ static struct block_device **f2fs_get_devices(struct super_block *sb,
 	*num_devs = sbi->s_ndevs;
 	return devs;
 }
 
 static const struct fscrypt_operations f2fs_cryptops = {
+	.inode_info_offs	= (int)offsetof(struct f2fs_inode_info, i_crypt_info) -
+				  (int)offsetof(struct f2fs_inode_info, vfs_inode),
 	.needs_bounce_pages	= 1,
 	.has_32bit_inodes	= 1,
 	.supports_subblock_data_units = 1,
 	.legacy_key_prefix	= "f2fs:",
 	.get_context		= f2fs_get_context,
@@ -3579,11 +3584,11 @@ static const struct fscrypt_operations f2fs_cryptops = {
 	.get_dummy_policy	= f2fs_get_dummy_policy,
 	.empty_dir		= f2fs_empty_dir,
 	.has_stable_inodes	= f2fs_has_stable_inodes,
 	.get_devices		= f2fs_get_devices,
 };
-#endif
+#endif /* CONFIG_FS_ENCRYPTION */
 
 static struct inode *f2fs_nfs_get_inode(struct super_block *sb,
 		u64 ino, u32 generation)
 {
 	struct f2fs_sb_info *sbi = F2FS_SB(sb);
-- 
2.50.1

[PATCH v5 05/13] ubifs: move crypt info pointer to fs-specific part of inode

[Eric Biggers]

Move the fscrypt_inode_info pointer into the filesystem-specific part of
the inode by adding the field ubifs_inode::i_crypt_info and configuring
fscrypt_operations::inode_info_offs accordingly.

This is a prerequisite for a later commit that removes
inode::i_crypt_info, saving memory and improving cache efficiency with
filesystems that don't support fscrypt.

Note that the initialization of ubifs_inode::i_crypt_info to NULL on
inode allocation is handled by the memset() in ubifs_alloc_inode().

Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 fs/ubifs/crypto.c | 2 ++
 fs/ubifs/ubifs.h  | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/fs/ubifs/crypto.c b/fs/ubifs/crypto.c
index fb5ac358077b1..0b14d004a095b 100644
--- a/fs/ubifs/crypto.c
+++ b/fs/ubifs/crypto.c
@@ -86,10 +86,12 @@ int ubifs_decrypt(const struct inode *inode, struct ubifs_data_node *dn,
 
 	return 0;
 }
 
 const struct fscrypt_operations ubifs_crypt_operations = {
+	.inode_info_offs	= (int)offsetof(struct ubifs_inode, i_crypt_info) -
+				  (int)offsetof(struct ubifs_inode, vfs_inode),
 	.legacy_key_prefix	= "ubifs:",
 	.get_context		= ubifs_crypt_get_context,
 	.set_context		= ubifs_crypt_set_context,
 	.empty_dir		= ubifs_crypt_empty_dir,
 };
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 5db45c9e26ee0..49e50431741cd 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -363,10 +363,11 @@ struct ubifs_gced_idx_leb {
  * @compr_type: default compression type used for this inode
  * @last_page_read: page number of last page read (for bulk read)
  * @read_in_a_row: number of consecutive pages read in a row (for bulk read)
  * @data_len: length of the data attached to the inode
  * @data: inode's data
+ * @i_crypt_info: inode's fscrypt information
  *
  * @ui_mutex exists for two main reasons. At first it prevents inodes from
  * being written back while UBIFS changing them, being in the middle of an VFS
  * operation. This way UBIFS makes sure the inode fields are consistent. For
  * example, in 'ubifs_rename()' we change 4 inodes simultaneously, and
@@ -414,10 +415,13 @@ struct ubifs_inode {
 	int flags;
 	pgoff_t last_page_read;
 	pgoff_t read_in_a_row;
 	int data_len;
 	void *data;
+#ifdef CONFIG_FS_ENCRYPTION
+	struct fscrypt_inode_info *i_crypt_info;
+#endif
 };
 
 /**
  * struct ubifs_unclean_leb - records a LEB recovered under read-only mode.
  * @list: list
-- 
2.50.1

[PATCH v5 06/13] ceph: move crypt info pointer to fs-specific part of inode

[Eric Biggers]

Move the fscrypt_inode_info pointer into the filesystem-specific part of
the inode by adding the field ceph_inode_info::i_crypt_info and
configuring fscrypt_operations::inode_info_offs accordingly.

This is a prerequisite for a later commit that removes
inode::i_crypt_info, saving memory and improving cache efficiency with
filesystems that don't support fscrypt.

Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 fs/ceph/crypto.c | 2 ++
 fs/ceph/inode.c  | 1 +
 fs/ceph/super.h  | 1 +
 3 files changed, 4 insertions(+)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index cab7226192073..7026e794813ca 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -131,10 +131,12 @@ static const union fscrypt_policy *ceph_get_dummy_policy(struct super_block *sb)
 {
 	return ceph_sb_to_fs_client(sb)->fsc_dummy_enc_policy.policy;
 }
 
 static struct fscrypt_operations ceph_fscrypt_ops = {
+	.inode_info_offs	= (int)offsetof(struct ceph_inode_info, i_crypt_info) -
+				  (int)offsetof(struct ceph_inode_info, netfs.inode),
 	.needs_bounce_pages	= 1,
 	.get_context		= ceph_crypt_get_context,
 	.set_context		= ceph_crypt_set_context,
 	.get_dummy_policy	= ceph_get_dummy_policy,
 	.empty_dir		= ceph_crypt_empty_dir,
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index fc543075b827a..480cb3a1d639a 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -663,10 +663,11 @@ struct inode *ceph_alloc_inode(struct super_block *sb)
 
 	INIT_WORK(&ci->i_work, ceph_inode_work);
 	ci->i_work_mask = 0;
 	memset(&ci->i_btime, '\0', sizeof(ci->i_btime));
 #ifdef CONFIG_FS_ENCRYPTION
+	ci->i_crypt_info = NULL;
 	ci->fscrypt_auth = NULL;
 	ci->fscrypt_auth_len = 0;
 #endif
 	return &ci->netfs.inode;
 }
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index cf176aab0f823..25d8bacbcf440 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -461,10 +461,11 @@ struct ceph_inode_info {
 
 	struct work_struct i_work;
 	unsigned long  i_work_mask;
 
 #ifdef CONFIG_FS_ENCRYPTION
+	struct fscrypt_inode_info *i_crypt_info;
 	u32 fscrypt_auth_len;
 	u32 fscrypt_file_len;
 	u8 *fscrypt_auth;
 	u8 *fscrypt_file;
 #endif
-- 
2.50.1

[PATCH v5 07/13] fs: remove inode::i_crypt_info

[Eric Biggers]

Now that all fscrypt-capable filesystems store the pointer to
fscrypt_inode_info in the filesystem-specific part of the inode
structure, inode::i_crypt_info is no longer needed.  Update
fscrypt_inode_info_addr() to no longer support the fallback to
inode::i_crypt_info.  Finally, remove inode::i_crypt_info itself along
with the now-unnecessary forward declaration of fscrypt_inode_info.

The end result of the migration to the filesystem-specific pointer is
memory savings on CONFIG_FS_ENCRYPTION=y kernels for all filesystems
that don't support fscrypt.  Specifically, their in-memory inodes are
now smaller by the size of a pointer: either 4 or 8 bytes.

Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 include/linux/fs.h      | 5 -----
 include/linux/fscrypt.h | 8 ++++++--
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index d7ab4f96d7051..1dafa18169be6 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -70,11 +70,10 @@ struct vfsmount;
 struct cred;
 struct swap_info_struct;
 struct seq_file;
 struct workqueue_struct;
 struct iov_iter;
-struct fscrypt_inode_info;
 struct fscrypt_operations;
 struct fsverity_info;
 struct fsverity_operations;
 struct fsnotify_mark_connector;
 struct fsnotify_sb_info;
@@ -778,14 +777,10 @@ struct inode {
 	__u32			i_fsnotify_mask; /* all events this inode cares about */
 	/* 32-bit hole reserved for expanding i_fsnotify_mask */
 	struct fsnotify_mark_connector __rcu	*i_fsnotify_marks;
 #endif
 
-#ifdef CONFIG_FS_ENCRYPTION
-	struct fscrypt_inode_info	*i_crypt_info;
-#endif
-
 #ifdef CONFIG_FS_VERITY
 	struct fsverity_info	*i_verity_info;
 #endif
 
 	void			*i_private; /* fs or device private pointer */
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index d7ff53accbfef..516aba5b858b5 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -199,15 +199,19 @@ struct fscrypt_operations {
 };
 
 int fscrypt_d_revalidate(struct inode *dir, const struct qstr *name,
 			 struct dentry *dentry, unsigned int flags);
 
+/*
+ * Returns the address of the fscrypt info pointer within the
+ * filesystem-specific part of the inode.  (To save memory on filesystems that
+ * don't support fscrypt, a field in 'struct inode' itself is no longer used.)
+ */
 static inline struct fscrypt_inode_info **
 fscrypt_inode_info_addr(const struct inode *inode)
 {
-	if (inode->i_sb->s_cop->inode_info_offs == 0)
-		return (struct fscrypt_inode_info **)&inode->i_crypt_info;
+	VFS_WARN_ON_ONCE(inode->i_sb->s_cop->inode_info_offs == 0);
 	return (void *)inode + inode->i_sb->s_cop->inode_info_offs;
 }
 
 /*
  * Load the inode's fscrypt info pointer, using a raw dereference.  Since this
-- 
2.50.1

[PATCH v5 08/13] fsverity: add support for info in fs-specific part of inode

[Eric Biggers]

Add an inode_info_offs field to struct fsverity_operations, and update
fs/verity/ to support it.  When set to a nonzero value, it specifies the
offset to the fsverity_info pointer within the filesystem-specific part
of the inode structure, to be used instead of inode::i_verity_info.

Since this makes inode::i_verity_info no longer necessarily used, update
comments that mentioned it.

This is a prerequisite for a later commit that removes
inode::i_verity_info, saving memory and improving cache efficiency on
filesystems that don't support fsverity.

Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 fs/verity/enable.c           |  6 ++---
 fs/verity/fsverity_private.h |  9 ++++----
 fs/verity/open.c             | 23 ++++++++++---------
 fs/verity/verify.c           |  2 +-
 include/linux/fsverity.h     | 44 ++++++++++++++++++++++++++++--------
 5 files changed, 55 insertions(+), 29 deletions(-)

diff --git a/fs/verity/enable.c b/fs/verity/enable.c
index 503268cf42962..89eccc4becf90 100644
--- a/fs/verity/enable.c
+++ b/fs/verity/enable.c
@@ -282,13 +282,13 @@ static int enable_verity(struct file *filp,
 		fsverity_free_info(vi);
 	} else {
 		/* Successfully enabled verity */
 
 		/*
-		 * Readers can start using ->i_verity_info immediately, so it
-		 * can't be rolled back once set.  So don't set it until just
-		 * after the filesystem has successfully enabled verity.
+		 * Readers can start using the inode's verity info immediately,
+		 * so it can't be rolled back once set.  So don't set it until
+		 * just after the filesystem has successfully enabled verity.
 		 */
 		fsverity_set_info(inode, vi);
 	}
 out:
 	kfree(params.hashstate);
diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
index 5fe854a5b9ad3..bc1d887c532e7 100644
--- a/fs/verity/fsverity_private.h
+++ b/fs/verity/fsverity_private.h
@@ -61,14 +61,15 @@ struct merkle_tree_params {
 
 /*
  * fsverity_info - cached verity metadata for an inode
  *
  * When a verity file is first opened, an instance of this struct is allocated
- * and stored in ->i_verity_info; it remains until the inode is evicted.  It
- * caches information about the Merkle tree that's needed to efficiently verify
- * data read from the file.  It also caches the file digest.  The Merkle tree
- * pages themselves are not cached here, but the filesystem may cache them.
+ * and a pointer to it is stored in the file's in-memory inode.  It remains
+ * until the inode is evicted.  It caches information about the Merkle tree
+ * that's needed to efficiently verify data read from the file.  It also caches
+ * the file digest.  The Merkle tree pages themselves are not cached here, but
+ * the filesystem may cache them.
  */
 struct fsverity_info {
 	struct merkle_tree_params tree_params;
 	u8 root_hash[FS_VERITY_MAX_DIGEST_SIZE];
 	u8 file_digest[FS_VERITY_MAX_DIGEST_SIZE];
diff --git a/fs/verity/open.c b/fs/verity/open.c
index c561e130cd0c6..77b1c977af025 100644
--- a/fs/verity/open.c
+++ b/fs/verity/open.c
@@ -242,21 +242,21 @@ struct fsverity_info *fsverity_create_info(const struct inode *inode,
 }
 
 void fsverity_set_info(struct inode *inode, struct fsverity_info *vi)
 {
 	/*
-	 * Multiple tasks may race to set ->i_verity_info, so use
-	 * cmpxchg_release().  This pairs with the smp_load_acquire() in
-	 * fsverity_get_info().  I.e., here we publish ->i_verity_info with a
-	 * RELEASE barrier so that other tasks can ACQUIRE it.
+	 * Multiple tasks may race to set the inode's verity info pointer, so
+	 * use cmpxchg_release().  This pairs with the smp_load_acquire() in
+	 * fsverity_get_info().  I.e., publish the pointer with a RELEASE
+	 * barrier so that other tasks can ACQUIRE it.
 	 */
-	if (cmpxchg_release(&inode->i_verity_info, NULL, vi) != NULL) {
-		/* Lost the race, so free the fsverity_info we allocated. */
+	if (cmpxchg_release(fsverity_info_addr(inode), NULL, vi) != NULL) {
+		/* Lost the race, so free the verity info we allocated. */
 		fsverity_free_info(vi);
 		/*
-		 * Afterwards, the caller may access ->i_verity_info directly,
-		 * so make sure to ACQUIRE the winning fsverity_info.
+		 * Afterwards, the caller may access the inode's verity info
+		 * directly, so make sure to ACQUIRE the winning verity info.
 		 */
 		(void)fsverity_get_info(inode);
 	}
 }
 
@@ -348,11 +348,10 @@ int fsverity_get_descriptor(struct inode *inode,
 
 	*desc_ret = desc;
 	return 0;
 }
 
-/* Ensure the inode has an ->i_verity_info */
 static int ensure_verity_info(struct inode *inode)
 {
 	struct fsverity_info *vi = fsverity_get_info(inode);
 	struct fsverity_descriptor *desc;
 	int err;
@@ -393,12 +392,14 @@ int __fsverity_prepare_setattr(struct dentry *dentry, struct iattr *attr)
 }
 EXPORT_SYMBOL_GPL(__fsverity_prepare_setattr);
 
 void __fsverity_cleanup_inode(struct inode *inode)
 {
-	fsverity_free_info(inode->i_verity_info);
-	inode->i_verity_info = NULL;
+	struct fsverity_info **vi_addr = fsverity_info_addr(inode);
+
+	fsverity_free_info(*vi_addr);
+	*vi_addr = NULL;
 }
 EXPORT_SYMBOL_GPL(__fsverity_cleanup_inode);
 
 void __init fsverity_init_info_cache(void)
 {
diff --git a/fs/verity/verify.c b/fs/verity/verify.c
index a1f00c3fd3b27..affc307eb6a6b 100644
--- a/fs/verity/verify.c
+++ b/fs/verity/verify.c
@@ -243,11 +243,11 @@ verify_data_block(struct inode *inode, struct fsverity_info *vi,
 static bool
 verify_data_blocks(struct folio *data_folio, size_t len, size_t offset,
 		   unsigned long max_ra_pages)
 {
 	struct inode *inode = data_folio->mapping->host;
-	struct fsverity_info *vi = inode->i_verity_info;
+	struct fsverity_info *vi = *fsverity_info_addr(inode);
 	const unsigned int block_size = vi->tree_params.block_size;
 	u64 pos = (u64)data_folio->index << PAGE_SHIFT;
 
 	if (WARN_ON_ONCE(len <= 0 || !IS_ALIGNED(len | offset, block_size)))
 		return false;
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index 1eb7eae580be7..e0f132cb78393 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -26,10 +26,16 @@
 /* Arbitrary limit to bound the kmalloc() size.  Can be changed. */
 #define FS_VERITY_MAX_DESCRIPTOR_SIZE	16384
 
 /* Verity operations for filesystems */
 struct fsverity_operations {
+	/**
+	 * The offset of the pointer to struct fsverity_info in the
+	 * filesystem-specific part of the inode, relative to the beginning of
+	 * the common part of the inode (the 'struct inode').
+	 */
+	ptrdiff_t inode_info_offs;
 
 	/**
 	 * Begin enabling verity on the given file.
 	 *
 	 * @filp: a readonly file descriptor for the file
@@ -122,19 +128,37 @@ struct fsverity_operations {
 				       u64 pos, unsigned int size);
 };
 
 #ifdef CONFIG_FS_VERITY
 
+static inline struct fsverity_info **
+fsverity_info_addr(const struct inode *inode)
+{
+	if (inode->i_sb->s_vop->inode_info_offs == 0)
+		return (struct fsverity_info **)&inode->i_verity_info;
+	return (void *)inode + inode->i_sb->s_vop->inode_info_offs;
+}
+
 static inline struct fsverity_info *fsverity_get_info(const struct inode *inode)
 {
 	/*
-	 * Pairs with the cmpxchg_release() in fsverity_set_info().
-	 * I.e., another task may publish ->i_verity_info concurrently,
-	 * executing a RELEASE barrier.  We need to use smp_load_acquire() here
-	 * to safely ACQUIRE the memory the other task published.
+	 * Since this function can be called on inodes belonging to filesystems
+	 * that don't support fsverity at all, and fsverity_info_addr() doesn't
+	 * work on such filesystems, we have to start with an IS_VERITY() check.
+	 * Checking IS_VERITY() here is also useful to minimize the overhead of
+	 * fsverity_active() on non-verity files.
+	 */
+	if (!IS_VERITY(inode))
+		return NULL;
+
+	/*
+	 * Pairs with the cmpxchg_release() in fsverity_set_info().  I.e.,
+	 * another task may publish the inode's verity info concurrently,
+	 * executing a RELEASE barrier.  Use smp_load_acquire() here to safely
+	 * ACQUIRE the memory the other task published.
 	 */
-	return smp_load_acquire(&inode->i_verity_info);
+	return smp_load_acquire(fsverity_info_addr(inode));
 }
 
 /* enable.c */
 
 int fsverity_ioctl_enable(struct file *filp, const void __user *arg);
@@ -154,15 +178,15 @@ void __fsverity_cleanup_inode(struct inode *inode);
 
 /**
  * fsverity_cleanup_inode() - free the inode's verity info, if present
  * @inode: an inode being evicted
  *
- * Filesystems must call this on inode eviction to free ->i_verity_info.
+ * Filesystems must call this on inode eviction to free the inode's verity info.
  */
 static inline void fsverity_cleanup_inode(struct inode *inode)
 {
-	if (inode->i_verity_info)
+	if (*fsverity_info_addr(inode))
 		__fsverity_cleanup_inode(inode);
 }
 
 /* read_metadata.c */
 
@@ -265,16 +289,16 @@ static inline bool fsverity_verify_page(struct page *page)
 
 /**
  * fsverity_active() - do reads from the inode need to go through fs-verity?
  * @inode: inode to check
  *
- * This checks whether ->i_verity_info has been set.
+ * This checks whether the inode's verity info has been set.
  *
  * Filesystems call this from ->readahead() to check whether the pages need to
  * be verified or not.  Don't use IS_VERITY() for this purpose; it's subject to
  * a race condition where the file is being read concurrently with
- * FS_IOC_ENABLE_VERITY completing.  (S_VERITY is set before ->i_verity_info.)
+ * FS_IOC_ENABLE_VERITY completing.  (S_VERITY is set before the verity info.)
  *
  * Return: true if reads need to go through fs-verity, otherwise false
  */
 static inline bool fsverity_active(const struct inode *inode)
 {
@@ -285,11 +309,11 @@ static inline bool fsverity_active(const struct inode *inode)
  * fsverity_file_open() - prepare to open a verity file
  * @inode: the inode being opened
  * @filp: the struct file being set up
  *
  * When opening a verity file, deny the open if it is for writing.  Otherwise,
- * set up the inode's ->i_verity_info if not already done.
+ * set up the inode's verity info if not already done.
  *
  * When combined with fscrypt, this must be called after fscrypt_file_open().
  * Otherwise, we won't have the key set up to decrypt the verity metadata.
  *
  * Return: 0 on success, -errno on failure
-- 
2.50.1

[PATCH v5 09/13] ext4: move verity info pointer to fs-specific part of inode

[Eric Biggers]

Move the fsverity_info pointer into the filesystem-specific part of the
inode by adding the field ext4_inode_info::i_verity_info and configuring
fsverity_operations::inode_info_offs accordingly.

This is a prerequisite for a later commit that removes
inode::i_verity_info, saving memory and improving cache efficiency on
filesystems that don't support fsverity.

Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 fs/ext4/ext4.h   | 4 ++++
 fs/ext4/super.c  | 3 +++
 fs/ext4/verity.c | 2 ++
 3 files changed, 9 insertions(+)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index c897109dadb15..6cb784a56b3ba 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1184,10 +1184,14 @@ struct ext4_inode_info {
 	kprojid_t i_projid;
 
 #ifdef CONFIG_FS_ENCRYPTION
 	struct fscrypt_inode_info *i_crypt_info;
 #endif
+
+#ifdef CONFIG_FS_VERITY
+	struct fsverity_info *i_verity_info;
+#endif
 };
 
 /*
  * File system states
  */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 0c3059ecce37c..46138a6cb32a3 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1471,10 +1471,13 @@ static void init_once(void *foo)
 	inode_init_once(&ei->vfs_inode);
 	ext4_fc_init_inode(&ei->vfs_inode);
 #ifdef CONFIG_FS_ENCRYPTION
 	ei->i_crypt_info = NULL;
 #endif
+#ifdef CONFIG_FS_VERITY
+	ei->i_verity_info = NULL;
+#endif
 }
 
 static int __init init_inodecache(void)
 {
 	ext4_inode_cachep = kmem_cache_create_usercopy("ext4_inode_cache",
diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
index d9203228ce979..b0acb0c503137 100644
--- a/fs/ext4/verity.c
+++ b/fs/ext4/verity.c
@@ -387,10 +387,12 @@ static int ext4_write_merkle_tree_block(struct inode *inode, const void *buf,
 
 	return pagecache_write(inode, buf, size, pos);
 }
 
 const struct fsverity_operations ext4_verityops = {
+	.inode_info_offs	= (int)offsetof(struct ext4_inode_info, i_verity_info) -
+				  (int)offsetof(struct ext4_inode_info, vfs_inode),
 	.begin_enable_verity	= ext4_begin_enable_verity,
 	.end_enable_verity	= ext4_end_enable_verity,
 	.get_verity_descriptor	= ext4_get_verity_descriptor,
 	.read_merkle_tree_page	= ext4_read_merkle_tree_page,
 	.write_merkle_tree_block = ext4_write_merkle_tree_block,
-- 
2.50.1

[PATCH v5 10/13] f2fs: move verity info pointer to fs-specific part of inode

[Eric Biggers]

Move the fsverity_info pointer into the filesystem-specific part of the
inode by adding the field f2fs_inode_info::i_verity_info and configuring
fsverity_operations::inode_info_offs accordingly.

This is a prerequisite for a later commit that removes
inode::i_verity_info, saving memory and improving cache efficiency on
filesystems that don't support fsverity.

Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 fs/f2fs/f2fs.h   | 3 +++
 fs/f2fs/super.c  | 3 +++
 fs/f2fs/verity.c | 2 ++
 3 files changed, 8 insertions(+)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 2f5c30c069c3c..6e465bbc85ee5 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -908,10 +908,13 @@ struct f2fs_inode_info {
 	unsigned int atomic_write_cnt;
 	loff_t original_i_size;		/* original i_size before atomic write */
 #ifdef CONFIG_FS_ENCRYPTION
 	struct fscrypt_inode_info *i_crypt_info; /* filesystem encryption info */
 #endif
+#ifdef CONFIG_FS_VERITY
+	struct fsverity_info *i_verity_info; /* filesystem verity info */
+#endif
 };
 
 static inline void get_read_extent_info(struct extent_info *ext,
 					struct f2fs_extent *i_ext)
 {
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index b42b55280d9e3..1db024b20e29b 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -481,10 +481,13 @@ static void init_once(void *foo)
 
 	inode_init_once(&fi->vfs_inode);
 #ifdef CONFIG_FS_ENCRYPTION
 	fi->i_crypt_info = NULL;
 #endif
+#ifdef CONFIG_FS_VERITY
+	fi->i_verity_info = NULL;
+#endif
 }
 
 #ifdef CONFIG_QUOTA
 static const char * const quotatypes[] = INITQFNAMES;
 #define QTYPE2NAME(t) (quotatypes[t])
diff --git a/fs/f2fs/verity.c b/fs/f2fs/verity.c
index 2287f238ae09e..f0ab9a3c7a82b 100644
--- a/fs/f2fs/verity.c
+++ b/fs/f2fs/verity.c
@@ -285,10 +285,12 @@ static int f2fs_write_merkle_tree_block(struct inode *inode, const void *buf,
 
 	return pagecache_write(inode, buf, size, pos);
 }
 
 const struct fsverity_operations f2fs_verityops = {
+	.inode_info_offs	= (int)offsetof(struct f2fs_inode_info, i_verity_info) -
+				  (int)offsetof(struct f2fs_inode_info, vfs_inode),
 	.begin_enable_verity	= f2fs_begin_enable_verity,
 	.end_enable_verity	= f2fs_end_enable_verity,
 	.get_verity_descriptor	= f2fs_get_verity_descriptor,
 	.read_merkle_tree_page	= f2fs_read_merkle_tree_page,
 	.write_merkle_tree_block = f2fs_write_merkle_tree_block,
-- 
2.50.1

[PATCH v5 11/13] btrfs: move verity info pointer to fs-specific part of inode

[Eric Biggers]

Move the fsverity_info pointer into the filesystem-specific part of the
inode by adding the field btrfs_inode::i_verity_info and configuring
fsverity_operations::inode_info_offs accordingly.

This is a prerequisite for a later commit that removes
inode::i_verity_info, saving memory and improving cache efficiency on
filesystems that don't support fsverity.

Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 fs/btrfs/btrfs_inode.h | 5 +++++
 fs/btrfs/inode.c       | 3 +++
 fs/btrfs/verity.c      | 2 ++
 3 files changed, 10 insertions(+)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index b99fb02732929..2c9489497cbea 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -336,10 +336,15 @@ struct btrfs_inode {
 
 	/* Hook into fs_info->delayed_iputs */
 	struct list_head delayed_iput;
 
 	struct rw_semaphore i_mmap_lock;
+
+#ifdef CONFIG_FS_VERITY
+	struct fsverity_info *i_verity_info;
+#endif
+
 	struct inode vfs_inode;
 };
 
 static inline u64 btrfs_get_first_dir_index_to_log(const struct btrfs_inode *inode)
 {
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b77dd22b8cdbe..de722b232ec14 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -7959,10 +7959,13 @@ int btrfs_drop_inode(struct inode *inode)
 static void init_once(void *foo)
 {
 	struct btrfs_inode *ei = foo;
 
 	inode_init_once(&ei->vfs_inode);
+#ifdef CONFIG_FS_VERITY
+	ei->i_verity_info = NULL;
+#endif
 }
 
 void __cold btrfs_destroy_cachep(void)
 {
 	/*
diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
index b7a96a005487e..4633cbcfcdb90 100644
--- a/fs/btrfs/verity.c
+++ b/fs/btrfs/verity.c
@@ -800,10 +800,12 @@ static int btrfs_write_merkle_tree_block(struct inode *inode, const void *buf,
 	return write_key_bytes(BTRFS_I(inode), BTRFS_VERITY_MERKLE_ITEM_KEY,
 			       pos, buf, size);
 }
 
 const struct fsverity_operations btrfs_verityops = {
+	.inode_info_offs         = (int)offsetof(struct btrfs_inode, i_verity_info) -
+				   (int)offsetof(struct btrfs_inode, vfs_inode),
 	.begin_enable_verity     = btrfs_begin_enable_verity,
 	.end_enable_verity       = btrfs_end_enable_verity,
 	.get_verity_descriptor   = btrfs_get_verity_descriptor,
 	.read_merkle_tree_page   = btrfs_read_merkle_tree_page,
 	.write_merkle_tree_block = btrfs_write_merkle_tree_block,
-- 
2.50.1

[PATCH v5 12/13] fs: remove inode::i_verity_info

[Eric Biggers]

Now that all fsverity-capable filesystems store the pointer to
fsverity_info in the filesystem-specific part of the inode structure,
inode::i_verity_info is no longer needed.  Update fsverity_info_addr()
to no longer support the fallback to inode::i_verity_info.  Finally,
remove inode::i_verity_info itself, and move the forward declaration of
struct fsverity_info from fs.h (which no longer needs it) to fsverity.h.

The end result of the migration to the filesystem-specific pointer is
memory savings on CONFIG_FS_VERITY=y kernels for all filesystems that
don't support fsverity.  Specifically, their in-memory inodes are now
smaller by the size of a pointer: either 4 or 8 bytes.

Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 include/linux/fs.h       |  5 -----
 include/linux/fsverity.h | 10 ++++++++--
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1dafa18169be6..12ecc6b0e6f96 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -71,11 +71,10 @@ struct cred;
 struct swap_info_struct;
 struct seq_file;
 struct workqueue_struct;
 struct iov_iter;
 struct fscrypt_operations;
-struct fsverity_info;
 struct fsverity_operations;
 struct fsnotify_mark_connector;
 struct fsnotify_sb_info;
 struct fs_context;
 struct fs_parameter_spec;
@@ -777,14 +776,10 @@ struct inode {
 	__u32			i_fsnotify_mask; /* all events this inode cares about */
 	/* 32-bit hole reserved for expanding i_fsnotify_mask */
 	struct fsnotify_mark_connector __rcu	*i_fsnotify_marks;
 #endif
 
-#ifdef CONFIG_FS_VERITY
-	struct fsverity_info	*i_verity_info;
-#endif
-
 	void			*i_private; /* fs or device private pointer */
 } __randomize_layout;
 
 static inline void inode_set_cached_link(struct inode *inode, char *link, int linklen)
 {
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index e0f132cb78393..844f7b8b56bbc 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -24,10 +24,12 @@
 #define FS_VERITY_MAX_DIGEST_SIZE	SHA512_DIGEST_SIZE
 
 /* Arbitrary limit to bound the kmalloc() size.  Can be changed. */
 #define FS_VERITY_MAX_DESCRIPTOR_SIZE	16384
 
+struct fsverity_info;
+
 /* Verity operations for filesystems */
 struct fsverity_operations {
 	/**
 	 * The offset of the pointer to struct fsverity_info in the
 	 * filesystem-specific part of the inode, relative to the beginning of
@@ -128,15 +130,19 @@ struct fsverity_operations {
 				       u64 pos, unsigned int size);
 };
 
 #ifdef CONFIG_FS_VERITY
 
+/*
+ * Returns the address of the verity info pointer within the filesystem-specific
+ * part of the inode.  (To save memory on filesystems that don't support
+ * fsverity, a field in 'struct inode' itself is no longer used.)
+ */
 static inline struct fsverity_info **
 fsverity_info_addr(const struct inode *inode)
 {
-	if (inode->i_sb->s_vop->inode_info_offs == 0)
-		return (struct fsverity_info **)&inode->i_verity_info;
+	VFS_WARN_ON_ONCE(inode->i_sb->s_vop->inode_info_offs == 0);
 	return (void *)inode + inode->i_sb->s_vop->inode_info_offs;
 }
 
 static inline struct fsverity_info *fsverity_get_info(const struct inode *inode)
 {
-- 
2.50.1

[PATCH v5 13/13] fsverity: check IS_VERITY() in fsverity_cleanup_inode()

[Eric Biggers]

Since getting the address of the fsverity_info has gotten a bit more
expensive, make fsverity_cleanup_inode() check for IS_VERITY() instead.
This avoids adding more overhead to non-verity files.

This assumes that verity info is never set when !IS_VERITY(), which is
currently true, but add a VFS_WARN_ON_ONCE() that asserts that.  (This
of course defeats the optimization, but only when CONFIG_VFS_DEBUG=y.)

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 include/linux/fsverity.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index 844f7b8b56bbc..5bc7280425a71 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -188,12 +188,19 @@ void __fsverity_cleanup_inode(struct inode *inode);
  *
  * Filesystems must call this on inode eviction to free the inode's verity info.
  */
 static inline void fsverity_cleanup_inode(struct inode *inode)
 {
-	if (*fsverity_info_addr(inode))
+	/*
+	 * Only IS_VERITY() inodes can have verity info, so start by checking
+	 * for IS_VERITY() (which is faster than retrieving the pointer to the
+	 * verity info).  This minimizes overhead for non-verity inodes.
+	 */
+	if (IS_VERITY(inode))
 		__fsverity_cleanup_inode(inode);
+	else
+		VFS_WARN_ON_ONCE(*fsverity_info_addr(inode) != NULL);
 }
 
 /* read_metadata.c */
 
 int fsverity_ioctl_read_metadata(struct file *filp, const void __user *uarg);
-- 
2.50.1

Re: [PATCH v5 00/13] Move fscrypt and fsverity info out of struct inode

[Christian Brauner]

On Sun, Aug 10, 2025 at 12:56:53AM -0700, Eric Biggers wrote:
> This is a cleaned-up implementation of moving the i_crypt_info and
> i_verity_info pointers out of 'struct inode' and into the fs-specific
> part of the inode, as proposed previously by Christian at
> https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> 
> The high-level concept is still the same: fs/crypto/ and fs/verity/
> locate the pointer by adding an offset to the address of struct inode.
> The offset is retrieved from fscrypt_operations or fsverity_operations.
> 
> I've cleaned up a lot of the details, including:
> - Grouped changes into patches differently
> - Rewrote commit messages and comments to be clearer
> - Adjusted code formatting to be consistent with existing code
> - Removed unneeded #ifdefs
> - Improved choice and location of VFS_WARN_ON_ONCE() statements
> - Added missing kerneldoc for ubifs_inode::i_crypt_info
> - Moved field initialization to init_once functions when they exist
> - Improved ceph offset calculation and removed unneeded static_asserts
> - fsverity_get_info() now checks IS_VERITY() instead of v_ops
> - fscrypt_put_encryption_info() no longer checks IS_ENCRYPTED(), since I
>   no longer think it's actually correct there.
> - verity_data_blocks() now keeps doing a raw dereference
> - Dropped fscrypt_set_inode_info() 
> - Renamed some functions
> - Do offset calculation using int, so we don't rely on unsigned overflow
> - And more.
> 
> For v4 and earlier, see
> https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> 
> I'd like to take this series through the fscrypt tree for 6.18.
> (fsverity normally has a separate tree, but by choosing just one tree
> for this, we'll avoid conflicts in some places.)

Woh woh. First, I had a cleaned up version ready for v6.18 so if you
plan on taking over someone's series and resend then maybe ask the
author first whether that's ok or not. I haven't seen you do that. You
just caused duplicated work for no reason.

And second general infrastructure changes that touch multiple fses and
generic fs infrastructure I very much want to go through VFS trees.
We'll simply use a shared tree.

Re: [PATCH v5 00/13] Move fscrypt and fsverity info out of struct inode

[Eric Biggers]

On Sun, Aug 10, 2025 at 10:47:32AM +0200, Christian Brauner wrote:
> On Sun, Aug 10, 2025 at 12:56:53AM -0700, Eric Biggers wrote:
> > This is a cleaned-up implementation of moving the i_crypt_info and
> > i_verity_info pointers out of 'struct inode' and into the fs-specific
> > part of the inode, as proposed previously by Christian at
> > https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> > 
> > The high-level concept is still the same: fs/crypto/ and fs/verity/
> > locate the pointer by adding an offset to the address of struct inode.
> > The offset is retrieved from fscrypt_operations or fsverity_operations.
> > 
> > I've cleaned up a lot of the details, including:
> > - Grouped changes into patches differently
> > - Rewrote commit messages and comments to be clearer
> > - Adjusted code formatting to be consistent with existing code
> > - Removed unneeded #ifdefs
> > - Improved choice and location of VFS_WARN_ON_ONCE() statements
> > - Added missing kerneldoc for ubifs_inode::i_crypt_info
> > - Moved field initialization to init_once functions when they exist
> > - Improved ceph offset calculation and removed unneeded static_asserts
> > - fsverity_get_info() now checks IS_VERITY() instead of v_ops
> > - fscrypt_put_encryption_info() no longer checks IS_ENCRYPTED(), since I
> >   no longer think it's actually correct there.
> > - verity_data_blocks() now keeps doing a raw dereference
> > - Dropped fscrypt_set_inode_info() 
> > - Renamed some functions
> > - Do offset calculation using int, so we don't rely on unsigned overflow
> > - And more.
> > 
> > For v4 and earlier, see
> > https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> > 
> > I'd like to take this series through the fscrypt tree for 6.18.
> > (fsverity normally has a separate tree, but by choosing just one tree
> > for this, we'll avoid conflicts in some places.)
> 
> Woh woh. First, I had a cleaned up version ready for v6.18 so if you
> plan on taking over someone's series and resend then maybe ask the
> author first whether that's ok or not. I haven't seen you do that. You
> just caused duplicated work for no reason.

Ah, sorry about that.  When I started looking at it again yesterday
there turned out to be way too many cleanups and fixes I wanted to make
(beyond the comments I gave earlier), and I hadn't seen activity from
you on it in a while.  So I figured it would be easier to just send a
series myself.  But I should have asked you first, sorry.

> And second general infrastructure changes that touch multiple fses and
> generic fs infrastructure I very much want to go through VFS trees.
> We'll simply use a shared tree.

So you'd like to discontinue the fscrypt and fsverity trees?  That's
what they are for: general infrastructure shared by multiple
filesystems.  Or is this comment just for this series in particular,
presumably because it touches 'struct inode'?

- Eric

Re: [PATCH v5 00/13] Move fscrypt and fsverity info out of struct inode

[Christoph Hellwig]

On Sun, Aug 10, 2025 at 12:56:53AM -0700, Eric Biggers wrote:
> This is a cleaned-up implementation of moving the i_crypt_info and
> i_verity_info pointers out of 'struct inode' and into the fs-specific
> part of the inode, as proposed previously by Christian at
> https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/

I would really much prefer to move fscrypt to use a hash lookup instead
of bloating all inodes for a each file system supporting it, even if
very few files on very few file systems are using it.  With the fsverity
xfs series posted again this is becoming personal :)

You mentioned you were looking into it but didn't like the rhashtable
API.  My offer to help with that still stands.

Re: [PATCH v5 00/13] Move fscrypt and fsverity info out of struct inode

[Eric Biggers]

On Sun, Aug 10, 2025 at 07:49:53AM -0700, Christoph Hellwig wrote:
> On Sun, Aug 10, 2025 at 12:56:53AM -0700, Eric Biggers wrote:
> > This is a cleaned-up implementation of moving the i_crypt_info and
> > i_verity_info pointers out of 'struct inode' and into the fs-specific
> > part of the inode, as proposed previously by Christian at
> > https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> 
> I would really much prefer to move fscrypt to use a hash lookup instead
> of bloating all inodes for a each file system supporting it, even if
> very few files on very few file systems are using it.  With the fsverity
> xfs series posted again this is becoming personal :)
> 
> You mentioned you were looking into it but didn't like the rhashtable
> API.  My offer to help with that still stands.

I assume you actually still mean fsverity, not fscrypt.  First, it would
be helpful not to use one solution for fscrypt and a totally different
solution for fsverity, as that would increase the maintenance cost well
beyond that of either solution individually.

Second, the fsverity info can be loaded very frequently.  For example,
curently it's loaded for each 4K data block processed.  Also, there
*are* use cases in which most files on the filesystem have fsverity
enabled.  Not super common, but they exist.

Yes, the rhashtable is technically O(1), but its code is a mess, both
source code and generated code.  Let's look at the x86_64 code generated
by the fs-specific field solution in this patchset:

     a74:   48 8b 47 28             mov    0x28(%rdi),%rax
     a78:   48 8b 80 a8 00 00 00    mov    0xa8(%rax),%rax
     a7f:   48 8b 00                mov    (%rax),%rax
     a82:   48 8b 04 07             mov    (%rdi,%rax,1),%rax

A few dependent loads, but nice and simple.

Now let's look at the rhashtable version of the load.  (I used the
'struct inode *' as the rhashtable key):

    ffffffff81487f00 <fsverity_get_info>:
    ffffffff81487f00:	f3 0f 1e fa          	endbr64
    ffffffff81487f04:	55                   	push   %rbp
    ffffffff81487f05:	48 89 e5             	mov    %rsp,%rbp
    ffffffff81487f08:	41 57                	push   %r15
    ffffffff81487f0a:	41 56                	push   %r14
    ffffffff81487f0c:	41 55                	push   %r13
    ffffffff81487f0e:	41 54                	push   %r12
    ffffffff81487f10:	53                   	push   %rbx
    ffffffff81487f11:	48 83 ec 10          	sub    $0x10,%rsp
    ffffffff81487f15:	48 89 7d d0          	mov    %rdi,-0x30(%rbp)
    ffffffff81487f19:	e8 d2 c6 e9 ff       	call   ffffffff813245f0 <__rcu_read_lock>
    ffffffff81487f1e:	48 8b 1d 5b 15 c9 00 	mov    0xc9155b(%rip),%rbx        # ffffffff82119480 <fsverity_info_hashtable>
    ffffffff81487f25:	8b 43 08             	mov    0x8(%rbx),%eax
    ffffffff81487f28:	8b 55 d4             	mov    -0x2c(%rbp),%edx
    ffffffff81487f2b:	8b 4d d0             	mov    -0x30(%rbp),%ecx
    ffffffff81487f2e:	8b 3b                	mov    (%rbx),%edi
    ffffffff81487f30:	2d 09 41 52 21       	sub    $0x21524109,%eax
    ffffffff81487f35:	01 c2                	add    %eax,%edx
    ffffffff81487f37:	01 c1                	add    %eax,%ecx
    ffffffff81487f39:	89 d6                	mov    %edx,%esi
    ffffffff81487f3b:	31 d0                	xor    %edx,%eax
    ffffffff81487f3d:	c1 c6 0e             	rol    $0xe,%esi
    ffffffff81487f40:	29 f0                	sub    %esi,%eax
    ffffffff81487f42:	89 c6                	mov    %eax,%esi
    ffffffff81487f44:	31 c1                	xor    %eax,%ecx
    ffffffff81487f46:	c1 c6 0b             	rol    $0xb,%esi
    ffffffff81487f49:	29 f1                	sub    %esi,%ecx
    ffffffff81487f4b:	89 ce                	mov    %ecx,%esi
    ffffffff81487f4d:	31 ca                	xor    %ecx,%edx
    ffffffff81487f4f:	c1 ce 07             	ror    $0x7,%esi
    ffffffff81487f52:	29 f2                	sub    %esi,%edx
    ffffffff81487f54:	89 d6                	mov    %edx,%esi
    ffffffff81487f56:	31 d0                	xor    %edx,%eax
    ffffffff81487f58:	c1 c6 10             	rol    $0x10,%esi
    ffffffff81487f5b:	29 f0                	sub    %esi,%eax
    ffffffff81487f5d:	89 c6                	mov    %eax,%esi
    ffffffff81487f5f:	31 c1                	xor    %eax,%ecx
    ffffffff81487f61:	c1 c6 04             	rol    $0x4,%esi
    ffffffff81487f64:	29 f1                	sub    %esi,%ecx
    ffffffff81487f66:	8d 77 ff             	lea    -0x1(%rdi),%esi
    ffffffff81487f69:	31 ca                	xor    %ecx,%edx
    ffffffff81487f6b:	c1 c1 0e             	rol    $0xe,%ecx
    ffffffff81487f6e:	29 ca                	sub    %ecx,%edx
    ffffffff81487f70:	31 d0                	xor    %edx,%eax
    ffffffff81487f72:	c1 ca 08             	ror    $0x8,%edx
    ffffffff81487f75:	29 d0                	sub    %edx,%eax
    ffffffff81487f77:	21 c6                	and    %eax,%esi
    ffffffff81487f79:	8b 43 04             	mov    0x4(%rbx),%eax
    ffffffff81487f7c:	4c 8d 6c f3 40       	lea    0x40(%rbx,%rsi,8),%r13
    ffffffff81487f81:	85 c0                	test   %eax,%eax
    ffffffff81487f83:	0f 85 8c 00 00 00    	jne    ffffffff81488015 <fsverity_get_info+0x115>
    ffffffff81487f89:	49 8b 4d 00          	mov    0x0(%r13),%rcx
    ffffffff81487f8d:	48 83 e1 fe          	and    $0xfffffffffffffffe,%rcx
    ffffffff81487f91:	74 71                	je     ffffffff81488004 <fsverity_get_info+0x104>
    ffffffff81487f93:	0f b7 05 f8 14 c9 00 	movzwl 0xc914f8(%rip),%eax        # ffffffff82119492 <fsverity_info_hashtable+0x12>
    ffffffff81487f9a:	44 0f b7 25 f2 14 c9 	movzwl 0xc914f2(%rip),%r12d        # ffffffff82119494 <fsverity_info_hashtable+0x14>
    ffffffff81487fa1:	00 
    ffffffff81487fa2:	49 89 ce             	mov    %rcx,%r14
    ffffffff81487fa5:	44 0f b7 3d e9 14 c9 	movzwl 0xc914e9(%rip),%r15d        # ffffffff82119496 <fsverity_info_hashtable+0x16>
    ffffffff81487fac:	00 
    ffffffff81487fad:	48 89 45 c8          	mov    %rax,-0x38(%rbp)
    ffffffff81487fb1:	4d 29 fc             	sub    %r15,%r12
    ffffffff81487fb4:	48 8b 55 c8          	mov    -0x38(%rbp),%rdx
    ffffffff81487fb8:	4b 8d 3c 26          	lea    (%r14,%r12,1),%rdi
    ffffffff81487fbc:	48 8d 75 d0          	lea    -0x30(%rbp),%rsi
    ffffffff81487fc0:	e8 2b 53 4b 00       	call   ffffffff8193d2f0 <memcmp>
    ffffffff81487fc5:	85 c0                	test   %eax,%eax
    ffffffff81487fc7:	75 26                	jne    ffffffff81487fef <fsverity_get_info+0xef>
    ffffffff81487fc9:	4d 85 f6             	test   %r14,%r14
    ffffffff81487fcc:	74 43                	je     ffffffff81488011 <fsverity_get_info+0x111>
    ffffffff81487fce:	4c 89 f3             	mov    %r14,%rbx
    ffffffff81487fd1:	4c 29 fb             	sub    %r15,%rbx
    ffffffff81487fd4:	e8 67 18 ea ff       	call   ffffffff81329840 <__rcu_read_unlock>
    ffffffff81487fd9:	48 89 d8             	mov    %rbx,%rax
    ffffffff81487fdc:	48 83 c4 10          	add    $0x10,%rsp
    ffffffff81487fe0:	5b                   	pop    %rbx
    ffffffff81487fe1:	41 5c                	pop    %r12
    ffffffff81487fe3:	41 5d                	pop    %r13
    ffffffff81487fe5:	41 5e                	pop    %r14
    ffffffff81487fe7:	41 5f                	pop    %r15
    ffffffff81487fe9:	5d                   	pop    %rbp
    ffffffff81487fea:	e9 01 53 4d 00       	jmp    ffffffff8195d2f0 <__pi___x86_return_thunk>
    ffffffff81487fef:	4d 8b 36             	mov    (%r14),%r14
    ffffffff81487ff2:	41 f6 c6 01          	test   $0x1,%r14b
    ffffffff81487ff6:	74 bc                	je     ffffffff81487fb4 <fsverity_get_info+0xb4>
    ffffffff81487ff8:	4c 89 e8             	mov    %r13,%rax
    ffffffff81487ffb:	48 83 c8 01          	or     $0x1,%rax
    ffffffff81487fff:	49 39 c6             	cmp    %rax,%r14
    ffffffff81488002:	75 85                	jne    ffffffff81487f89 <fsverity_get_info+0x89>
    ffffffff81488004:	48 8b 5b 30          	mov    0x30(%rbx),%rbx
    ffffffff81488008:	48 85 db             	test   %rbx,%rbx
    ffffffff8148800b:	0f 85 14 ff ff ff    	jne    ffffffff81487f25 <fsverity_get_info+0x25>
    ffffffff81488011:	31 db                	xor    %ebx,%ebx
    ffffffff81488013:	eb bf                	jmp    ffffffff81487fd4 <fsverity_get_info+0xd4>
    ffffffff81488015:	48 89 df             	mov    %rbx,%rdi
    ffffffff81488018:	e8 33 94 11 00       	call   ffffffff815a1450 <rht_bucket_nested>
    ffffffff8148801d:	49 89 c5             	mov    %rax,%r13
    ffffffff81488020:	e9 64 ff ff ff       	jmp    ffffffff81487f89 <fsverity_get_info+0x89>
    ffffffff81488025:	66 66 2e 0f 1f 84 00 	data16 cs nopw 0x0(%rax,%rax,1)
    ffffffff8148802c:	00 00 00 00 

It doesn't really seem like the kind of solution that's a good choice
for a frequently-loaded field.  And that's only the load; it's not
getting into the insertion (and resizing) part.

Reliability due to the code complexity is also a concern, of course.
'git log --grep="rhashtable:" --grep="Fixes:" --all-match' turns up
quite a few issues in rhashtable over time...

If we're going so far as to use a rhashtable, I have to wonder why we
aren't first prioritizing other fields.  For example ext4_inode_info
unconditionally has 40 bytes for fast_commit information, even though
fast_commit is an experimental ext4 feature that isn't enabled on most
filesystems.  That's 5 times as much as i_verity_info.  And quota has 24
bytes under CONFIG_QUOTA.  And there are even holes in the
ext4_inode_info struct; we could also just improve the field packing!

The fs-specific field solution from this patchset is much more efficient
than the rhashtable: efficient enough that we don't really have to worry
about it, regardless of fscrypt or fsverity.  So I think it's a good
middle ground, and I'd like to just do it this way.

- Eric

Re: [PATCH v5 00/13] Move fscrypt and fsverity info out of struct inode

[Christoph Hellwig]

On Sun, Aug 10, 2025 at 10:03:11AM -0700, Eric Biggers wrote:
> I assume you actually still mean fsverity, not fscrypt.

Yes, sorry.

> First, it would
> be helpful not to use one solution for fscrypt and a totally different
> solution for fsverity, as that would increase the maintenance cost well
> beyond that of either solution individually.

I agree that reducing the number of infrastructures is a goal.  But I
don't think we should limit us to a single "solution" for different
kinds of problems.

> 
> Second, the fsverity info can be loaded very frequently.  For example,
> curently it's loaded for each 4K data block processed.

Well, we can easily keep a once looked up data structure around for
any operation that does not leave file system control.  So for writing
that's a single ioctl context.  For read that is a single call into
->readahead, or maybe even ->read_iter.

> Also, there
> *are* use cases in which most files on the filesystem have fsverity
> enabled.  Not super common, but they exist.

Sure.  But the typical use case is a few files, and even that is just
a tiny minority of all ext4/f2fs/xfs file systems.

> It doesn't really seem like the kind of solution that's a good choice
> for a frequently-loaded field.  And that's only the load; it's not
> getting into the insertion (and resizing) part.

Assuming you actually get it down to once per high-level operation
above, it will still be absolute noise compared to the I/O generated.

> If we're going so far as to use a rhashtable, I have to wonder why we
> aren't first prioritizing other fields.  For example ext4_inode_info
> unconditionally has 40 bytes for fast_commit information, even though
> fast_commit is an experimental ext4 feature that isn't enabled on most
> filesystems.  That's 5 times as much as i_verity_info.  And quota has 24
> bytes under CONFIG_QUOTA.  And there are even holes in the
> ext4_inode_info struct; we could also just improve the field packing!

All that does sound like a good idea, independent of what we are
discussing here.

Re: [PATCH v5 03/13] ext4: move crypt info pointer to fs-specific part of inode

[Theodore Ts'o]

On Sun, Aug 10, 2025 at 12:56:56AM -0700, Eric Biggers wrote:
> Move the fscrypt_inode_info pointer into the filesystem-specific part of
> the inode by adding the field ext4_inode_info::i_crypt_info and
> configuring fscrypt_operations::inode_info_offs accordingly.
> 
> This is a prerequisite for a later commit that removes
> inode::i_crypt_info, saving memory and improving cache efficiency with
> filesystems that don't support fscrypt.
> 
> Co-developed-by: Christian Brauner <brauner@kernel.org>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>

Signed-off-by: Theodore Ts'o <tytso@mit.edu>

Re: [PATCH v5 09/13] ext4: move verity info pointer to fs-specific part of inode

[Theodore Ts'o]

On Sun, Aug 10, 2025 at 12:57:02AM -0700, Eric Biggers wrote:
> Move the fsverity_info pointer into the filesystem-specific part of the
> inode by adding the field ext4_inode_info::i_verity_info and configuring
> fsverity_operations::inode_info_offs accordingly.
> 
> This is a prerequisite for a later commit that removes
> inode::i_verity_info, saving memory and improving cache efficiency on
> filesystems that don't support fsverity.
> 
> Co-developed-by: Christian Brauner <brauner@kernel.org>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>

Acked-by: Theodore Ts'o <tytso@mit.edu>

Re: [PATCH v5 00/13] Move fscrypt and fsverity info out of struct inode

[Christian Brauner]

On Sun, Aug 10, 2025 at 02:03:02AM -0700, Eric Biggers wrote:
> On Sun, Aug 10, 2025 at 10:47:32AM +0200, Christian Brauner wrote:
> > On Sun, Aug 10, 2025 at 12:56:53AM -0700, Eric Biggers wrote:
> > > This is a cleaned-up implementation of moving the i_crypt_info and
> > > i_verity_info pointers out of 'struct inode' and into the fs-specific
> > > part of the inode, as proposed previously by Christian at
> > > https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> > > 
> > > The high-level concept is still the same: fs/crypto/ and fs/verity/
> > > locate the pointer by adding an offset to the address of struct inode.
> > > The offset is retrieved from fscrypt_operations or fsverity_operations.
> > > 
> > > I've cleaned up a lot of the details, including:
> > > - Grouped changes into patches differently
> > > - Rewrote commit messages and comments to be clearer
> > > - Adjusted code formatting to be consistent with existing code
> > > - Removed unneeded #ifdefs
> > > - Improved choice and location of VFS_WARN_ON_ONCE() statements
> > > - Added missing kerneldoc for ubifs_inode::i_crypt_info
> > > - Moved field initialization to init_once functions when they exist
> > > - Improved ceph offset calculation and removed unneeded static_asserts
> > > - fsverity_get_info() now checks IS_VERITY() instead of v_ops
> > > - fscrypt_put_encryption_info() no longer checks IS_ENCRYPTED(), since I
> > >   no longer think it's actually correct there.
> > > - verity_data_blocks() now keeps doing a raw dereference
> > > - Dropped fscrypt_set_inode_info() 
> > > - Renamed some functions
> > > - Do offset calculation using int, so we don't rely on unsigned overflow
> > > - And more.
> > > 
> > > For v4 and earlier, see
> > > https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> > > 
> > > I'd like to take this series through the fscrypt tree for 6.18.
> > > (fsverity normally has a separate tree, but by choosing just one tree
> > > for this, we'll avoid conflicts in some places.)
> > 
> > Woh woh. First, I had a cleaned up version ready for v6.18 so if you
> > plan on taking over someone's series and resend then maybe ask the
> > author first whether that's ok or not. I haven't seen you do that. You
> > just caused duplicated work for no reason.
> 
> Ah, sorry about that.  When I started looking at it again yesterday
> there turned out to be way too many cleanups and fixes I wanted to make
> (beyond the comments I gave earlier), and I hadn't seen activity from
> you on it in a while.  So I figured it would be easier to just send a
> series myself.  But I should have asked you first, sorry.

So I started working on this pretty much right away. And I had planned
on sending it out rather soon but then thought to better wait for -rc1
to be released because I saw you had a bunch of crypto changes in for
-rc1 that would've caused merge conflicts. It's no big deal overall but
I just don't like that I wasted massaging all that stuff. So next time a
heads-up would be nice. Thank you!

> 
> > And second general infrastructure changes that touch multiple fses and
> > generic fs infrastructure I very much want to go through VFS trees.
> > We'll simply use a shared tree.
> 
> So you'd like to discontinue the fscrypt and fsverity trees?  That's
> what they are for: general infrastructure shared by multiple
> filesystems.  Or is this comment just for this series in particular,
> presumably because it touches 'struct inode'?

My comment just applies this series. I'm not here to take away your
trees ofc unless you would like to have them go through the VFS batch.
That's something that some people like Amir have started doing.

I'll put the series into vfs-6.17.inode and push it out then you can
base any additional changes on top of that. I'll not touch it unless you
tell me to. Linus knows that VFS trees often have work that is used as
the base for other trees so he will merge VFS trees before any of the
smaller trees and I always mention this to him.

Re: [PATCH v5 00/13] Move fscrypt and fsverity info out of struct inode

[Christian Brauner]

On Mon, Aug 11, 2025 at 03:17:01PM +0200, Christian Brauner wrote:
> On Sun, Aug 10, 2025 at 02:03:02AM -0700, Eric Biggers wrote:
> > On Sun, Aug 10, 2025 at 10:47:32AM +0200, Christian Brauner wrote:
> > > On Sun, Aug 10, 2025 at 12:56:53AM -0700, Eric Biggers wrote:
> > > > This is a cleaned-up implementation of moving the i_crypt_info and
> > > > i_verity_info pointers out of 'struct inode' and into the fs-specific
> > > > part of the inode, as proposed previously by Christian at
> > > > https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> > > > 
> > > > The high-level concept is still the same: fs/crypto/ and fs/verity/
> > > > locate the pointer by adding an offset to the address of struct inode.
> > > > The offset is retrieved from fscrypt_operations or fsverity_operations.
> > > > 
> > > > I've cleaned up a lot of the details, including:
> > > > - Grouped changes into patches differently
> > > > - Rewrote commit messages and comments to be clearer
> > > > - Adjusted code formatting to be consistent with existing code
> > > > - Removed unneeded #ifdefs
> > > > - Improved choice and location of VFS_WARN_ON_ONCE() statements
> > > > - Added missing kerneldoc for ubifs_inode::i_crypt_info
> > > > - Moved field initialization to init_once functions when they exist
> > > > - Improved ceph offset calculation and removed unneeded static_asserts
> > > > - fsverity_get_info() now checks IS_VERITY() instead of v_ops
> > > > - fscrypt_put_encryption_info() no longer checks IS_ENCRYPTED(), since I
> > > >   no longer think it's actually correct there.
> > > > - verity_data_blocks() now keeps doing a raw dereference
> > > > - Dropped fscrypt_set_inode_info() 
> > > > - Renamed some functions
> > > > - Do offset calculation using int, so we don't rely on unsigned overflow
> > > > - And more.
> > > > 
> > > > For v4 and earlier, see
> > > > https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> > > > 
> > > > I'd like to take this series through the fscrypt tree for 6.18.
> > > > (fsverity normally has a separate tree, but by choosing just one tree
> > > > for this, we'll avoid conflicts in some places.)
> > > 
> > > Woh woh. First, I had a cleaned up version ready for v6.18 so if you
> > > plan on taking over someone's series and resend then maybe ask the
> > > author first whether that's ok or not. I haven't seen you do that. You
> > > just caused duplicated work for no reason.
> > 
> > Ah, sorry about that.  When I started looking at it again yesterday
> > there turned out to be way too many cleanups and fixes I wanted to make
> > (beyond the comments I gave earlier), and I hadn't seen activity from
> > you on it in a while.  So I figured it would be easier to just send a
> > series myself.  But I should have asked you first, sorry.
> 
> So I started working on this pretty much right away. And I had planned
> on sending it out rather soon but then thought to better wait for -rc1
> to be released because I saw you had a bunch of crypto changes in for
> -rc1 that would've caused merge conflicts. It's no big deal overall but
> I just don't like that I wasted massaging all that stuff. So next time a
> heads-up would be nice. Thank you!

I just pulled the series and now I see that you also changed the
authorship of every single patch in the series from me to you and put my
Co-developed-by in there.

I mean I acknowledge that there's changes between the branches and
there's some function renaming but it's not to the point where
authorship should be changed. And if you think that's necessary than it
would be something you would want to talk to me about first.

I don't care about the stats but it was always hugely frustrating to me
when I started kernel development when senior kernel developers waltzed
in and thought they'd just take things over so I try very hard to not do
that unless this is agreed upon first.

> > > And second general infrastructure changes that touch multiple fses and
> > > generic fs infrastructure I very much want to go through VFS trees.
> > > We'll simply use a shared tree.
> > 
> > So you'd like to discontinue the fscrypt and fsverity trees?  That's
> > what they are for: general infrastructure shared by multiple
> > filesystems.  Or is this comment just for this series in particular,
> > presumably because it touches 'struct inode'?
> 
> My comment just applies this series. I'm not here to take away your
> trees ofc unless you would like to have them go through the VFS batch.
> That's something that some people like Amir have started doing.
> 
> I'll put the series into vfs-6.17.inode and push it out then you can
> base any additional changes on top of that. I'll not touch it unless you
> tell me to. Linus knows that VFS trees often have work that is used as
> the base for other trees so he will merge VFS trees before any of the
> smaller trees and I always mention this to him.

Re: [PATCH v5 00/13] Move fscrypt and fsverity info out of struct inode

[Christian Brauner]

> The fs-specific field solution from this patchset is much more efficient
> than the rhashtable: efficient enough that we don't really have to worry
> about it, regardless of fscrypt or fsverity.  So I think it's a good
> middle ground, and I'd like to just do it this way.

I agree.

Re: [PATCH v5 00/13] Move fscrypt and fsverity info out of struct inode

[Eric Biggers]

On Mon, Aug 11, 2025 at 03:34:35PM +0200, Christian Brauner wrote:
> On Mon, Aug 11, 2025 at 03:17:01PM +0200, Christian Brauner wrote:
> > On Sun, Aug 10, 2025 at 02:03:02AM -0700, Eric Biggers wrote:
> > > On Sun, Aug 10, 2025 at 10:47:32AM +0200, Christian Brauner wrote:
> > > > On Sun, Aug 10, 2025 at 12:56:53AM -0700, Eric Biggers wrote:
> > > > > This is a cleaned-up implementation of moving the i_crypt_info and
> > > > > i_verity_info pointers out of 'struct inode' and into the fs-specific
> > > > > part of the inode, as proposed previously by Christian at
> > > > > https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> > > > > 
> > > > > The high-level concept is still the same: fs/crypto/ and fs/verity/
> > > > > locate the pointer by adding an offset to the address of struct inode.
> > > > > The offset is retrieved from fscrypt_operations or fsverity_operations.
> > > > > 
> > > > > I've cleaned up a lot of the details, including:
> > > > > - Grouped changes into patches differently
> > > > > - Rewrote commit messages and comments to be clearer
> > > > > - Adjusted code formatting to be consistent with existing code
> > > > > - Removed unneeded #ifdefs
> > > > > - Improved choice and location of VFS_WARN_ON_ONCE() statements
> > > > > - Added missing kerneldoc for ubifs_inode::i_crypt_info
> > > > > - Moved field initialization to init_once functions when they exist
> > > > > - Improved ceph offset calculation and removed unneeded static_asserts
> > > > > - fsverity_get_info() now checks IS_VERITY() instead of v_ops
> > > > > - fscrypt_put_encryption_info() no longer checks IS_ENCRYPTED(), since I
> > > > >   no longer think it's actually correct there.
> > > > > - verity_data_blocks() now keeps doing a raw dereference
> > > > > - Dropped fscrypt_set_inode_info() 
> > > > > - Renamed some functions
> > > > > - Do offset calculation using int, so we don't rely on unsigned overflow
> > > > > - And more.
> > > > > 
> > > > > For v4 and earlier, see
> > > > > https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> > > > > 
> > > > > I'd like to take this series through the fscrypt tree for 6.18.
> > > > > (fsverity normally has a separate tree, but by choosing just one tree
> > > > > for this, we'll avoid conflicts in some places.)
> > > > 
> > > > Woh woh. First, I had a cleaned up version ready for v6.18 so if you
> > > > plan on taking over someone's series and resend then maybe ask the
> > > > author first whether that's ok or not. I haven't seen you do that. You
> > > > just caused duplicated work for no reason.
> > > 
> > > Ah, sorry about that.  When I started looking at it again yesterday
> > > there turned out to be way too many cleanups and fixes I wanted to make
> > > (beyond the comments I gave earlier), and I hadn't seen activity from
> > > you on it in a while.  So I figured it would be easier to just send a
> > > series myself.  But I should have asked you first, sorry.
> > 
> > So I started working on this pretty much right away. And I had planned
> > on sending it out rather soon but then thought to better wait for -rc1
> > to be released because I saw you had a bunch of crypto changes in for
> > -rc1 that would've caused merge conflicts. It's no big deal overall but
> > I just don't like that I wasted massaging all that stuff. So next time a
> > heads-up would be nice. Thank you!
> 
> I just pulled the series and now I see that you also changed the
> authorship of every single patch in the series from me to you and put my
> Co-developed-by in there.
> 
> I mean I acknowledge that there's changes between the branches and
> there's some function renaming but it's not to the point where
> authorship should be changed. And if you think that's necessary than it
> would be something you would want to talk to me about first.
> 
> I don't care about the stats but it was always hugely frustrating to me
> when I started kernel development when senior kernel developers waltzed
> in and thought they'd just take things over so I try very hard to not do
> that unless this is agreed upon first.

If you want to keep the authorship that's fine with me.  Make sure
you've checked the diff: the diff between v4 and v5 is larger than the
diff between the base and either version.  And as I mentioned, I rewrote
the commit messages and divided some of the changes into commits
differently as well.  If the situation was flipped, I wouldn't want to
be kept as the author.  But I realize there are different opinions about
this sort of thing, and I'm totally fine with whatever you prefer.

Thanks,

- Eric

Re: [PATCH v5 00/13] Move fscrypt and fsverity info out of struct inode

[Christian Brauner]

On Mon, Aug 11, 2025 at 09:39:07AM -0700, Eric Biggers wrote:
> On Mon, Aug 11, 2025 at 03:34:35PM +0200, Christian Brauner wrote:
> > On Mon, Aug 11, 2025 at 03:17:01PM +0200, Christian Brauner wrote:
> > > On Sun, Aug 10, 2025 at 02:03:02AM -0700, Eric Biggers wrote:
> > > > On Sun, Aug 10, 2025 at 10:47:32AM +0200, Christian Brauner wrote:
> > > > > On Sun, Aug 10, 2025 at 12:56:53AM -0700, Eric Biggers wrote:
> > > > > > This is a cleaned-up implementation of moving the i_crypt_info and
> > > > > > i_verity_info pointers out of 'struct inode' and into the fs-specific
> > > > > > part of the inode, as proposed previously by Christian at
> > > > > > https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> > > > > > 
> > > > > > The high-level concept is still the same: fs/crypto/ and fs/verity/
> > > > > > locate the pointer by adding an offset to the address of struct inode.
> > > > > > The offset is retrieved from fscrypt_operations or fsverity_operations.
> > > > > > 
> > > > > > I've cleaned up a lot of the details, including:
> > > > > > - Grouped changes into patches differently
> > > > > > - Rewrote commit messages and comments to be clearer
> > > > > > - Adjusted code formatting to be consistent with existing code
> > > > > > - Removed unneeded #ifdefs
> > > > > > - Improved choice and location of VFS_WARN_ON_ONCE() statements
> > > > > > - Added missing kerneldoc for ubifs_inode::i_crypt_info
> > > > > > - Moved field initialization to init_once functions when they exist
> > > > > > - Improved ceph offset calculation and removed unneeded static_asserts
> > > > > > - fsverity_get_info() now checks IS_VERITY() instead of v_ops
> > > > > > - fscrypt_put_encryption_info() no longer checks IS_ENCRYPTED(), since I
> > > > > >   no longer think it's actually correct there.
> > > > > > - verity_data_blocks() now keeps doing a raw dereference
> > > > > > - Dropped fscrypt_set_inode_info() 
> > > > > > - Renamed some functions
> > > > > > - Do offset calculation using int, so we don't rely on unsigned overflow
> > > > > > - And more.
> > > > > > 
> > > > > > For v4 and earlier, see
> > > > > > https://lore.kernel.org/r/20250723-work-inode-fscrypt-v4-0-c8e11488a0e6@kernel.org/
> > > > > > 
> > > > > > I'd like to take this series through the fscrypt tree for 6.18.
> > > > > > (fsverity normally has a separate tree, but by choosing just one tree
> > > > > > for this, we'll avoid conflicts in some places.)
> > > > > 
> > > > > Woh woh. First, I had a cleaned up version ready for v6.18 so if you
> > > > > plan on taking over someone's series and resend then maybe ask the
> > > > > author first whether that's ok or not. I haven't seen you do that. You
> > > > > just caused duplicated work for no reason.
> > > > 
> > > > Ah, sorry about that.  When I started looking at it again yesterday
> > > > there turned out to be way too many cleanups and fixes I wanted to make
> > > > (beyond the comments I gave earlier), and I hadn't seen activity from
> > > > you on it in a while.  So I figured it would be easier to just send a
> > > > series myself.  But I should have asked you first, sorry.
> > > 
> > > So I started working on this pretty much right away. And I had planned
> > > on sending it out rather soon but then thought to better wait for -rc1
> > > to be released because I saw you had a bunch of crypto changes in for
> > > -rc1 that would've caused merge conflicts. It's no big deal overall but
> > > I just don't like that I wasted massaging all that stuff. So next time a
> > > heads-up would be nice. Thank you!
> > 
> > I just pulled the series and now I see that you also changed the
> > authorship of every single patch in the series from me to you and put my
> > Co-developed-by in there.
> > 
> > I mean I acknowledge that there's changes between the branches and
> > there's some function renaming but it's not to the point where
> > authorship should be changed. And if you think that's necessary than it
> > would be something you would want to talk to me about first.
> > 
> > I don't care about the stats but it was always hugely frustrating to me
> > when I started kernel development when senior kernel developers waltzed
> > in and thought they'd just take things over so I try very hard to not do
> > that unless this is agreed upon first.
> 
> If you want to keep the authorship that's fine with me.  Make sure
> you've checked the diff: the diff between v4 and v5 is larger than the
> diff between the base and either version.  And as I mentioned, I rewrote
> the commit messages and divided some of the changes into commits
> differently as well.  If the situation was flipped, I wouldn't want to
> be kept as the author.  But I realize there are different opinions about
> this sort of thing, and I'm totally fine with whatever you prefer.

It's not that I oppose it per se it's just that it would be nice to have
gotten a heads-up about both the rewrite and the authorship change.
(Sorry, still on vacation and so answers are delayed.)