[PATCH] fscrypt: Replace mk_users keyring with simple list

[PATCH] fscrypt: Replace mk_users keyring with simple list

[Eric Biggers]

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.

Reported-by: syzbot+f55b043dacf43776b50c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f55b043dacf43776b50c
Reported-by: Mohammed EL Kadiri <med08elkadiri@gmail.com>
Closes: https://lore.kernel.org/keyrings/20260614150041.21172-1-med08elkadiri@gmail.com/
Fixes: 23c688b54016 ("fscrypt: allow unprivileged users to add/remove keys for v2 policies")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---

I'm planning to take this via the fscrypt tree for 7.2

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

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 4263cac24b32..0053b5c45412 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -494,10 +494,23 @@ fscrypt_is_key_prepared(const struct fscrypt_prepared_key *prep_key,
 }
 #endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
 
 /* 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
  */
 struct fscrypt_master_key_secret {
 
@@ -609,23 +622,22 @@ struct fscrypt_master_key {
 	 * For v2 policy keys: a cryptographic hash of this key (->identifier).
 	 */
 	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
 	 * inodes to be evicted efficiently if the key is removed.
 	 */
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 6ce6b436c34f..16bc348213ca 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -63,26 +63,23 @@ static void fscrypt_free_master_key(struct rcu_head *head)
 	 * Nevertheless, use kfree_sensitive() in case anything was missed.
 	 */
 	kfree_sensitive(mk);
 }
 
+static void clear_mk_users(struct fscrypt_master_key *mk);
+
 void fscrypt_put_master_key(struct fscrypt_master_key *mk)
 {
 	if (!refcount_dec_and_test(&mk->mk_struct_refs))
 		return;
 	/*
-	 * No structural references left, so free ->mk_users, and also free the
+	 * No structural references left, so clear ->mk_users, and also free the
 	 * fscrypt_master_key struct itself after an RCU grace period ensures
 	 * that concurrent keyring lookups can no longer find it.
 	 */
 	WARN_ON_ONCE(refcount_read(&mk->mk_active_refs) != 0);
-	if (mk->mk_users) {
-		/* Clear the keyring so the quota gets released right away. */
-		keyring_clear(mk->mk_users);
-		key_put(mk->mk_users);
-		mk->mk_users = NULL;
-	}
+	clear_mk_users(mk);
 	call_rcu(&mk->mk_rcu_head, fscrypt_free_master_key);
 }
 
 void fscrypt_put_master_key_activeref(struct super_block *sb,
 				      struct fscrypt_master_key *mk)
@@ -163,12 +160,12 @@ static void fscrypt_user_key_describe(const struct key *key, struct seq_file *m)
 {
 	seq_puts(m, key->description);
 }
 
 /*
- * Type of key in ->mk_users.  Each key of this type represents a particular
- * user who has added a particular master key.
+ * Type of fscrypt_master_key_user::quota_key.  This contains no secret; it
+ * exists solely to charge a user's key quota.
  *
  * Note that the name of this key type really should be something like
  * ".fscrypt-user" instead of simply ".fscrypt".  But the shorter name is chosen
  * mainly for simplicity of presentation in /proc/keys when read by a non-root
  * user.  And it is expected to be rare that a key is actually added by multiple
@@ -178,34 +175,13 @@ static struct key_type key_type_fscrypt_user = {
 	.name			= ".fscrypt",
 	.instantiate		= fscrypt_user_key_instantiate,
 	.describe		= fscrypt_user_key_describe,
 };
 
-#define FSCRYPT_MK_USERS_DESCRIPTION_SIZE	\
-	(CONST_STRLEN("fscrypt-") + 2 * FSCRYPT_KEY_IDENTIFIER_SIZE + \
-	 CONST_STRLEN("-users") + 1)
-
 #define FSCRYPT_MK_USER_DESCRIPTION_SIZE	\
 	(2 * FSCRYPT_KEY_IDENTIFIER_SIZE + CONST_STRLEN(".uid.") + 10 + 1)
 
-static void format_mk_users_keyring_description(
-			char description[FSCRYPT_MK_USERS_DESCRIPTION_SIZE],
-			const u8 mk_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
-{
-	sprintf(description, "fscrypt-%*phN-users",
-		FSCRYPT_KEY_IDENTIFIER_SIZE, mk_identifier);
-}
-
-static void format_mk_user_description(
-			char description[FSCRYPT_MK_USER_DESCRIPTION_SIZE],
-			const u8 mk_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
-{
-
-	sprintf(description, "%*phN.uid.%u", FSCRYPT_KEY_IDENTIFIER_SIZE,
-		mk_identifier, __kuid_val(current_fsuid()));
-}
-
 /* Create ->s_master_keys if needed.  Synchronized by fscrypt_add_key_mutex. */
 static int allocate_filesystem_keyring(struct super_block *sb)
 {
 	struct fscrypt_keyring *keyring;
 
@@ -336,95 +312,98 @@ fscrypt_find_master_key(struct super_block *sb,
 out:
 	rcu_read_unlock();
 	return mk;
 }
 
-static int allocate_master_key_users_keyring(struct fscrypt_master_key *mk)
-{
-	char description[FSCRYPT_MK_USERS_DESCRIPTION_SIZE];
-	struct key *keyring;
-
-	format_mk_users_keyring_description(description,
-					    mk->mk_spec.u.identifier);
-	keyring = keyring_alloc(description, GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
-				current_cred(), KEY_POS_SEARCH |
-				  KEY_USR_SEARCH | KEY_USR_READ | KEY_USR_VIEW,
-				KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
-	if (IS_ERR(keyring))
-		return PTR_ERR(keyring);
-
-	mk->mk_users = keyring;
-	return 0;
-}
-
-/*
- * Find the current user's "key" in the master key's ->mk_users.
- * Returns ERR_PTR(-ENOKEY) if not found.
- */
-static struct key *find_master_key_user(struct fscrypt_master_key *mk)
+/* Find the current user's claim in ->mk_users.  ->mk_sem must be held. */
+static struct fscrypt_master_key_user *
+find_master_key_user(struct fscrypt_master_key *mk)
 {
-	char description[FSCRYPT_MK_USER_DESCRIPTION_SIZE];
-	key_ref_t keyref;
+	struct fscrypt_master_key_user *mk_user;
+	kuid_t uid = current_fsuid();
 
-	format_mk_user_description(description, mk->mk_spec.u.identifier);
-
-	/*
-	 * We need to mark the keyring reference as "possessed" so that we
-	 * acquire permission to search it, via the KEY_POS_SEARCH permission.
-	 */
-	keyref = keyring_search(make_key_ref(mk->mk_users, true /*possessed*/),
-				&key_type_fscrypt_user, description, false);
-	if (IS_ERR(keyref)) {
-		if (PTR_ERR(keyref) == -EAGAIN || /* not found */
-		    PTR_ERR(keyref) == -EKEYREVOKED) /* recently invalidated */
-			keyref = ERR_PTR(-ENOKEY);
-		return ERR_CAST(keyref);
+	list_for_each_entry(mk_user, &mk->mk_users, link) {
+		if (uid_eq(mk_user->uid, uid))
+			return mk_user;
 	}
-	return key_ref_to_ptr(keyref);
+	return NULL;
 }
 
 /*
- * Give the current user a "key" in ->mk_users.  This charges the user's quota
+ * Give the current user a claim in ->mk_users.  This charges the user's quota
  * and marks the master key as added by the current user, so that it cannot be
  * removed by another user with the key.  Either ->mk_sem must be held for
  * write, or the master key must be still undergoing initialization.
  */
 static int add_master_key_user(struct fscrypt_master_key *mk)
 {
+	kuid_t uid = current_fsuid();
 	char description[FSCRYPT_MK_USER_DESCRIPTION_SIZE];
-	struct key *mk_user;
+	struct key *quota_key;
+	struct fscrypt_master_key_user *mk_user;
 	int err;
 
-	format_mk_user_description(description, mk->mk_spec.u.identifier);
-	mk_user = key_alloc(&key_type_fscrypt_user, description,
-			    current_fsuid(), current_gid(), current_cred(),
-			    KEY_POS_SEARCH | KEY_USR_VIEW, 0, NULL);
-	if (IS_ERR(mk_user))
-		return PTR_ERR(mk_user);
+	snprintf(description, sizeof(description), "%*phN.uid.%u",
+		 FSCRYPT_KEY_IDENTIFIER_SIZE, mk->mk_spec.u.identifier,
+		 __kuid_val(uid));
+	quota_key = key_alloc(&key_type_fscrypt_user, description, uid,
+			      current_gid(), current_cred(),
+			      KEY_POS_SEARCH | KEY_USR_VIEW, 0, NULL);
+	if (IS_ERR(quota_key))
+		return PTR_ERR(quota_key);
+
+	err = key_instantiate_and_link(quota_key, NULL, 0, NULL, NULL);
+	if (err) {
+		key_put(quota_key);
+		return err;
+	}
 
-	err = key_instantiate_and_link(mk_user, NULL, 0, mk->mk_users, NULL);
-	key_put(mk_user);
-	return err;
+	mk_user = kzalloc_obj(*mk_user);
+	if (!mk_user) {
+		key_put(quota_key);
+		return -ENOMEM;
+	}
+	mk_user->uid = uid;
+	mk_user->quota_key = quota_key;
+	list_add(&mk_user->link, &mk->mk_users);
+	return 0;
+}
+
+static void unlink_and_free_mk_user(struct fscrypt_master_key_user *mk_user)
+{
+	list_del(&mk_user->link);
+	key_put(mk_user->quota_key);
+	kfree(mk_user);
 }
 
 /*
- * Remove the current user's "key" from ->mk_users.
+ * Remove the current user's claim from ->mk_users.
  * ->mk_sem must be held for write.
  *
- * Returns 0 if removed, -ENOKEY if not found, or another -errno code.
+ * Returns 0 if removed or -ENOKEY if not found.
  */
 static int remove_master_key_user(struct fscrypt_master_key *mk)
 {
-	struct key *mk_user;
-	int err;
+	struct fscrypt_master_key_user *mk_user;
 
 	mk_user = find_master_key_user(mk);
-	if (IS_ERR(mk_user))
-		return PTR_ERR(mk_user);
-	err = key_unlink(mk->mk_users, mk_user);
-	key_put(mk_user);
-	return err;
+	if (!mk_user)
+		return -ENOKEY;
+	unlink_and_free_mk_user(mk_user);
+	return 0;
+}
+
+/*
+ * Clear ->mk_users.  Either ->mk_sem must be held for write, or 'mk' must have
+ * no structural references left.
+ */
+static void clear_mk_users(struct fscrypt_master_key *mk)
+{
+	struct fscrypt_master_key_user *mk_user, *tmp;
+
+	list_for_each_entry_safe(mk_user, tmp, &mk->mk_users, link)
+		unlink_and_free_mk_user(mk_user);
 }
 
 /*
  * Allocate a new fscrypt_master_key, transfer the given secret over to it, and
  * insert it into sb->s_master_keys.
@@ -443,19 +422,18 @@ static int add_new_master_key(struct super_block *sb,
 
 	init_rwsem(&mk->mk_sem);
 	refcount_set(&mk->mk_struct_refs, 1);
 	mk->mk_spec = *mk_spec;
 
+	INIT_LIST_HEAD(&mk->mk_users);
+
 	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)
-			goto out_put;
 		err = add_master_key_user(mk);
 		if (err)
 			goto out_put;
 	}
 
@@ -480,23 +458,17 @@ static int add_existing_master_key(struct fscrypt_master_key *mk,
 				   struct fscrypt_master_key_secret *secret)
 {
 	int err;
 
 	/*
-	 * If the current user is already in ->mk_users, then there's nothing to
-	 * do.  Otherwise, we need to add the user to ->mk_users.  (Neither is
-	 * applicable for v1 policy keys, which have NULL ->mk_users.)
+	 * For v2 policy keys (FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER): If the current
+	 * user is already in ->mk_users, then there's nothing to do.
+	 * Otherwise, add the user to ->mk_users.
 	 */
-	if (mk->mk_users) {
-		struct key *mk_user = find_master_key_user(mk);
-
-		if (mk_user != ERR_PTR(-ENOKEY)) {
-			if (IS_ERR(mk_user))
-				return PTR_ERR(mk_user);
-			key_put(mk_user);
+	if (mk->mk_spec.type == FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER) {
+		if (find_master_key_user(mk) != NULL)
 			return 0;
-		}
 		err = add_master_key_user(mk);
 		if (err)
 			return err;
 	}
 
@@ -890,11 +862,10 @@ int fscrypt_add_test_dummy_key(struct super_block *sb,
 int fscrypt_verify_key_added(struct super_block *sb,
 			     const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
 {
 	struct fscrypt_key_specifier mk_spec;
 	struct fscrypt_master_key *mk;
-	struct key *mk_user;
 	int err;
 
 	mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
 	memcpy(mk_spec.u.identifier, identifier, FSCRYPT_KEY_IDENTIFIER_SIZE);
 
@@ -902,17 +873,14 @@ int fscrypt_verify_key_added(struct super_block *sb,
 	if (!mk) {
 		err = -ENOKEY;
 		goto out;
 	}
 	down_read(&mk->mk_sem);
-	mk_user = find_master_key_user(mk);
-	if (IS_ERR(mk_user)) {
-		err = PTR_ERR(mk_user);
-	} else {
-		key_put(mk_user);
+	if (find_master_key_user(mk) != NULL)
 		err = 0;
-	}
+	else
+		err = -ENOKEY;
 	up_read(&mk->mk_sem);
 	fscrypt_put_master_key(mk);
 out:
 	if (err == -ENOKEY && capable(CAP_FOWNER))
 		err = 0;
@@ -1100,20 +1068,22 @@ static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users)
 	if (!mk)
 		return -ENOKEY;
 	down_write(&mk->mk_sem);
 
 	/* If relevant, remove current user's (or all users) claim to the key */
-	if (mk->mk_users && mk->mk_users->keys.nr_leaves_on_tree != 0) {
-		if (all_users)
-			err = keyring_clear(mk->mk_users);
-		else
+	if (!list_empty(&mk->mk_users)) {
+		if (all_users) {
+			clear_mk_users(mk);
+			err = 0;
+		} else {
 			err = remove_master_key_user(mk);
+		}
 		if (err) {
 			up_write(&mk->mk_sem);
 			goto out_put_key;
 		}
-		if (mk->mk_users->keys.nr_leaves_on_tree != 0) {
+		if (!list_empty(&mk->mk_users)) {
 			/*
 			 * Other users have still added the key too.  We removed
 			 * the current user's claim to the key, but we still
 			 * can't remove the key itself.
 			 */
@@ -1195,10 +1165,12 @@ EXPORT_SYMBOL_GPL(fscrypt_ioctl_remove_key_all_users);
 int fscrypt_ioctl_get_key_status(struct file *filp, void __user *uarg)
 {
 	struct super_block *sb = file_inode(filp)->i_sb;
 	struct fscrypt_get_key_status_arg arg;
 	struct fscrypt_master_key *mk;
+	kuid_t uid;
+	const struct fscrypt_master_key_user *mk_user;
 	int err;
 
 	if (copy_from_user(&arg, uarg, sizeof(arg)))
 		return -EFAULT;
 
@@ -1227,23 +1199,17 @@ int fscrypt_ioctl_get_key_status(struct file *filp, void __user *uarg)
 		err = 0;
 		goto out_release_key;
 	}
 
 	arg.status = FSCRYPT_KEY_STATUS_PRESENT;
-	if (mk->mk_users) {
-		struct key *mk_user;
 
-		arg.user_count = mk->mk_users->keys.nr_leaves_on_tree;
-		mk_user = find_master_key_user(mk);
-		if (!IS_ERR(mk_user)) {
+	uid = current_fsuid();
+	list_for_each_entry(mk_user, &mk->mk_users, link) {
+		arg.user_count++;
+		if (uid_eq(mk_user->uid, uid))
 			arg.status_flags |=
 				FSCRYPT_KEY_STATUS_FLAG_ADDED_BY_SELF;
-			key_put(mk_user);
-		} else if (mk_user != ERR_PTR(-ENOKEY)) {
-			err = PTR_ERR(mk_user);
-			goto out_release_key;
-		}
 	}
 	err = 0;
 out_release_key:
 	up_read(&mk->mk_sem);
 	fscrypt_put_master_key(mk);

base-commit: 83f1454877cc292b88baf13c829c16ce6937d120
prerequisite-patch-id: 319d2891e88c7df1ebb5ebf434d18b68f770399f
prerequisite-patch-id: 5330c9e4b65644baae81bd177a46be6223d2b494
-- 
2.54.0

Re: [PATCH] fscrypt: Replace mk_users keyring with simple list

[Luis Henriques]

Hi Eric!

On Thu, Jun 18 2026, Eric Biggers wrote:

> 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.
>
> Reported-by: syzbot+f55b043dacf43776b50c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f55b043dacf43776b50c
> Reported-by: Mohammed EL Kadiri <med08elkadiri@gmail.com>
> Closes: https://lore.kernel.org/keyrings/20260614150041.21172-1-med08elkadiri@gmail.com/
> Fixes: 23c688b54016 ("fscrypt: allow unprivileged users to add/remove keys for v2 policies")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
>
> I'm planning to take this via the fscrypt tree for 7.2

I was hoping to have some more time to test this patch, but I don't think
that will happen any time soon.

I've done a review of the patch and couldn't find any obvious problem.
Though, again, a more in-depth review would require more time as it has
been a while since I took a look into this code.

I just wonder if this is really stable material.  It's a bit intrusive
(doesn't even apply cleanly in mainline), but above all it's fixing a
lockdep false positive.  Other than syzbot, has this been seen in the
wild?

Cheers,
-- 
Luís

>
>  fs/crypto/fscrypt_private.h |  32 ++++--
>  fs/crypto/keyring.c         | 216 +++++++++++++++---------------------
>  2 files changed, 113 insertions(+), 135 deletions(-)
>
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index 4263cac24b32..0053b5c45412 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -494,10 +494,23 @@ fscrypt_is_key_prepared(const struct fscrypt_prepared_key *prep_key,
>  }
>  #endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
>  
>  /* 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
>   */
>  struct fscrypt_master_key_secret {
>  
> @@ -609,23 +622,22 @@ struct fscrypt_master_key {
>  	 * For v2 policy keys: a cryptographic hash of this key (->identifier).
>  	 */
>  	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
>  	 * inodes to be evicted efficiently if the key is removed.
>  	 */
> diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
> index 6ce6b436c34f..16bc348213ca 100644
> --- a/fs/crypto/keyring.c
> +++ b/fs/crypto/keyring.c
> @@ -63,26 +63,23 @@ static void fscrypt_free_master_key(struct rcu_head *head)
>  	 * Nevertheless, use kfree_sensitive() in case anything was missed.
>  	 */
>  	kfree_sensitive(mk);
>  }
>  
> +static void clear_mk_users(struct fscrypt_master_key *mk);
> +
>  void fscrypt_put_master_key(struct fscrypt_master_key *mk)
>  {
>  	if (!refcount_dec_and_test(&mk->mk_struct_refs))
>  		return;
>  	/*
> -	 * No structural references left, so free ->mk_users, and also free the
> +	 * No structural references left, so clear ->mk_users, and also free the
>  	 * fscrypt_master_key struct itself after an RCU grace period ensures
>  	 * that concurrent keyring lookups can no longer find it.
>  	 */
>  	WARN_ON_ONCE(refcount_read(&mk->mk_active_refs) != 0);
> -	if (mk->mk_users) {
> -		/* Clear the keyring so the quota gets released right away. */
> -		keyring_clear(mk->mk_users);
> -		key_put(mk->mk_users);
> -		mk->mk_users = NULL;
> -	}
> +	clear_mk_users(mk);
>  	call_rcu(&mk->mk_rcu_head, fscrypt_free_master_key);
>  }
>  
>  void fscrypt_put_master_key_activeref(struct super_block *sb,
>  				      struct fscrypt_master_key *mk)
> @@ -163,12 +160,12 @@ static void fscrypt_user_key_describe(const struct key *key, struct seq_file *m)
>  {
>  	seq_puts(m, key->description);
>  }
>  
>  /*
> - * Type of key in ->mk_users.  Each key of this type represents a particular
> - * user who has added a particular master key.
> + * Type of fscrypt_master_key_user::quota_key.  This contains no secret; it
> + * exists solely to charge a user's key quota.
>   *
>   * Note that the name of this key type really should be something like
>   * ".fscrypt-user" instead of simply ".fscrypt".  But the shorter name is chosen
>   * mainly for simplicity of presentation in /proc/keys when read by a non-root
>   * user.  And it is expected to be rare that a key is actually added by multiple
> @@ -178,34 +175,13 @@ static struct key_type key_type_fscrypt_user = {
>  	.name			= ".fscrypt",
>  	.instantiate		= fscrypt_user_key_instantiate,
>  	.describe		= fscrypt_user_key_describe,
>  };
>  
> -#define FSCRYPT_MK_USERS_DESCRIPTION_SIZE	\
> -	(CONST_STRLEN("fscrypt-") + 2 * FSCRYPT_KEY_IDENTIFIER_SIZE + \
> -	 CONST_STRLEN("-users") + 1)
> -
>  #define FSCRYPT_MK_USER_DESCRIPTION_SIZE	\
>  	(2 * FSCRYPT_KEY_IDENTIFIER_SIZE + CONST_STRLEN(".uid.") + 10 + 1)
>  
> -static void format_mk_users_keyring_description(
> -			char description[FSCRYPT_MK_USERS_DESCRIPTION_SIZE],
> -			const u8 mk_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
> -{
> -	sprintf(description, "fscrypt-%*phN-users",
> -		FSCRYPT_KEY_IDENTIFIER_SIZE, mk_identifier);
> -}
> -
> -static void format_mk_user_description(
> -			char description[FSCRYPT_MK_USER_DESCRIPTION_SIZE],
> -			const u8 mk_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
> -{
> -
> -	sprintf(description, "%*phN.uid.%u", FSCRYPT_KEY_IDENTIFIER_SIZE,
> -		mk_identifier, __kuid_val(current_fsuid()));
> -}
> -
>  /* Create ->s_master_keys if needed.  Synchronized by fscrypt_add_key_mutex. */
>  static int allocate_filesystem_keyring(struct super_block *sb)
>  {
>  	struct fscrypt_keyring *keyring;
>  
> @@ -336,95 +312,98 @@ fscrypt_find_master_key(struct super_block *sb,
>  out:
>  	rcu_read_unlock();
>  	return mk;
>  }
>  
> -static int allocate_master_key_users_keyring(struct fscrypt_master_key *mk)
> -{
> -	char description[FSCRYPT_MK_USERS_DESCRIPTION_SIZE];
> -	struct key *keyring;
> -
> -	format_mk_users_keyring_description(description,
> -					    mk->mk_spec.u.identifier);
> -	keyring = keyring_alloc(description, GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
> -				current_cred(), KEY_POS_SEARCH |
> -				  KEY_USR_SEARCH | KEY_USR_READ | KEY_USR_VIEW,
> -				KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
> -	if (IS_ERR(keyring))
> -		return PTR_ERR(keyring);
> -
> -	mk->mk_users = keyring;
> -	return 0;
> -}
> -
> -/*
> - * Find the current user's "key" in the master key's ->mk_users.
> - * Returns ERR_PTR(-ENOKEY) if not found.
> - */
> -static struct key *find_master_key_user(struct fscrypt_master_key *mk)
> +/* Find the current user's claim in ->mk_users.  ->mk_sem must be held. */
> +static struct fscrypt_master_key_user *
> +find_master_key_user(struct fscrypt_master_key *mk)
>  {
> -	char description[FSCRYPT_MK_USER_DESCRIPTION_SIZE];
> -	key_ref_t keyref;
> +	struct fscrypt_master_key_user *mk_user;
> +	kuid_t uid = current_fsuid();
>  
> -	format_mk_user_description(description, mk->mk_spec.u.identifier);
> -
> -	/*
> -	 * We need to mark the keyring reference as "possessed" so that we
> -	 * acquire permission to search it, via the KEY_POS_SEARCH permission.
> -	 */
> -	keyref = keyring_search(make_key_ref(mk->mk_users, true /*possessed*/),
> -				&key_type_fscrypt_user, description, false);
> -	if (IS_ERR(keyref)) {
> -		if (PTR_ERR(keyref) == -EAGAIN || /* not found */
> -		    PTR_ERR(keyref) == -EKEYREVOKED) /* recently invalidated */
> -			keyref = ERR_PTR(-ENOKEY);
> -		return ERR_CAST(keyref);
> +	list_for_each_entry(mk_user, &mk->mk_users, link) {
> +		if (uid_eq(mk_user->uid, uid))
> +			return mk_user;
>  	}
> -	return key_ref_to_ptr(keyref);
> +	return NULL;
>  }
>  
>  /*
> - * Give the current user a "key" in ->mk_users.  This charges the user's quota
> + * Give the current user a claim in ->mk_users.  This charges the user's quota
>   * and marks the master key as added by the current user, so that it cannot be
>   * removed by another user with the key.  Either ->mk_sem must be held for
>   * write, or the master key must be still undergoing initialization.
>   */
>  static int add_master_key_user(struct fscrypt_master_key *mk)
>  {
> +	kuid_t uid = current_fsuid();
>  	char description[FSCRYPT_MK_USER_DESCRIPTION_SIZE];
> -	struct key *mk_user;
> +	struct key *quota_key;
> +	struct fscrypt_master_key_user *mk_user;
>  	int err;
>  
> -	format_mk_user_description(description, mk->mk_spec.u.identifier);
> -	mk_user = key_alloc(&key_type_fscrypt_user, description,
> -			    current_fsuid(), current_gid(), current_cred(),
> -			    KEY_POS_SEARCH | KEY_USR_VIEW, 0, NULL);
> -	if (IS_ERR(mk_user))
> -		return PTR_ERR(mk_user);
> +	snprintf(description, sizeof(description), "%*phN.uid.%u",
> +		 FSCRYPT_KEY_IDENTIFIER_SIZE, mk->mk_spec.u.identifier,
> +		 __kuid_val(uid));
> +	quota_key = key_alloc(&key_type_fscrypt_user, description, uid,
> +			      current_gid(), current_cred(),
> +			      KEY_POS_SEARCH | KEY_USR_VIEW, 0, NULL);
> +	if (IS_ERR(quota_key))
> +		return PTR_ERR(quota_key);
> +
> +	err = key_instantiate_and_link(quota_key, NULL, 0, NULL, NULL);
> +	if (err) {
> +		key_put(quota_key);
> +		return err;
> +	}
>  
> -	err = key_instantiate_and_link(mk_user, NULL, 0, mk->mk_users, NULL);
> -	key_put(mk_user);
> -	return err;
> +	mk_user = kzalloc_obj(*mk_user);
> +	if (!mk_user) {
> +		key_put(quota_key);
> +		return -ENOMEM;
> +	}
> +	mk_user->uid = uid;
> +	mk_user->quota_key = quota_key;
> +	list_add(&mk_user->link, &mk->mk_users);
> +	return 0;
> +}
> +
> +static void unlink_and_free_mk_user(struct fscrypt_master_key_user *mk_user)
> +{
> +	list_del(&mk_user->link);
> +	key_put(mk_user->quota_key);
> +	kfree(mk_user);
>  }
>  
>  /*
> - * Remove the current user's "key" from ->mk_users.
> + * Remove the current user's claim from ->mk_users.
>   * ->mk_sem must be held for write.
>   *
> - * Returns 0 if removed, -ENOKEY if not found, or another -errno code.
> + * Returns 0 if removed or -ENOKEY if not found.
>   */
>  static int remove_master_key_user(struct fscrypt_master_key *mk)
>  {
> -	struct key *mk_user;
> -	int err;
> +	struct fscrypt_master_key_user *mk_user;
>  
>  	mk_user = find_master_key_user(mk);
> -	if (IS_ERR(mk_user))
> -		return PTR_ERR(mk_user);
> -	err = key_unlink(mk->mk_users, mk_user);
> -	key_put(mk_user);
> -	return err;
> +	if (!mk_user)
> +		return -ENOKEY;
> +	unlink_and_free_mk_user(mk_user);
> +	return 0;
> +}
> +
> +/*
> + * Clear ->mk_users.  Either ->mk_sem must be held for write, or 'mk' must have
> + * no structural references left.
> + */
> +static void clear_mk_users(struct fscrypt_master_key *mk)
> +{
> +	struct fscrypt_master_key_user *mk_user, *tmp;
> +
> +	list_for_each_entry_safe(mk_user, tmp, &mk->mk_users, link)
> +		unlink_and_free_mk_user(mk_user);
>  }
>  
>  /*
>   * Allocate a new fscrypt_master_key, transfer the given secret over to it, and
>   * insert it into sb->s_master_keys.
> @@ -443,19 +422,18 @@ static int add_new_master_key(struct super_block *sb,
>  
>  	init_rwsem(&mk->mk_sem);
>  	refcount_set(&mk->mk_struct_refs, 1);
>  	mk->mk_spec = *mk_spec;
>  
> +	INIT_LIST_HEAD(&mk->mk_users);
> +
>  	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)
> -			goto out_put;
>  		err = add_master_key_user(mk);
>  		if (err)
>  			goto out_put;
>  	}
>  
> @@ -480,23 +458,17 @@ static int add_existing_master_key(struct fscrypt_master_key *mk,
>  				   struct fscrypt_master_key_secret *secret)
>  {
>  	int err;
>  
>  	/*
> -	 * If the current user is already in ->mk_users, then there's nothing to
> -	 * do.  Otherwise, we need to add the user to ->mk_users.  (Neither is
> -	 * applicable for v1 policy keys, which have NULL ->mk_users.)
> +	 * For v2 policy keys (FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER): If the current
> +	 * user is already in ->mk_users, then there's nothing to do.
> +	 * Otherwise, add the user to ->mk_users.
>  	 */
> -	if (mk->mk_users) {
> -		struct key *mk_user = find_master_key_user(mk);
> -
> -		if (mk_user != ERR_PTR(-ENOKEY)) {
> -			if (IS_ERR(mk_user))
> -				return PTR_ERR(mk_user);
> -			key_put(mk_user);
> +	if (mk->mk_spec.type == FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER) {
> +		if (find_master_key_user(mk) != NULL)
>  			return 0;
> -		}
>  		err = add_master_key_user(mk);
>  		if (err)
>  			return err;
>  	}
>  
> @@ -890,11 +862,10 @@ int fscrypt_add_test_dummy_key(struct super_block *sb,
>  int fscrypt_verify_key_added(struct super_block *sb,
>  			     const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
>  {
>  	struct fscrypt_key_specifier mk_spec;
>  	struct fscrypt_master_key *mk;
> -	struct key *mk_user;
>  	int err;
>  
>  	mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
>  	memcpy(mk_spec.u.identifier, identifier, FSCRYPT_KEY_IDENTIFIER_SIZE);
>  
> @@ -902,17 +873,14 @@ int fscrypt_verify_key_added(struct super_block *sb,
>  	if (!mk) {
>  		err = -ENOKEY;
>  		goto out;
>  	}
>  	down_read(&mk->mk_sem);
> -	mk_user = find_master_key_user(mk);
> -	if (IS_ERR(mk_user)) {
> -		err = PTR_ERR(mk_user);
> -	} else {
> -		key_put(mk_user);
> +	if (find_master_key_user(mk) != NULL)
>  		err = 0;
> -	}
> +	else
> +		err = -ENOKEY;
>  	up_read(&mk->mk_sem);
>  	fscrypt_put_master_key(mk);
>  out:
>  	if (err == -ENOKEY && capable(CAP_FOWNER))
>  		err = 0;
> @@ -1100,20 +1068,22 @@ static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users)
>  	if (!mk)
>  		return -ENOKEY;
>  	down_write(&mk->mk_sem);
>  
>  	/* If relevant, remove current user's (or all users) claim to the key */
> -	if (mk->mk_users && mk->mk_users->keys.nr_leaves_on_tree != 0) {
> -		if (all_users)
> -			err = keyring_clear(mk->mk_users);
> -		else
> +	if (!list_empty(&mk->mk_users)) {
> +		if (all_users) {
> +			clear_mk_users(mk);
> +			err = 0;
> +		} else {
>  			err = remove_master_key_user(mk);
> +		}
>  		if (err) {
>  			up_write(&mk->mk_sem);
>  			goto out_put_key;
>  		}
> -		if (mk->mk_users->keys.nr_leaves_on_tree != 0) {
> +		if (!list_empty(&mk->mk_users)) {
>  			/*
>  			 * Other users have still added the key too.  We removed
>  			 * the current user's claim to the key, but we still
>  			 * can't remove the key itself.
>  			 */
> @@ -1195,10 +1165,12 @@ EXPORT_SYMBOL_GPL(fscrypt_ioctl_remove_key_all_users);
>  int fscrypt_ioctl_get_key_status(struct file *filp, void __user *uarg)
>  {
>  	struct super_block *sb = file_inode(filp)->i_sb;
>  	struct fscrypt_get_key_status_arg arg;
>  	struct fscrypt_master_key *mk;
> +	kuid_t uid;
> +	const struct fscrypt_master_key_user *mk_user;
>  	int err;
>  
>  	if (copy_from_user(&arg, uarg, sizeof(arg)))
>  		return -EFAULT;
>  
> @@ -1227,23 +1199,17 @@ int fscrypt_ioctl_get_key_status(struct file *filp, void __user *uarg)
>  		err = 0;
>  		goto out_release_key;
>  	}
>  
>  	arg.status = FSCRYPT_KEY_STATUS_PRESENT;
> -	if (mk->mk_users) {
> -		struct key *mk_user;
>  
> -		arg.user_count = mk->mk_users->keys.nr_leaves_on_tree;
> -		mk_user = find_master_key_user(mk);
> -		if (!IS_ERR(mk_user)) {
> +	uid = current_fsuid();
> +	list_for_each_entry(mk_user, &mk->mk_users, link) {
> +		arg.user_count++;
> +		if (uid_eq(mk_user->uid, uid))
>  			arg.status_flags |=
>  				FSCRYPT_KEY_STATUS_FLAG_ADDED_BY_SELF;
> -			key_put(mk_user);
> -		} else if (mk_user != ERR_PTR(-ENOKEY)) {
> -			err = PTR_ERR(mk_user);
> -			goto out_release_key;
> -		}
>  	}
>  	err = 0;
>  out_release_key:
>  	up_read(&mk->mk_sem);
>  	fscrypt_put_master_key(mk);
>
> base-commit: 83f1454877cc292b88baf13c829c16ce6937d120
> prerequisite-patch-id: 319d2891e88c7df1ebb5ebf434d18b68f770399f
> prerequisite-patch-id: 5330c9e4b65644baae81bd177a46be6223d2b494
> -- 
> 2.54.0
>

Re: [PATCH] fscrypt: Replace mk_users keyring with simple list

[Eric Biggers]

On Fri, Jun 26, 2026 at 09:16:35AM +0100, Luis Henriques wrote:
> Hi Eric!
> 
> On Thu, Jun 18 2026, Eric Biggers wrote:
> 
> > 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.
> >
> > Reported-by: syzbot+f55b043dacf43776b50c@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=f55b043dacf43776b50c
> > Reported-by: Mohammed EL Kadiri <med08elkadiri@gmail.com>
> > Closes: https://lore.kernel.org/keyrings/20260614150041.21172-1-med08elkadiri@gmail.com/
> > Fixes: 23c688b54016 ("fscrypt: allow unprivileged users to add/remove keys for v2 policies")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > ---
> >
> > I'm planning to take this via the fscrypt tree for 7.2
> 
> I was hoping to have some more time to test this patch, but I don't think
> that will happen any time soon.
> 
> I've done a review of the patch and couldn't find any obvious problem.
> Though, again, a more in-depth review would require more time as it has
> been a while since I took a look into this code.
> 
> I just wonder if this is really stable material.  It's a bit intrusive
> (doesn't even apply cleanly in mainline), but above all it's fixing a
> lockdep false positive.  Other than syzbot, has this been seen in the
> wild?

Thanks!

It applies on top of
"[PATCH] fscrypt: Fix key setup in edge case with multiple data unit sizes"
(https://lore.kernel.org/linux-fscrypt/20260618180652.52742-1-ebiggers@kernel.org/).
This time I tried just relying on the prerequisite-patch-id footer (as
generated by 'git format-patch') to express the dependency.  But
evidently that still doesn't work: for one, 'b4 am' just ignores it.

Since this patch has "Reported-by: syzbot" as well as "Fixes", the
stable maintainers will apply it anyway.  If I actually wanted to stop
that, I'd have to actively oppose the backport, likely multiple times
indefinitely since people will continue to try to backport it.  And
syzkaller would continue to get the lockdep warning on stable kernels.

So I'd rather just get it out the way and backport it the same time as
"fscrypt: Fix key setup in edge case with multiple data unit sizes",
which similarly tweaks some data structures in struct fscrypt_master_key
and needs to be backported too.  "fscrypt: stop using keyrings subsystem
for fscrypt_master_key" several years ago was backported too.

- Eric

Re: [PATCH] fscrypt: Replace mk_users keyring with simple list

[Eric Biggers]

On Fri, Jun 26, 2026 at 07:02:32PM +0000, Eric Biggers wrote:
> On Fri, Jun 26, 2026 at 09:16:35AM +0100, Luis Henriques wrote:
> > Hi Eric!
> > 
> > On Thu, Jun 18 2026, Eric Biggers wrote:
> > 
> > > 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.
> > >
> > > Reported-by: syzbot+f55b043dacf43776b50c@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=f55b043dacf43776b50c
> > > Reported-by: Mohammed EL Kadiri <med08elkadiri@gmail.com>
> > > Closes: https://lore.kernel.org/keyrings/20260614150041.21172-1-med08elkadiri@gmail.com/
> > > Fixes: 23c688b54016 ("fscrypt: allow unprivileged users to add/remove keys for v2 policies")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > > ---
> > >
> > > I'm planning to take this via the fscrypt tree for 7.2
> > 
> > I was hoping to have some more time to test this patch, but I don't think
> > that will happen any time soon.
> > 
> > I've done a review of the patch and couldn't find any obvious problem.
> > Though, again, a more in-depth review would require more time as it has
> > been a while since I took a look into this code.
> > 
> > I just wonder if this is really stable material.  It's a bit intrusive
> > (doesn't even apply cleanly in mainline), but above all it's fixing a
> > lockdep false positive.  Other than syzbot, has this been seen in the
> > wild?
> 
> Thanks!
> 
> It applies on top of
> "[PATCH] fscrypt: Fix key setup in edge case with multiple data unit sizes"
> (https://lore.kernel.org/linux-fscrypt/20260618180652.52742-1-ebiggers@kernel.org/).
> This time I tried just relying on the prerequisite-patch-id footer (as
> generated by 'git format-patch') to express the dependency.  But
> evidently that still doesn't work: for one, 'b4 am' just ignores it.
> 
> Since this patch has "Reported-by: syzbot" as well as "Fixes", the
> stable maintainers will apply it anyway.  If I actually wanted to stop
> that, I'd have to actively oppose the backport, likely multiple times
> indefinitely since people will continue to try to backport it.  And
> syzkaller would continue to get the lockdep warning on stable kernels.
> 
> So I'd rather just get it out the way and backport it the same time as
> "fscrypt: Fix key setup in edge case with multiple data unit sizes",
> which similarly tweaks some data structures in struct fscrypt_master_key
> and needs to be backported too.  "fscrypt: stop using keyrings subsystem
> for fscrypt_master_key" several years ago was backported too.

FWIW, I would also not be surprised if the old code would also fail
fuzzing in other ways, like keyctl() being used to directly manipulate
the keyrings from underneath what fs/crypto/ assumes.  I remember at
least considering that scenario when adding this code years ago, but I
think the reasoning was quite subtle and I may have missed something.

The 'struct key' keyrings just have a lot of obscure sharp corners.
Whereas simple lists, hash tables, etc. are much easier to evaluate.

- Eric