Backport object allocation APIs and two fscrypt fixes

Like I did in an earlier series for 6.18
(https://lore.kernel.org/linux-hardening/[email protected]/),
this series backports kmalloc_obj() et al to 6.12 so that more upstream commits will cherry-pick cleanly. As in 6.18, the flex counter integration isn't included.

Patches 6-7 then backport two fscrypt fixes (which, like lots of other kernel code, happen to use kzalloc_obj()). Patch 7 is a clean cherry-pick, while patch 6 required resolving some other conflicts.

Eric Biggers (2):
fscrypt: Fix key setup in edge case with multiple data unit sizes
fscrypt: Replace mk_users keyring with simple list

Kees Cook (2):
slab: Introduce kmalloc_obj() and family
slab: Introduce kmalloc_flex() and family

Linus Torvalds (2):
add default_gfp() helper macro and use it in the new *alloc_obj()
helpers
default_gfp(): avoid using the "newfangled" __VA_OPT__ trick

Randy Dunlap (1):
slab: recognize @GFP parameter as optional in kernel-doc

 Documentation/process/deprecated.rst |  31 ++++
 fs/crypto/fscrypt_private.h          |  87 ++++++----
 fs/crypto/inline_crypt.c             |   8 +-
 fs/crypto/keyring.c                  | 239 ++++++++++++---------------
 fs/crypto/keysetup.c                 | 119 ++++++++-----
 include/linux/gfp.h                  |   4 +
 include/linux/slab.h                 | 100 +++++++++++
 7 files changed, 373 insertions(+), 215 deletions(-)

base-commit: 296aabce459470a4c1b68ffd0c0c0920e563aaad

2.55.0

From: Kees Cook <[email protected]>

commit 2932ba8d9c99875b98c951d9d3fd6d651d35df3a upstream.

Introduce type-aware kmalloc-family helpers to replace the common idioms for single object and arrays of objects allocation:

	ptr = kmalloc(sizeof(*ptr), gfp);
	ptr = kmalloc(sizeof(struct some_obj_name), gfp);
	ptr = kzalloc(sizeof(*ptr), gfp);
	ptr = kmalloc_array(count, sizeof(*ptr), gfp);
	ptr = kcalloc(count, sizeof(*ptr), gfp);

These become, respectively:

	ptr = kmalloc_obj(*ptr, gfp);
	ptr = kmalloc_obj(*ptr, gfp);
	ptr = kzalloc_obj(*ptr, gfp);
	ptr = kmalloc_objs(*ptr, count, gfp);
	ptr = kzalloc_objs(*ptr, count, gfp);

Beyond the other benefits outlined below, the primary ergonomic benefit is the elimination of needing "sizeof" nor the type name, and the enforcement of assignment types (they do not return "void *", but rather a pointer to the type of the first argument). The type name _can_ be used, though, in the case where an assignment is indirect (e.g. via "return"). This additionally allows[1] variables to be declared via __auto_type:

	__auto_type ptr = kmalloc_obj(struct foo, gfp);

Internal introspection of the allocated type now becomes possible, allowing for future alignment-aware choices to be made by the allocator and future hardening work that can be type sensitive. For example, adding __alignof(*ptr) as an argument to the internal allocators so that appropriate/efficient alignment choices can be made, or being able to correctly choose per-allocation offset randomization within a bucket that does not break alignment requirements.

---
 Documentation/process/deprecated.rst | 24 ++++++++++++
 include/linux/slab.h                 | 58 ++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+)

index 1f7f3e6c9cda..91c628fa2d59 100644
@@ -372,3 +372,27 @@ The helper must be used::
 			DECLARE_FLEX_ARRAY(struct type2, two);
 		};
 	};
+
+Open-coded kmalloc assignments for struct objects
+-------------------------------------------------
+Performing open-coded kmalloc()-family allocation assignments prevents
+the kernel (and compiler) from being able to examine the type of the
+variable being assigned, which limits any related introspection that
+may help with alignment, wrap-around, or additional hardening. The
+kmalloc_obj()-family of macros provide this introspection, which can be
+used for the common code patterns for single, array, and flexible object
+allocations. For example, these open coded assignments::
+
+	ptr = kmalloc(sizeof(*ptr), gfp);
+	ptr = kzalloc(sizeof(*ptr), gfp);
+	ptr = kmalloc_array(count, sizeof(*ptr), gfp);
+	ptr = kcalloc(count, sizeof(*ptr), gfp);
+	ptr = kmalloc(sizeof(struct foo, gfp);
+
+become, respectively::
+
+	ptr = kmalloc_obj(*ptr, gfp);
+	ptr = kzalloc_obj(*ptr, gfp);
+	ptr = kmalloc_objs(*ptr, count, gfp);
+	ptr = kzalloc_objs(*ptr, count, gfp);
+	__auto_type ptr = kmalloc_obj(struct foo, gfp);

2.55.0

From: Kees Cook <[email protected]>

commit e4c8b46b924eb8de66c6f0accc9cdd0c2e8fa23b upstream.

As done for kmalloc_obj*(), introduce a type-aware allocator for flexible arrays, which may also have "counted_by" annotations:

	ptr = kmalloc(struct_size(ptr, flex_member, count), gfp);

becomes:

	ptr = kmalloc_flex(*ptr, flex_member, count, gfp);

The internal use of __flex_counter() allows for automatically setting the counter member of a struct's flexible array member when it has been annotated with __counted_by(), avoiding any missed early size initializations while __counted_by() annotations are added to the kernel. Additionally, this also checks for "too large" allocations based on the type size of the counter variable. For example:

	if (count > type_max(ptr->flex_counter))
		fail...;
	size = struct_size(ptr, flex_member, count);
	ptr = kmalloc(size, gfp);
	if (!ptr)
		fail...;
	ptr->flex_counter = count;

becomes (n.b. unchanged from earlier example):

	ptr = kmalloc_flex(*ptr, flex_member, count, gfp);
	if (!ptr)
		fail...;
	ptr->flex_counter = count;

Note that manual initialization of the flexible array counter is still required (at some point) after allocation as not all compiler versions support the __counted_by annotation yet. But doing it internally makes sure they cannot be missed when __counted_by _is_ available, meaning that the bounds checker will not trip due to the lack of "early enough" initializations that used to work before enabling the stricter bounds checking. For example:

	ptr = kmalloc_flex(*ptr, flex_member, count, gfp);
	fill(ptr->flex, count);
	ptr->flex_count = count;

This works correctly before adding a __counted_by annotation (since nothing is checking ptr->flex accesses against ptr->flex_count). After adding the annotation, the bounds sanitizer would trip during fill() because ptr->flex_count wasn't set yet. But with kmalloc_flex() setting ptr->flex_count internally at allocation time, the existing code works without needing to move the ptr->flex_count assignment before the call to fill(). (This has been a stumbling block for __counted_by adoption.)

[Backport-notes: Removed the actual flex counter handling. That's a new
feature, which isn't necessary for just adding the new allocation APIs to get backports to apply cleanly. Also, the allocation-time overflow check in the upstream commit was reverted upstream.]

---
 Documentation/process/deprecated.rst |  7 +++++
 include/linux/slab.h                 | 42 ++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

index 91c628fa2d59..fed56864d036 100644
@@ -387,6 +387,7 @@ allocations. For example, these open coded assignments::
 	ptr = kzalloc(sizeof(*ptr), gfp);
 	ptr = kmalloc_array(count, sizeof(*ptr), gfp);
 	ptr = kcalloc(count, sizeof(*ptr), gfp);
+	ptr = kmalloc(struct_size(ptr, flex_member, count), gfp);
 	ptr = kmalloc(sizeof(struct foo, gfp);
 
 become, respectively::

@@ -395,4 +396,10 @@ become, respectively::
 	ptr = kzalloc_obj(*ptr, gfp);
 	ptr = kmalloc_objs(*ptr, count, gfp);
 	ptr = kzalloc_objs(*ptr, count, gfp);
+	ptr = kmalloc_flex(*ptr, flex_member, count, gfp);
 	__auto_type ptr = kmalloc_obj(struct foo, gfp);
+
+If `ptr->flex_member` is annotated with __counted_by(), the allocation
+will automatically fail if `count` is larger than the maximum
+representable value that can be stored in the counter member associated
+with `flex_member`.

2.55.0

From: Linus Torvalds <[email protected]>

commit e19e1b480ac73c3e62ffebbca1174f0f511f43e7 upstream.

Most simple allocations use GFP_KERNEL, and with the new allocation helpers being introduced, let's just take advantage of that to simplify that default case.

It's a numbers game:

git grep 'alloc_obj(' |
sed 's/.*\(GFP_[_A-Z]*\).*/\1/' |
sort | uniq -c | sort -n | tail

shows that about 90% of all those new allocator instances just use that standard GFP_KERNEL.

Those helpers are already macros, and we can easily just make it be the default case when the gfp argument is missing.

And yes, we could do that for all the legacy interfaces too, but let's keep it to just the new ones at least for now, since those all got converted recently anyway, so this is not any "extra" noise outside of that limited conversion.

And, in fact, I want to do this before doing the -rc1 release, exactly so that we don't get extra merge conflicts.

---
 include/linux/gfp.h  |  4 ++++
 include/linux/slab.h | 48 ++++++++++++++++++++++----------------------
 2 files changed, 28 insertions(+), 24 deletions(-)

index bc59016743fb..abe4282fbdb1 100644
@@ -12,6 +12,10 @@
 struct vm_area_struct;
 struct mempolicy;
 
+/* Helper macro to avoid gfp flags if they are the default one */
+#define __default_gfp(a,...) a
+#define default_gfp(...) __default_gfp(__VA_ARGS__ __VA_OPT__(,) GFP_KERNEL)
+
 /* Convert GFP flags to their corresponding migrate type */
 #define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE)
 #define GFP_MOVABLE_SHIFT 3

2.55.0

From: Linus Torvalds <[email protected]>

commit 551d44200152cb26f75d2ef990aeb6185b7e37fd upstream.

The default_gfp() helper that I added is not wrong, but it turns out that it causes unnecessary headaches for 'sparse' which doesn't support the use of __VA_OPT__ (introduced in C++20 and C23, and supported by gcc and clang for a long time).

We do already use __VA_OPT__ in some other cases in the kernel (drm/xe and btrfs), but it has been fairly limited. Now it triggers for pretty much everything, and sparse ends up not working at all.

We can use the traditional gcc ',##__VA_ARGS__' syntax instead: it may not be the "C standard" way and is slightly less natural in this context, but it is the traditional model for this and avoids the sparse problem.

---
 include/linux/gfp.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index abe4282fbdb1..c7621a0714c1 100644
@@ -13,8 +13,8 @@ struct vm_area_struct;
 struct mempolicy;
 
 /* Helper macro to avoid gfp flags if they are the default one */
-#define __default_gfp(a,...) a
-#define default_gfp(...) __default_gfp(__VA_ARGS__ __VA_OPT__(,) GFP_KERNEL)
+#define __default_gfp(a,b,...) b
+#define default_gfp(...) __default_gfp(,##__VA_ARGS__,GFP_KERNEL)
 
 /* Convert GFP flags to their corresponding migrate type */
 #define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE)

2.55.0

From: Randy Dunlap <[email protected]>

commit 7b5f5865fb11e60edd03c5e063e2d228b7062317 upstream.

Since the @GFP parameter in kmalloc_obj() etc. is now optional, change the kernel-doc to indicate that it is optional. This avoids kernel-doc warnings:

WARNING: include/linux/slab.h:1101 Excess function parameter 'GFP' description in 'kmalloc_obj'
WARNING: include/linux/slab.h:1113 Excess function parameter 'GFP' description in 'kmalloc_objs'
WARNING: include/linux/slab.h:1128 Excess function parameter 'GFP' description in 'kmalloc_flex'

---
 include/linux/slab.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index 18a149e51346..773843a71960 100644
@@ -924,7 +924,7 @@ static __always_inline __alloc_size(1) void *kmalloc_noprof(size_t size, gfp_t f
 /**
  * kmalloc_obj - Allocate a single instance of the given type
  * @VAR_OR_TYPE: Variable or type to allocate.
- * @GFP: GFP flags for the allocation.
+ * @...: optional GFP flags for the allocation (GFP_KERNEL when not specified).
  *
  * Returns: newly allocated pointer to a @VAR_OR_TYPE on success, or NULL
  * on failure.

@@ -936,7 +936,7 @@ static __always_inline __alloc_size(1) void *kmalloc_noprof(size_t size, gfp_t f
  * kmalloc_objs - Allocate an array of the given type
  * @VAR_OR_TYPE: Variable or type to allocate an array of.
  * @COUNT: How many elements in the array.
- * @GFP: GFP flags for the allocation.
+ * @...: optional GFP flags for the allocation (GFP_KERNEL when not specified).
  *
  * Returns: newly allocated pointer to array of @VAR_OR_TYPE on success,
  * or NULL on failure.

@@ -949,7 +949,7 @@ static __always_inline __alloc_size(1) void *kmalloc_noprof(size_t size, gfp_t f
  * @VAR_OR_TYPE: Variable or type to allocate (with its flex array).
  * @FAM: The name of the flexible array member of the structure.
  * @COUNT: How many flexible array member elements are desired.
- * @GFP: GFP flags for the allocation.
+ * @...: optional GFP flags for the allocation (GFP_KERNEL when not specified).
  *
  * Returns: newly allocated pointer to @VAR_OR_TYPE on success, NULL on
  * failure. If @FAM has been annotated with __counted_by(), the allocation

2.55.0

commit dd015b566d505d698386103e9c80b739c7336eb8 upstream.

The addition of support for customizable data unit sizes introduced an edge case where a file's contents can be en/decrypted with the wrong data unit size. It occurs when there are multiple v2 policies that:

- Have *different* data unit sizes, via the log2_data_unit_size field

- Share the same master_key_identifier, contents_encryption_mode, and either FSCRYPT_POLICY_FLAG_DIRECT_KEY, FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32, or FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64

- Are being used on the same filesystem, which also must be mounted with the "inlinecrypt" mount option.

Fortunately this edge case doesn't actually occur in practice. I just found it via code review. But it needs to be fixed regardless.

The bug is caused by the data unit size not being fully considered when blk_crypto_keys are cached in mk_direct_keys, mk_iv_ino_lblk_32_keys, and mk_iv_ino_lblk_64_keys. They're differentiated only by master key, encryption mode, and flag. However, each one actually has a data unit size too. Only the first data unit size that is cached is used.

To fix this, start using the data unit size to differentiate the cached keys. For several reasons, including avoiding increasing the size of struct fscrypt_master_key, just replace all three arrays with a single linked list instead of changing them into two-dimensional arrays. This works well when considering that in practice at most 2 entries are used across all three arrays, so it was already mostly wasted space.

For simplicity, make the list also take over the publish/subscribe of the prepared key itself. That is, create separate list nodes for blk_crypto_keys vs crypto_skciphers, and add nodes to the list only when their key is actually prepared. (Note that the legacy fscrypt_direct_keys table in fs/crypto/keysetup_v1.c already works this way.) This eliminates the need for the additional memory barriers when reading and writing the fields of struct fscrypt_prepared_key.

Note that I technically should have included the data unit size in the HKDF info string as well. But it's too late to change that.

---
 fs/crypto/fscrypt_private.h |  55 ++++++++++-------
 fs/crypto/inline_crypt.c    |   8 +--
 fs/crypto/keyring.c         |  23 ++++---
 fs/crypto/keysetup.c        | 119 +++++++++++++++++++++++-------------
 4 files changed, 125 insertions(+), 80 deletions(-)

index 25bcfcc2d706..dd169048017b 100644
@@ -27,6 +27,9 @@
  */
 #define FSCRYPT_MIN_KEY_SIZE	16
 
+/* Maximum size of a raw fscrypt master key */
+#define FSCRYPT_MAX_RAW_KEY_SIZE	64
+
 /*
  * This mask is passed as the third argument to the crypto_alloc_*() functions
  * to prevent fscrypt from using the Crypto API drivers for non-inline crypto

@@ -217,7 +220,7 @@ struct fscrypt_symlink_data {
  * @tfm: crypto API transform object
  * @blk_key: key for blk-crypto
  *
- * Normally only one of the fields will be non-NULL.
+ * Only one of the fields is non-NULL.
  */
 struct fscrypt_prepared_key {
 	struct crypto_skcipher *tfm;

@@ -226,6 +229,15 @@ struct fscrypt_prepared_key {
 #endif
 };
 
+/* An entry in the linked list ->mk_mode_keys */
+struct fscrypt_mode_key {
+	struct fscrypt_prepared_key key;
+	struct list_head link;
+	u8 hkdf_context;
+	u8 mode_num;
+	u8 data_unit_bits;
+};
+
 /*
  * fscrypt_inode_info - the "encryption key" for an inode
  *

@@ -413,20 +425,12 @@ void fscrypt_destroy_inline_crypt_key(struct super_block *sb,
  * @prep_key, depending on which encryption implementation the file will use.
  */
 static inline bool
-fscrypt_is_key_prepared(struct fscrypt_prepared_key *prep_key,
+fscrypt_is_key_prepared(const struct fscrypt_prepared_key *prep_key,
 			const struct fscrypt_inode_info *ci)
 {
-	/*
-	 * The two smp_load_acquire()'s here pair with the smp_store_release()'s
-	 * in fscrypt_prepare_inline_crypt_key() and fscrypt_prepare_key().
-	 * I.e., in some cases (namely, if this prep_key is a per-mode
-	 * encryption key) another task can publish blk_key or tfm concurrently,
-	 * executing a RELEASE barrier.  We need to use smp_load_acquire() here
-	 * to safely ACQUIRE the memory the other task published.
-	 */
 	if (fscrypt_using_inline_encryption(ci))
-		return smp_load_acquire(&prep_key->blk_key) != NULL;
-	return smp_load_acquire(&prep_key->tfm) != NULL;
+		return prep_key->blk_key != NULL;
+	return prep_key->tfm != NULL;
 }
 
 #else /* CONFIG_FS_ENCRYPTION_INLINE_CRYPT */

@@ -458,10 +462,10 @@ fscrypt_destroy_inline_crypt_key(struct super_block *sb,
 }
 
 static inline bool
-fscrypt_is_key_prepared(struct fscrypt_prepared_key *prep_key,
+fscrypt_is_key_prepared(const struct fscrypt_prepared_key *prep_key,
 			const struct fscrypt_inode_info *ci)
 {
-	return smp_load_acquire(&prep_key->tfm) != NULL;
+	return prep_key->tfm != NULL;
 }
 #endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
 

@@ -531,8 +535,8 @@ struct fscrypt_master_key {
 	/*
 	 * Active and structural reference counts.  An active ref guarantees
 	 * that the struct continues to exist, continues to be in the keyring
-	 * ->s_master_keys, and that any embedded subkeys (e.g.
-	 * ->mk_direct_keys) that have been prepared continue to exist.
+	 * ->s_master_keys, and that any non-file-scoped subkeys (e.g.
+	 * ->mk_mode_keys) that have been prepared continue to exist.
 	 * A structural ref only guarantees that the struct continues to exist.
 	 *
 	 * There is one active ref associated with ->mk_present being true, and

@@ -586,12 +590,21 @@ struct fscrypt_master_key {
 	spinlock_t		mk_decrypted_inodes_lock;
 
 	/*
-	 * Per-mode encryption keys for the various types of encryption policies
-	 * that use them.  Allocated and derived on-demand.
+	 * A list of 'struct fscrypt_mode_key' for the (hkdf_context, mode_num,
+	 * data_unit_bits, inlinecrypt) combinations that are in use for this
+	 * master key, for hkdf_context in [HKDF_CONTEXT_DIRECT_KEY,
+	 * HKDF_CONTEXT_IV_INO_LBLK_32_KEY, HKDF_CONTEXT_IV_INO_LBLK_64_KEY].
+	 *
+	 * This is a linked list and not a hash table because in practice
+	 * there's just a single encryption policy per master key, using
+	 * _at most_ 2 nodes in this list.  Per-file keys don't use this at all.
+	 *
+	 * This list is append-only until the master key is fully removed, at
+	 * which time the list is cleared.  Before then,
+	 * fscrypt_mode_key_setup_mutex synchronizes appends, and searches use
+	 * the RCU read lock together with ->mk_sem held for read.
 	 */
-	struct fscrypt_prepared_key mk_direct_keys[FSCRYPT_MODE_MAX + 1];
-	struct fscrypt_prepared_key mk_iv_ino_lblk_64_keys[FSCRYPT_MODE_MAX + 1];
-	struct fscrypt_prepared_key mk_iv_ino_lblk_32_keys[FSCRYPT_MODE_MAX + 1];
+	struct list_head	mk_mode_keys;
 
 	/* Hash key for inode numbers.  Initialized only when needed. */
 	siphash_key_t		mk_ino_hash_key;

index 40de69860dcf..4b1f10f1cbd8 100644
@@ -191,13 +191,7 @@ int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
 		goto fail;
 	}
 
-	/*
-	 * Pairs with the smp_load_acquire() in fscrypt_is_key_prepared().
-	 * I.e., here we publish ->blk_key with a RELEASE barrier so that
-	 * concurrent tasks can ACQUIRE it.  Note that this concurrency is only
-	 * possible for per-mode keys, not for per-file keys.
-	 */
-	smp_store_release(&prep_key->blk_key, blk_key);
+	prep_key->blk_key = blk_key;
 	return 0;
 
 fail:

index 206835e31efa..6dd6d198be63 100644
@@ -86,14 +86,14 @@ void fscrypt_put_master_key(struct fscrypt_master_key *mk)
 void fscrypt_put_master_key_activeref(struct super_block *sb,
 				      struct fscrypt_master_key *mk)
 {
-	size_t i;
+	struct fscrypt_mode_key *node, *tmp;
 
 	if (!refcount_dec_and_test(&mk->mk_active_refs))
 		return;
 	/*
 	 * No active references left, so complete the full removal of this
 	 * fscrypt_master_key struct by removing it from the keyring and
-	 * destroying any subkeys embedded in it.
+	 * destroying any non-file-scoped subkeys.
 	 */
 
 	if (WARN_ON_ONCE(!sb->s_master_keys))

@@ -109,13 +109,16 @@ void fscrypt_put_master_key_activeref(struct super_block *sb,
 	WARN_ON_ONCE(mk->mk_present);
 	WARN_ON_ONCE(!list_empty(&mk->mk_decrypted_inodes));
 
-	for (i = 0; i <= FSCRYPT_MODE_MAX; i++) {
-		fscrypt_destroy_prepared_key(
-				sb, &mk->mk_direct_keys[i]);
-		fscrypt_destroy_prepared_key(
-				sb, &mk->mk_iv_ino_lblk_64_keys[i]);
-		fscrypt_destroy_prepared_key(
-				sb, &mk->mk_iv_ino_lblk_32_keys[i]);
+	/*
+	 * Destroy any non-file-scoped subkeys.  Since ->mk_active_refs == 0,
+	 * they're no longer referenced by any inodes.  Nor can key setup run
+	 * and use them again.  So they're no longer needed.  (This implies no
+	 * concurrent readers, so we don't need list_del_rcu() for example.)
+	 */
+	list_for_each_entry_safe(node, tmp, &mk->mk_mode_keys, link) {
+		fscrypt_destroy_prepared_key(sb, &node->key);
+		list_del(&node->link);
+		kfree(node);
 	}
 	memzero_explicit(&mk->mk_ino_hash_key,
 			 sizeof(mk->mk_ino_hash_key));

@@ -444,6 +447,8 @@ static int add_new_master_key(struct super_block *sb,
 	INIT_LIST_HEAD(&mk->mk_decrypted_inodes);
 	spin_lock_init(&mk->mk_decrypted_inodes_lock);
 
+	INIT_LIST_HEAD(&mk->mk_mode_keys);
+
 	if (mk_spec->type == FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER) {
 		err = allocate_master_key_users_keyring(mk);
 		if (err)

2.55.0

commit 696c030e1e3438955aba443b308ee8b6faa3983e upstream.

Change mk_users (the set of user claims to an fscrypt master key) from a 'struct key' keyring to a simple linked list.

It's still a collection of 'struct key' for quota tracking. It was originally thought to be natural that a collection of 'struct key' should be held in a 'struct key' keyring. In reality, it's just been causing problems, similar to how using 'struct key' for the filesystem keyring caused problems and was removed in commit d7e7b9af104c ("fscrypt: stop using keyrings subsystem for fscrypt_master_key").

Commit d3a7bd420076 ("fscrypt: clear keyring before calling key_put()") fixed mk_users cleanup to be synchronous. But that apparently wasn't
enough: the keyring subsystem's redundant locking is still generating lockdep false positives due to the interaction with filesystem reclaim.

With the simple list, the redundant locking and lockdep issue goes away.

Of course, searching a linked list is linear-time whereas the 'struct key' keyring used a fancy constant-time associative array. But that's fine here, since in practice there's just one entry in the list. In fact the new code is much faster in practice, since it's much smaller and doesn't have to convert the kuid_t into a string to search for it.

---
 fs/crypto/fscrypt_private.h |  32 ++++--
 fs/crypto/keyring.c         | 216 +++++++++++++++---------------------
 2 files changed, 113 insertions(+), 135 deletions(-)

index dd169048017b..eac9f657caea 100644
@@ -471,6 +471,19 @@ fscrypt_is_key_prepared(const struct fscrypt_prepared_key *prep_key,
 
 /* keyring.c */
 
+/*
+ * fscrypt_master_key_user - a user's claim to a master key
+ */
+struct fscrypt_master_key_user {
+	struct list_head link;
+	kuid_t uid;
+	/*
+	 * This 'struct key' contains no secret.  It exists solely to charge the
+	 * appropriate user's key quota.
+	 */
+	struct key *quota_key;
+};
+
 /*
  * fscrypt_master_key_secret - secret key material of an in-use master key
  */

@@ -568,19 +581,18 @@ struct fscrypt_master_key {
 	struct fscrypt_key_specifier		mk_spec;
 
 	/*
-	 * Keyring which contains a key of type 'key_type_fscrypt_user' for each
-	 * user who has added this key.  Normally each key will be added by just
-	 * one user, but it's possible that multiple users share a key, and in
-	 * that case we need to keep track of those users so that one user can't
-	 * remove the key before the others want it removed too.
+	 * List of user claims to this key (struct fscrypt_master_key_user).
+	 * Normally each key will be added by just one user, but it's possible
+	 * that multiple users share a key, and in that case we need to keep
+	 * track of those users so that one user can't remove the key before the
+	 * others want it removed too.
 	 *
-	 * This is NULL for v1 policy keys; those can only be added by root.
+	 * Used only for v2 policy keys.  v1 policy keys can be added only by
+	 * root, so user tracking doesn't apply to them.
 	 *
-	 * Locking: protected by ->mk_sem.  (We don't just rely on the keyrings
-	 * subsystem semaphore ->mk_users->sem, as we need support for atomic
-	 * search+insert along with proper synchronization with other fields.)
+	 * Locking: protected by ->mk_sem.
 	 */
-	struct key		*mk_users;
+	struct list_head	mk_users;
 
 	/*
 	 * List of inodes that were unlocked using this key.  This allows the

2.55.0

All now queued up, thanks!

greg k-h