[RFC PATCH v3 00/11] KEYS: Sanitize the garbage collector

[RFC PATCH v3 00/11] KEYS: Sanitize the garbage collector

[Jarkko Sakkinen]

1. Replace key_type_dead retyping with module reference pinning so live
   keys and restrictions keep their backing key_type module loaded until
   key_gc_unused_keys() or keyring_destroy() completes.
2. Process serial-tree scans and graveyard destruction in fixed 64-key
   chunks to a bound workqueue execution latency without hurting overall
   throughput for a significant amount.

The first part has a user-space visible effect: rmmod returns -EBUSY while
keys or keyring restrictions still pin the backing module.

Internal key caches or transport state bottlenecks are addressed by either
clearing their private caches on last-client teardown or splitting key
types into dedicated provider modules.

CHANGES
=======

v3:
- Split dns_resolver and rxrpc key types into dedicated provider modules
  (dns_resolver_key.ko and rxrpc_keys.ko) instead of client refcounting,
  so cached or retained keys do not pin dns_resolver.ko or rxrpc.ko.
- Dropped the standalone nvme-keyring fix as it was picked up by the
  subsystem maintainers.
- Hardened key_gc_flush() to serialize against chunked key_gc_work
  continuations, and fixed a restriction module reference leak on failed
  keyring instantiation.

v2:
- Used v1 as a test probe for Sashiko to detect the corner cases instead
  of trying to resolve even the known ones beforehand.
- Converted the client subsystems to be compatible with the module
  pinning semantics.

VERSIONS
========

- v2:
  https://lore.kernel.org/linux-integrity/20261103030226.2685251-1-jarkko@kernel.org/

Cc: keyrings@vger.kernel.org
Cc: linux-afs@lists.infradead.org
Cc: linux-cifs@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-nfs@vger.kernel.org
Cc: linux-s390@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: samba-technical@lists.samba.org
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Anna Schumaker <anna@kernel.org>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Biggers <ebiggers@kernel.org>
Cc: Eric Dumazet <edumazet@kernel.org>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Marc Dionne <marc.dionne@auristor.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Paulo Alcantara <pc@manguebit.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Sumit Garg <sumit.garg@kernel.org>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Trond Myklebust <trondmy@kernel.org>
Cc: Vasily Gorbik <gor@linux.ibm.com>

Jarkko Sakkinen (11):
  KEYS: Add key_gc_flush() for draining RCU callbacks
  dns_resolver: Move the key type into a separate module
  NFSv4: Clear idmap keyring cache when the last client unloads
  smb: client: Clear idmap/SPNEGO caches when the last session is
    unloaded
  s390/cert_store: Start tracking subsystem keys
  KEYS: trusted: TEE: Disable sysfs bind attributes
  KEYS: trusted: TEE: Synchronize session lifetime
  rxrpc: Move key types to their own module
  KEYS: Pin the backing kernel module for a key type
  KEYS: Remove key_type_dead
  KEYS: key_garbage_collector(): process in chunks

 arch/s390/kernel/cert_store.c             | 161 +++++-------
 fs/nfs/nfs4idmap.c                        |  21 ++
 fs/smb/client/cifs_spnego.c               |   6 +
 fs/smb/client/cifsacl.c                   |   6 +
 fs/smb/client/cifsproto.h                 |   8 +
 fs/smb/client/misc.c                      |  19 +-
 include/linux/key-type.h                  |   6 +-
 include/linux/key.h                       |   2 +
 net/Makefile                              |   4 +-
 net/dns_resolver/Kconfig                  |  15 ++
 net/dns_resolver/Makefile                 |   4 +-
 net/dns_resolver/dns_key.c                |  65 +----
 net/dns_resolver/dns_query.c              |  64 +++++
 net/dns_resolver/internal.h               |   5 -
 net/rxrpc/Kconfig                         |  20 ++
 net/rxrpc/Makefile                        |   7 +-
 net/rxrpc/af_rxrpc.c                      |  19 +-
 net/rxrpc/ar-internal.h                   |  25 +-
 net/rxrpc/key.c                           | 122 +++------
 net/rxrpc/rxgk.c                          |  66 -----
 net/rxrpc/rxkad.c                         |  47 ----
 net/rxrpc/security.c                      | 184 ++++++++++++++
 net/rxrpc/server_key.c                    | 260 ++++++++++++--------
 security/keys/gc.c                        | 287 ++++++++++------------
 security/keys/internal.h                  |   7 +-
 security/keys/key.c                       |  61 +++--
 security/keys/keyring.c                   |  88 +++----
 security/keys/trusted-keys/trusted_core.c |  15 +-
 security/keys/trusted-keys/trusted_tee.c  |  89 +++++--
 29 files changed, 906 insertions(+), 777 deletions(-)

--
2.47.3

[RFC PATCH v3 01/11] KEYS: Add key_gc_flush() for draining RCU callbacks

[Jarkko Sakkinen]

keyring_clear() and key_invalidate() release keyring links via call_rcu(),
and key_put() defers final key destruction to key_gc_work.

Once keys pin their type, callers that clear an internal cache before
module unload need a synchronous barrier so module references can be
dropped before delete_module() checks the refcount.

Add key_gc_flush() for flushing the remaining key_gc_work passes.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 include/linux/key.h |  2 ++
 security/keys/gc.c  | 11 +++++++++++
 2 files changed, 13 insertions(+)

diff --git a/include/linux/key.h b/include/linux/key.h
index 420412aab926..0f408255f740 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -439,6 +439,7 @@ extern int restrict_link_reject(struct key *keyring,
 				struct key *restriction_key);
 
 extern int keyring_clear(struct key *keyring);
+void key_gc_flush(void);
 
 extern key_ref_t keyring_search(key_ref_t keyring,
 				struct key_type *type,
@@ -526,6 +527,7 @@ extern void key_init(void);
 #define key_free_user_ns(ns)		do { } while(0)
 #define key_remove_domain(d)		do { } while(0)
 #define key_lookup(k)			NULL
+#define key_gc_flush()			do { } while (0)
 
 #endif /* CONFIG_KEYS */
 #endif /* __KERNEL__ */
diff --git a/security/keys/gc.c b/security/keys/gc.c
index eda445f815d4..2c3eddc0443a 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -21,6 +21,17 @@ unsigned key_gc_delay = 5 * 60;
 static void key_garbage_collector(struct work_struct *work);
 DECLARE_WORK(key_gc_work, key_garbage_collector);
 
+/**
+ * key_gc_flush - Drain the garbage collector RCU callbacks
+ */
+void key_gc_flush(void)
+{
+	do {
+		rcu_barrier();
+	} while (flush_work(&key_gc_work));
+}
+EXPORT_SYMBOL_GPL(key_gc_flush);
+
 /*
  * Reaper for links from keyrings to dead keys.
  */
-- 
2.47.3

[RFC PATCH v3 02/11] dns_resolver: Move the key type into a separate module

[Jarkko Sakkinen]

Once keys pin to the kernel module of their key type, having the DNS key
type in the same modules as the resolver will introduce a circular
dependency preventing module_exit() from ever running.

Introduce dns_resolver_key.ko containing key_type_dns_resolver. On a module
exit, drop the credentials and call key_gc_flush() in order the drain the
remaining RCU callbacks.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 net/Makefile                 |  2 +-
 net/dns_resolver/Kconfig     | 15 +++++++++
 net/dns_resolver/Makefile    |  4 ++-
 net/dns_resolver/dns_key.c   | 65 +++++-------------------------------
 net/dns_resolver/dns_query.c | 64 +++++++++++++++++++++++++++++++++++
 net/dns_resolver/internal.h  |  5 ---
 6 files changed, 92 insertions(+), 63 deletions(-)

diff --git a/net/Makefile b/net/Makefile
index 5b2dd7f07a85..69c16a7851b4 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -55,7 +55,7 @@ obj-$(CONFIG_IEEE802154)	+= ieee802154/
 obj-$(CONFIG_MAC802154)		+= mac802154/
 
 obj-$(CONFIG_SYSCTL)		+= sysctl_net.o
-obj-$(CONFIG_DNS_RESOLVER)	+= dns_resolver/
+obj-$(CONFIG_DNS_KEY)		+= dns_resolver/
 obj-$(CONFIG_CEPH_LIB)		+= ceph/
 obj-$(CONFIG_BATMAN_ADV)	+= batman-adv/
 obj-$(CONFIG_NFC)		+= nfc/
diff --git a/net/dns_resolver/Kconfig b/net/dns_resolver/Kconfig
index 7c2dba273e35..2618bad2e771 100644
--- a/net/dns_resolver/Kconfig
+++ b/net/dns_resolver/Kconfig
@@ -1,10 +1,25 @@
 # SPDX-License-Identifier: GPL-2.0-only
+
+#
+# Configuration for DNS Resolver key type
+#
+config DNS_KEY
+	tristate "DNS Resolver key type"
+	depends on KEYS
+	help
+	  Support the key type used to cache DNS lookups independently of
+	  the in-kernel DNS query service.
+
+	  To compile this as a module, choose M here: the module is called
+	  dns_resolver_key.
+
 #
 # Configuration for DNS Resolver
 #
 config DNS_RESOLVER
 	tristate "DNS Resolver support"
 	depends on KEYS
+	select DNS_KEY
 	help
 	  Saying Y here will include support for the DNS Resolver key type
 	  which can be used to make upcalls to perform DNS lookups in
diff --git a/net/dns_resolver/Makefile b/net/dns_resolver/Makefile
index 877532d662d0..586e4c6514de 100644
--- a/net/dns_resolver/Makefile
+++ b/net/dns_resolver/Makefile
@@ -3,6 +3,8 @@
 # Makefile for the Linux DNS Resolver.
 #
 
+obj-$(CONFIG_DNS_KEY) += dns_resolver_key.o
 obj-$(CONFIG_DNS_RESOLVER) += dns_resolver.o
 
-dns_resolver-y :=  dns_key.o dns_query.o
+dns_resolver-y := dns_query.o
+dns_resolver_key-y := dns_key.o
diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index 451247864a63..fecbdc1d6beb 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -9,8 +9,8 @@
  *              Wang Lei (wang840925@gmail.com)
  *		David Howells (dhowells@redhat.com)
  */
+
 #include <linux/module.h>
-#include <linux/moduleparam.h>
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/kernel.h>
@@ -22,15 +22,12 @@
 #include <keys/user-type.h>
 #include "internal.h"
 
-MODULE_DESCRIPTION("DNS Resolver");
+MODULE_DESCRIPTION("DNS Resolver key type");
 MODULE_AUTHOR("Wang Lei");
 MODULE_LICENSE("GPL");
 
 unsigned int dns_resolver_debug;
-module_param_named(debug, dns_resolver_debug, uint, 0644);
-MODULE_PARM_DESC(debug, "DNS Resolver debugging mask");
-
-const struct cred *dns_resolver_cache;
+EXPORT_SYMBOL_GPL(dns_resolver_debug);
 
 #define	DNS_ERRORNO_OPTION	"dnserror"
 
@@ -318,61 +315,17 @@ struct key_type key_type_dns_resolver = {
 	.describe	= dns_resolver_describe,
 	.read		= dns_resolver_read,
 };
+EXPORT_SYMBOL_GPL(key_type_dns_resolver);
 
-static int __init init_dns_resolver(void)
+static int __init init_dns_key(void)
 {
-	struct cred *cred;
-	struct key *keyring;
-	int ret;
-
-	/* create an override credential set with a special thread keyring in
-	 * which DNS requests are cached
-	 *
-	 * this is used to prevent malicious redirections from being installed
-	 * with add_key().
-	 */
-	cred = prepare_kernel_cred(&init_task);
-	if (!cred)
-		return -ENOMEM;
-
-	keyring = keyring_alloc(".dns_resolver",
-				GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, cred,
-				(KEY_POS_ALL & ~KEY_POS_SETATTR) |
-				KEY_USR_VIEW | KEY_USR_READ,
-				KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
-	if (IS_ERR(keyring)) {
-		ret = PTR_ERR(keyring);
-		goto failed_put_cred;
-	}
-
-	ret = register_key_type(&key_type_dns_resolver);
-	if (ret < 0)
-		goto failed_put_key;
-
-	/* instruct request_key() to use this special keyring as a cache for
-	 * the results it looks up */
-	set_bit(KEY_FLAG_ROOT_CAN_CLEAR, &keyring->flags);
-	cred->thread_keyring = keyring;
-	cred->jit_keyring = KEY_REQKEY_DEFL_THREAD_KEYRING;
-	dns_resolver_cache = cred;
-
-	kdebug("DNS resolver keyring: %d\n", key_serial(keyring));
-	return 0;
-
-failed_put_key:
-	key_put(keyring);
-failed_put_cred:
-	put_cred(cred);
-	return ret;
+	return register_key_type(&key_type_dns_resolver);
 }
 
-static void __exit exit_dns_resolver(void)
+static void __exit exit_dns_key(void)
 {
-	key_revoke(dns_resolver_cache->thread_keyring);
 	unregister_key_type(&key_type_dns_resolver);
-	put_cred(dns_resolver_cache);
 }
 
-module_init(init_dns_resolver)
-module_exit(exit_dns_resolver)
-MODULE_LICENSE("GPL");
+module_init(init_dns_key);
+module_exit(exit_dns_key);
diff --git a/net/dns_resolver/dns_query.c b/net/dns_resolver/dns_query.c
index b8fb9a1245bf..ae4dde190101 100644
--- a/net/dns_resolver/dns_query.c
+++ b/net/dns_resolver/dns_query.c
@@ -24,8 +24,10 @@
  */
 
 #include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/slab.h>
 #include <linux/cred.h>
+#include <linux/keyctl.h>
 #include <linux/dns_resolver.h>
 #include <linux/err.h>
 #include <net/net_namespace.h>
@@ -35,6 +37,8 @@
 
 #include "internal.h"
 
+static const struct cred *dns_resolver_cache;
+
 /**
  * dns_query - Query the DNS
  * @net: The network namespace to operate in.
@@ -137,3 +141,63 @@ int dns_query(struct net *net,
 	return ret;
 }
 EXPORT_SYMBOL(dns_query);
+
+static int __init init_dns_resolver(void)
+{
+	struct cred *cred;
+	struct key *keyring;
+	int ret;
+
+	/*
+	 * Create an override credential set with a special thread keyring in
+	 * which DNS requests are cached.  This prevents malicious redirections
+	 * from being installed with add_key().
+	 */
+	cred = prepare_kernel_cred(&init_task);
+	if (!cred)
+		return -ENOMEM;
+
+	keyring = keyring_alloc(".dns_resolver",
+				GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, cred,
+				(KEY_POS_ALL & ~KEY_POS_SETATTR) |
+				KEY_USR_VIEW | KEY_USR_READ,
+				KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
+	if (IS_ERR(keyring)) {
+		ret = PTR_ERR(keyring);
+		goto failed_put_cred;
+	}
+
+	/*
+	 * Instruct request_key() to use this special keyring as a cache for
+	 * the results it looks up.
+	 */
+	set_bit(KEY_FLAG_ROOT_CAN_CLEAR, &keyring->flags);
+	cred->thread_keyring = keyring;
+	cred->jit_keyring = KEY_REQKEY_DEFL_THREAD_KEYRING;
+	dns_resolver_cache = cred;
+
+	kdebug("DNS resolver keyring: %d\n", key_serial(keyring));
+	return 0;
+
+failed_put_cred:
+	put_cred(cred);
+	return ret;
+}
+
+static void __exit exit_dns_resolver(void)
+{
+	key_revoke(dns_resolver_cache->thread_keyring);
+	put_cred(dns_resolver_cache);
+	/* Drain deferred credential and keyring destruction. */
+	key_gc_flush();
+}
+
+module_init(init_dns_resolver);
+module_exit(exit_dns_resolver);
+
+MODULE_DESCRIPTION("DNS Resolver");
+MODULE_AUTHOR("Wang Lei");
+MODULE_LICENSE("GPL");
+
+module_param_named(debug, dns_resolver_debug, uint, 0644);
+MODULE_PARM_DESC(debug, "DNS Resolver debugging mask");
diff --git a/net/dns_resolver/internal.h b/net/dns_resolver/internal.h
index d0d8edcea092..3478bad78a9a 100644
--- a/net/dns_resolver/internal.h
+++ b/net/dns_resolver/internal.h
@@ -18,11 +18,6 @@ enum {
 	dns_key_error,
 };
 
-/*
- * dns_key.c
- */
-extern const struct cred *dns_resolver_cache;
-
 /*
  * debug tracing
  */
-- 
2.47.3

[RFC PATCH v3 03/11] NFSv4: Clear idmap keyring cache when the last client unloads

[Jarkko Sakkinen]

id_resolver_cache->thread_keyring caches id_resolver_legacy keys up until
nfs_idmap_quit() tears down the keyring.

Once keys pin to the kernel module of their key type, the cached keys
prevent module_exit() from ever happening.

Track the active idmap instances under a mutex and clear the idmap cache
keyring once the last client has been destroyed.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 fs/nfs/nfs4idmap.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
index bc397110d977..c88444cc6bf6 100644
--- a/fs/nfs/nfs4idmap.c
+++ b/fs/nfs/nfs4idmap.c
@@ -47,6 +47,8 @@
 #include <keys/request_key_auth-type.h>
 #include <linux/module.h>
 #include <linux/user_namespace.h>
+#include <linux/mutex.h>
+#include <linux/ratelimit.h>
 
 #include "internal.h"
 #include "netns.h"
@@ -58,6 +60,10 @@
 static const struct cred *id_resolver_cache;
 static struct key_type key_type_id_resolver_legacy;
 
+/* Serializes the idmap cache. */
+static DEFINE_MUTEX(nfs_idmap_cache_mutex);
+static unsigned int nfs_idmap_users;
+
 struct idmap_legacy_upcalldata {
 	struct rpc_pipe_msg pipe_msg;
 	struct idmap_msg idmap_msg;
@@ -469,7 +475,10 @@ nfs_idmap_new(struct nfs_client *clp)
 	if (error)
 		goto err_destroy_pipe;
 
+	mutex_lock(&nfs_idmap_cache_mutex);
+	nfs_idmap_users++;
 	clp->cl_idmap = idmap;
+	mutex_unlock(&nfs_idmap_cache_mutex);
 	return 0;
 err_destroy_pipe:
 	rpc_destroy_pipe_data(idmap->idmap_pipe);
@@ -483,6 +492,7 @@ void
 nfs_idmap_delete(struct nfs_client *clp)
 {
 	struct idmap *idmap = clp->cl_idmap;
+	int ret;
 
 	if (!idmap)
 		return;
@@ -493,6 +503,17 @@ nfs_idmap_delete(struct nfs_client *clp)
 	rpc_destroy_pipe_data(idmap->idmap_pipe);
 	put_user_ns(idmap->user_ns);
 	kfree(idmap);
+
+	mutex_lock(&nfs_idmap_cache_mutex);
+	if (--nfs_idmap_users == 0) {
+		ret = keyring_clear(id_resolver_cache->thread_keyring);
+		if (ret)
+			pr_warn_ratelimited("NFS: Failed to clear idmap cache: %d; cache pins remain\n",
+					    ret);
+		else
+			key_gc_flush();
+	}
+	mutex_unlock(&nfs_idmap_cache_mutex);
 }
 
 static int nfs_idmap_prepare_message(char *desc, struct idmap *idmap,
-- 
2.47.3

[RFC PATCH v3 04/11] smb: client: Clear idmap/SPNEGO caches when the last session is unloaded

[Jarkko Sakkinen]

CIFS caches cifs.idmap and cifs.spnego keys in the module-owned keyrings
that are only destroyed in module_exit().

Once keys pin the kernel module of the key type, the cached entries will
retain CIFS pinned after the last SMB session has been torned down.

Serialize the session count transitions with a mutex and clear the idmap
and SPNEGO cache keyrings when the last session is freed.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 fs/smb/client/cifs_spnego.c |  6 ++++++
 fs/smb/client/cifsacl.c     |  6 ++++++
 fs/smb/client/cifsproto.h   |  8 ++++++++
 fs/smb/client/misc.c        | 19 +++++++++++++++++--
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/cifs_spnego.c b/fs/smb/client/cifs_spnego.c
index 44c407275680..939e14e755e5 100644
--- a/fs/smb/client/cifs_spnego.c
+++ b/fs/smb/client/cifs_spnego.c
@@ -190,6 +190,12 @@ cifs_get_spnego_key(struct cifs_ses *sesInfo,
 	return spnego_key;
 }
 
+void
+cifs_spnego_clear_cache(void)
+{
+	WARN_ON(keyring_clear(spnego_cred->thread_keyring));
+}
+
 int
 init_cifs_spnego(void)
 {
diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
index 346899da91eb..126b976c2547 100644
--- a/fs/smb/client/cifsacl.c
+++ b/fs/smb/client/cifsacl.c
@@ -539,6 +539,12 @@ sid_to_id(struct cifs_sb_info *cifs_sb, struct smb_sid *psid,
 	return 0;
 }
 
+void
+cifs_idmap_clear_cache(void)
+{
+	WARN_ON(keyring_clear(root_cred->thread_keyring));
+}
+
 int
 init_cifs_idmap(void)
 {
diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index f9ea35597bc2..2930a40d1b67 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -58,8 +58,16 @@ do {									\
 } while (0)
 int init_cifs_idmap(void);
 void exit_cifs_idmap(void);
+void cifs_idmap_clear_cache(void);
 int init_cifs_spnego(void);
 void exit_cifs_spnego(void);
+#ifdef CONFIG_CIFS_UPCALL
+void cifs_spnego_clear_cache(void);
+#else
+static inline void cifs_spnego_clear_cache(void)
+{
+}
+#endif
 const char *build_path_from_dentry(struct dentry *direntry, void *page);
 char *__build_path_from_dentry_optional_prefix(struct dentry *direntry,
 					       void *page, const char *tree,
diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
index ec28f102ea08..11c0330be489 100644
--- a/fs/smb/client/misc.c
+++ b/fs/smb/client/misc.c
@@ -9,6 +9,8 @@
 #include <linux/slab.h>
 #include <linux/ctype.h>
 #include <linux/mempool.h>
+#include <linux/key.h>
+#include <linux/mutex.h>
 #include <linux/vmalloc.h>
 #include "cifsglob.h"
 #include "cifsproto.h"
@@ -33,6 +35,9 @@ struct tcon_list {
 	struct cifs_tcon *tcon;
 };
 
+/* Serialize session admission with the last session's cache clear. */
+static DEFINE_MUTEX(cifs_session_mutex);
+
 /* The xid serves as a useful identifier for each incoming vfs request,
    in a similar way to the mid which is useful to track each sent smb,
    and CurrentXid can also provide a running counter (although it
@@ -74,7 +79,6 @@ sesInfoAlloc(void)
 
 	ret_buf = kzalloc_obj(struct cifs_ses);
 	if (ret_buf) {
-		atomic_inc(&sesInfoAllocCount);
 		spin_lock_init(&ret_buf->ses_lock);
 		ret_buf->ses_status = SES_NEW;
 		++ret_buf->ses_count;
@@ -84,6 +88,10 @@ sesInfoAlloc(void)
 		spin_lock_init(&ret_buf->iface_lock);
 		INIT_LIST_HEAD(&ret_buf->iface_list);
 		spin_lock_init(&ret_buf->chan_lock);
+
+		mutex_lock(&cifs_session_mutex);
+		atomic_inc(&sesInfoAllocCount);
+		mutex_unlock(&cifs_session_mutex);
 	}
 	return ret_buf;
 }
@@ -99,7 +107,6 @@ sesInfoFree(struct cifs_ses *buf_to_free)
 	}
 
 	unload_nls(buf_to_free->local_nls);
-	atomic_dec(&sesInfoAllocCount);
 	kfree(buf_to_free->serverOS);
 	kfree(buf_to_free->serverDomain);
 	kfree(buf_to_free->serverNOS);
@@ -115,6 +122,14 @@ sesInfoFree(struct cifs_ses *buf_to_free)
 		kref_put(&iface->refcount, release_iface);
 	spin_unlock(&buf_to_free->iface_lock);
 	kfree_sensitive(buf_to_free);
+
+	mutex_lock(&cifs_session_mutex);
+	if (atomic_dec_and_test(&sesInfoAllocCount)) {
+		cifs_idmap_clear_cache();
+		cifs_spnego_clear_cache();
+		key_gc_flush();
+	}
+	mutex_unlock(&cifs_session_mutex);
 }
 
 struct cifs_tcon *
-- 
2.47.3

[RFC PATCH v3 05/11] s390/cert_store: Start tracking subsystem keys

[Jarkko Sakkinen]

cert_store refreshes its keyring by unregistering and then registering
again key_type_cert_store_key. Once keys pin to the kernel module of their
key type, this is no longer necessary.

Register key_type_cert_store_key once during init before creating sysfs
attributes, and track subsystem-created keys explicitly so that the refresh
operationunlinks and invalidates previous entries directly.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 arch/s390/kernel/cert_store.c | 161 ++++++++++++++--------------------
 1 file changed, 67 insertions(+), 94 deletions(-)

diff --git a/arch/s390/kernel/cert_store.c b/arch/s390/kernel/cert_store.c
index 998f2da56175..ab1a78cdfd4c 100644
--- a/arch/s390/kernel/cert_store.c
+++ b/arch/s390/kernel/cert_store.c
@@ -17,6 +17,7 @@
 #include <linux/key.h>
 #include <linux/keyctl.h>
 #include <linux/kobject.h>
+#include <linux/list.h>
 #include <linux/module.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
@@ -296,6 +297,18 @@ static int check_certificate_valid(const struct vce *vce)
 	return check_certificate_hash(vce);
 }
 
+struct cert_store_key {
+	struct list_head list;
+	struct key *key;
+};
+
+static DEFINE_MUTEX(cs_refresh_lock);
+/* Owned references, protected by cs_refresh_lock. */
+static LIST_HEAD(cs_keys);
+static struct key *active_cs_keyring;
+static struct key *cs_keyring_parent;
+
+/* Return an owned reference to the user session keyring. */
 static struct key *get_user_session_keyring(void)
 {
 	key_ref_t us_keyring_ref;
@@ -305,118 +318,66 @@ static struct key *get_user_session_keyring(void)
 	if (IS_ERR(us_keyring_ref)) {
 		pr_dbf_msg("Couldn't get user session keyring: %ld",
 			   PTR_ERR(us_keyring_ref));
-		return ERR_PTR(-ENOKEY);
+		return ERR_CAST(us_keyring_ref);
 	}
-	key_ref_put(us_keyring_ref);
 	return key_ref_to_ptr(us_keyring_ref);
 }
 
-/* Invalidate all keys from cert_store keyring. */
-static int invalidate_keyring_keys(struct key *keyring)
-{
-	unsigned long num_keys, key_index;
-	size_t keyring_payload_len;
-	key_serial_t *key_array;
-	struct key *current_key;
-	int rc;
-
-	keyring_payload_len = key_type_keyring.read(keyring, NULL, 0);
-	num_keys = keyring_payload_len / sizeof(key_serial_t);
-	key_array = kzalloc_objs(key_serial_t, num_keys);
-	if (!key_array)
-		return -ENOMEM;
-
-	rc = key_type_keyring.read(keyring, (char *)key_array, keyring_payload_len);
-	if (rc != keyring_payload_len) {
-		pr_dbf_msg("Couldn't read keyring payload");
-		goto out;
-	}
-
-	for (key_index = 0; key_index < num_keys; key_index++) {
-		current_key = key_lookup(key_array[key_index]);
-		pr_dbf_msg("Invalidating key %08x", current_key->serial);
-
-		key_invalidate(current_key);
-		key_put(current_key);
-		rc = key_unlink(keyring, current_key);
-		if (rc) {
-			pr_dbf_msg("Couldn't unlink key %08x: %d", current_key->serial, rc);
-			break;
-		}
-	}
-out:
-	kfree(key_array);
-	return rc;
-}
-
-static struct key *find_cs_keyring(void)
-{
-	key_ref_t cs_keyring_ref;
-	struct key *cs_keyring;
-
-	cs_keyring_ref = keyring_search(make_key_ref(get_user_session_keyring(), true),
-					&key_type_keyring, CERT_STORE_KEYRING_NAME,
-					false);
-	if (!IS_ERR(cs_keyring_ref)) {
-		cs_keyring = key_ref_to_ptr(cs_keyring_ref);
-		key_ref_put(cs_keyring_ref);
-		goto found;
-	}
-	/* Search default locations: thread, process, session keyrings */
-	cs_keyring = request_key(&key_type_keyring, CERT_STORE_KEYRING_NAME, NULL);
-	if (IS_ERR(cs_keyring))
-		return NULL;
-	key_put(cs_keyring);
-found:
-	return cs_keyring;
-}
-
 static void cleanup_cs_keys(void)
 {
-	struct key *cs_keyring;
+	struct cert_store_key *entry, *tmp;
+	int rc;
 
-	cs_keyring = find_cs_keyring();
-	if (!cs_keyring)
+	if (!active_cs_keyring)
 		return;
 
-	pr_dbf_msg("Found cert_store keyring. Purging...");
-	/*
-	 * Remove cert_store_key_type in case invalidation
-	 * of old cert_store keys failed (= severe error).
-	 */
-	if (invalidate_keyring_keys(cs_keyring))
-		unregister_key_type(&key_type_cert_store_key);
+	/* Invalidate even certificates unlinked or linked into other keyrings. */
+	list_for_each_entry_safe(entry, tmp, &cs_keys, list) {
+		key_invalidate(entry->key);
+		key_put(entry->key);
+		list_del(&entry->list);
+		kfree(entry);
+	}
 
-	keyring_clear(cs_keyring);
-	key_invalidate(cs_keyring);
-	key_put(cs_keyring);
-	key_unlink(get_user_session_keyring(), cs_keyring);
+	rc = keyring_clear(active_cs_keyring);
+	if (rc)
+		pr_dbf_msg("Couldn't clear cert_store keyring: %d", rc);
+	key_invalidate(active_cs_keyring);
+	rc = key_unlink(cs_keyring_parent, active_cs_keyring);
+	if (rc)
+		pr_dbf_msg("Couldn't unlink cert_store keyring: %d", rc);
+	key_put(active_cs_keyring);
+	key_put(cs_keyring_parent);
+	active_cs_keyring = NULL;
+	cs_keyring_parent = NULL;
 }
 
+/* Return a borrowed reference; cleanup_cs_keys() owns the keyring. */
 static struct key *create_cs_keyring(void)
 {
-	static struct key *cs_keyring;
+	struct key *cs_keyring, *parent;
 
 	/* Cleanup previous cs_keyring and all associated keys if any. */
 	cleanup_cs_keys();
+	parent = get_user_session_keyring();
+	if (IS_ERR(parent))
+		return parent;
+
 	cs_keyring = keyring_alloc(CERT_STORE_KEYRING_NAME, GLOBAL_ROOT_UID,
 				   GLOBAL_ROOT_GID, current_cred(),
 				   (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW | KEY_USR_READ,
 				   KEY_ALLOC_NOT_IN_QUOTA | KEY_ALLOC_SET_KEEP,
-				   NULL, get_user_session_keyring());
+				   NULL, parent);
 	if (IS_ERR(cs_keyring)) {
 		pr_dbf_msg("Can't allocate cert_store keyring");
-		return NULL;
+		key_put(parent);
+		return cs_keyring;
 	}
 
+	active_cs_keyring = cs_keyring;
+	cs_keyring_parent = parent;
 	pr_dbf_msg("Successfully allocated cert_store keyring: %08x", cs_keyring->serial);
 
-	/*
-	 * In case a previous clean-up ran into an
-	 * error and unregistered key type.
-	 */
-	register_key_type(&key_type_cert_store_key);
-
 	return cs_keyring;
 }
 
@@ -454,6 +415,7 @@ static char *get_key_description(struct vcssb *vcssb, const struct vce *vce)
 static int create_key_from_vce(struct vcssb *vcssb, struct vce *vce,
 			       struct key *keyring)
 {
+	struct cert_store_key *entry;
 	key_ref_t newkey;
 	char *desc;
 	int rc;
@@ -462,6 +424,13 @@ static int create_key_from_vce(struct vcssb *vcssb, struct vce *vce,
 	if (!desc)
 		return -ENOMEM;
 
+	/* Reserve cleanup tracking before the key can be linked elsewhere. */
+	entry = kmalloc_obj(*entry);
+	if (!entry) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
 	newkey = key_create_or_update(
 		make_key_ref(keyring, true), CERT_STORE_KEY_TYPE_NAME,
 		desc, (u8 *)vce + vce->vce_hdr.vc_offset,
@@ -473,10 +442,12 @@ static int create_key_from_vce(struct vcssb *vcssb, struct vce *vce,
 	if (rc) {
 		pr_dbf_msg("Couldn't create a key from Certificate Entry (%d)", rc);
 		rc = -ENOKEY;
+		kfree(entry);
 		goto out;
 	}
 
-	key_ref_put(newkey);
+	entry->key = key_ref_to_ptr(newkey);
+	list_add_tail(&entry->list, &cs_keys);
 out:
 	kfree(desc);
 	return rc;
@@ -704,19 +675,18 @@ static int fill_cs_keyring(void)
 	if (rc)
 		goto cleanup_keys;
 
-	rc = -ENOMEM;
 	cs_keyring = create_cs_keyring();
-	if (!cs_keyring)
+	if (IS_ERR(cs_keyring)) {
+		rc = PTR_ERR(cs_keyring);
 		goto cleanup_keys;
+	}
 
 	rc = add_certificates_to_keyring(vcssb, cs_keyring);
 	if (rc)
-		goto cleanup_cs_keyring;
+		goto cleanup_keys;
 
 	goto out;
 
-cleanup_cs_keyring:
-	key_put(cs_keyring);
 cleanup_keys:
 	cleanup_cs_keys();
 out:
@@ -724,7 +694,6 @@ static int fill_cs_keyring(void)
 	return rc;
 }
 
-static DEFINE_MUTEX(cs_refresh_lock);
 static int cs_status_val = -1;
 
 static ssize_t cs_status_show(struct kobject *kobj,
@@ -794,14 +763,18 @@ static int __init cert_store_init(void)
 	if (!cert_store_kobj)
 		goto cleanup_dbf;
 
-	rc = sysfs_create_files(cert_store_kobj, cert_store_attrs);
+	rc = register_key_type(&key_type_cert_store_key);
 	if (rc)
 		goto cleanup_kobj;
 
-	register_key_type(&key_type_cert_store_key);
+	rc = sysfs_create_files(cert_store_kobj, cert_store_attrs);
+	if (rc)
+		goto cleanup_key_type;
 
 	return rc;
 
+cleanup_key_type:
+	unregister_key_type(&key_type_cert_store_key);
 cleanup_kobj:
 	kobject_put(cert_store_kobj);
 cleanup_dbf:
-- 
2.47.3

[RFC PATCH v3 06/11] KEYS: trusted: TEE: Disable sysfs bind attributes

[Jarkko Sakkinen]

Suppress sysfs bind and unbind attributes on trusted_key_driver so that
userspace cannot unbind the driver and unregister key_type_trusted while
the trusted module remains loaded.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 security/keys/trusted-keys/trusted_tee.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
index 6e465c8bef5e..227efb668aa9 100644
--- a/security/keys/trusted-keys/trusted_tee.c
+++ b/security/keys/trusted-keys/trusted_tee.c
@@ -263,7 +263,8 @@ static struct tee_client_driver trusted_key_driver = {
 	.remove		= trusted_key_remove,
 	.id_table	= trusted_key_id_table,
 	.driver		= {
-		.name		= DRIVER_NAME,
+		.name			= DRIVER_NAME,
+		.suppress_bind_attrs	= true,
 	},
 };
 
-- 
2.47.3

[RFC PATCH v3 07/11] KEYS: trusted: TEE: Synchronize session lifetime

[Jarkko Sakkinen]

While the sysfs bind attributes are now suppressed, the parent TEE device
can still remove the device while the instances keep trusted.ko loaded.

Update the static calls before invoking backend init() and reset them in
the case of a failure.

In the TEE backend, guard session context state with a mutex, publish
pvt_data.dev before registering key_type_trusted and sanitize pvt_data on
probe failure and removal.

When an active TEE has not been bound, reject seal, unseal and get_random
with -ENODEV.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 security/keys/trusted-keys/trusted_core.c | 15 ++--
 security/keys/trusted-keys/trusted_tee.c  | 86 +++++++++++++++++------
 2 files changed, 75 insertions(+), 26 deletions(-)

diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index 0509d9955f2a..11d7d296d063 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -373,14 +373,19 @@ static int __init init_trusted(void)
 		if (!get_random)
 			get_random = kernel_get_random;
 
+		static_call_update(trusted_key_seal, trusted_key_sources[i].ops->seal);
+		static_call_update(trusted_key_unseal, trusted_key_sources[i].ops->unseal);
+		static_call_update(trusted_key_get_random, get_random);
+		migratable = trusted_key_sources[i].ops->migratable;
+
 		ret = trusted_key_sources[i].ops->init();
 		if (!ret) {
-			static_call_update(trusted_key_seal, trusted_key_sources[i].ops->seal);
-			static_call_update(trusted_key_unseal, trusted_key_sources[i].ops->unseal);
-			static_call_update(trusted_key_get_random, get_random);
-
 			trusted_key_exit = trusted_key_sources[i].ops->exit;
-			migratable = trusted_key_sources[i].ops->migratable;
+		} else {
+			static_call_update(trusted_key_seal, NULL);
+			static_call_update(trusted_key_unseal, NULL);
+			static_call_update(trusted_key_get_random, NULL);
+			migratable = 0;
 		}
 
 		if (!ret || ret != -ENODEV)
diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
index 227efb668aa9..87ea549f7f0b 100644
--- a/security/keys/trusted-keys/trusted_tee.c
+++ b/security/keys/trusted-keys/trusted_tee.c
@@ -9,6 +9,7 @@
 #include <linux/err.h>
 #include <linux/key-type.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/tee_drv.h>
@@ -56,6 +57,8 @@ struct trusted_key_tee_private {
 };
 
 static struct trusted_key_tee_private pvt_data;
+/* Do not hold this mutex across key type registration or unregistration. */
+static DEFINE_MUTEX(trusted_key_tee_mutex);
 
 /*
  * Have the TEE seal(encrypt) the symmetric key
@@ -67,6 +70,12 @@ static int trusted_tee_seal(struct trusted_key_payload *p, char *datablob)
 	struct tee_param param[4];
 	struct tee_shm *reg_shm = NULL;
 
+	mutex_lock(&trusted_key_tee_mutex);
+	if (!pvt_data.ctx) {
+		ret = -ENODEV;
+		goto out_unlock;
+	}
+
 	memset(&inv_arg, 0, sizeof(inv_arg));
 	memset(&param, 0, sizeof(param));
 
@@ -74,7 +83,8 @@ static int trusted_tee_seal(struct trusted_key_payload *p, char *datablob)
 					      sizeof(p->key) + sizeof(p->blob));
 	if (IS_ERR(reg_shm)) {
 		dev_err(pvt_data.dev, "shm register failed\n");
-		return PTR_ERR(reg_shm);
+		ret = PTR_ERR(reg_shm);
+		goto out_unlock;
 	}
 
 	inv_arg.func = TA_CMD_SEAL;
@@ -100,7 +110,8 @@ static int trusted_tee_seal(struct trusted_key_payload *p, char *datablob)
 	}
 
 	tee_shm_free(reg_shm);
-
+out_unlock:
+	mutex_unlock(&trusted_key_tee_mutex);
 	return ret;
 }
 
@@ -114,6 +125,12 @@ static int trusted_tee_unseal(struct trusted_key_payload *p, char *datablob)
 	struct tee_param param[4];
 	struct tee_shm *reg_shm = NULL;
 
+	mutex_lock(&trusted_key_tee_mutex);
+	if (!pvt_data.ctx) {
+		ret = -ENODEV;
+		goto out_unlock;
+	}
+
 	memset(&inv_arg, 0, sizeof(inv_arg));
 	memset(&param, 0, sizeof(param));
 
@@ -121,7 +138,8 @@ static int trusted_tee_unseal(struct trusted_key_payload *p, char *datablob)
 					      sizeof(p->key) + sizeof(p->blob));
 	if (IS_ERR(reg_shm)) {
 		dev_err(pvt_data.dev, "shm register failed\n");
-		return PTR_ERR(reg_shm);
+		ret = PTR_ERR(reg_shm);
+		goto out_unlock;
 	}
 
 	inv_arg.func = TA_CMD_UNSEAL;
@@ -147,7 +165,8 @@ static int trusted_tee_unseal(struct trusted_key_payload *p, char *datablob)
 	}
 
 	tee_shm_free(reg_shm);
-
+out_unlock:
+	mutex_unlock(&trusted_key_tee_mutex);
 	return ret;
 }
 
@@ -161,13 +180,20 @@ static int trusted_tee_get_random(unsigned char *key, size_t key_len)
 	struct tee_param param[4];
 	struct tee_shm *reg_shm = NULL;
 
+	mutex_lock(&trusted_key_tee_mutex);
+	if (!pvt_data.ctx) {
+		ret = -ENODEV;
+		goto out_unlock;
+	}
+
 	memset(&inv_arg, 0, sizeof(inv_arg));
 	memset(&param, 0, sizeof(param));
 
 	reg_shm = tee_shm_register_kernel_buf(pvt_data.ctx, key, key_len);
 	if (IS_ERR(reg_shm)) {
 		dev_err(pvt_data.dev, "key shm register failed\n");
-		return PTR_ERR(reg_shm);
+		ret = PTR_ERR(reg_shm);
+		goto out_unlock;
 	}
 
 	inv_arg.func = TA_CMD_GET_RANDOM;
@@ -189,7 +215,8 @@ static int trusted_tee_get_random(unsigned char *key, size_t key_len)
 	}
 
 	tee_shm_free(reg_shm);
-
+out_unlock:
+	mutex_unlock(&trusted_key_tee_mutex);
 	return ret;
 }
 
@@ -205,50 +232,67 @@ static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
 static int trusted_key_probe(struct tee_client_device *rng_device)
 {
 	struct device *dev = &rng_device->dev;
+	struct tee_context *ctx;
 	int ret;
 	struct tee_ioctl_open_session_arg sess_arg;
 
+	mutex_lock(&trusted_key_tee_mutex);
+	if (pvt_data.ctx) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
 	memset(&sess_arg, 0, sizeof(sess_arg));
 
-	pvt_data.ctx = tee_client_open_context(NULL, optee_ctx_match, NULL,
-					       NULL);
-	if (IS_ERR(pvt_data.ctx))
-		return -ENODEV;
+	ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL);
+	if (IS_ERR(ctx)) {
+		ret = -ENODEV;
+		goto out_unlock;
+	}
 
 	memcpy(sess_arg.uuid, rng_device->id.uuid.b, TEE_IOCTL_UUID_LEN);
 	sess_arg.clnt_login = TEE_IOCTL_LOGIN_REE_KERNEL;
 	sess_arg.num_params = 0;
 
-	ret = tee_client_open_session(pvt_data.ctx, &sess_arg, NULL);
+	ret = tee_client_open_session(ctx, &sess_arg, NULL);
 	if ((ret < 0) || (sess_arg.ret != 0)) {
 		dev_err(dev, "tee_client_open_session failed, err: %x\n",
 			sess_arg.ret);
 		ret = -EINVAL;
 		goto out_ctx;
 	}
+	pvt_data.ctx = ctx;
 	pvt_data.session_id = sess_arg.session;
-
-	ret = register_key_type(&key_type_trusted);
-	if (ret < 0)
-		goto out_sess;
-
 	pvt_data.dev = dev;
+	mutex_unlock(&trusted_key_tee_mutex);
 
-	return 0;
+	ret = register_key_type(&key_type_trusted);
+	if (!ret)
+		return 0;
 
-out_sess:
-	tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
+	mutex_lock(&trusted_key_tee_mutex);
+	tee_client_close_session(ctx, pvt_data.session_id);
 out_ctx:
-	tee_client_close_context(pvt_data.ctx);
-
+	tee_client_close_context(ctx);
+	pvt_data.ctx = NULL;
+	pvt_data.dev = NULL;
+	pvt_data.session_id = 0;
+out_unlock:
+	mutex_unlock(&trusted_key_tee_mutex);
 	return ret;
 }
 
 static void trusted_key_remove(struct tee_client_device *dev)
 {
 	unregister_key_type(&key_type_trusted);
+
+	mutex_lock(&trusted_key_tee_mutex);
 	tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
 	tee_client_close_context(pvt_data.ctx);
+	pvt_data.ctx = NULL;
+	pvt_data.dev = NULL;
+	pvt_data.session_id = 0;
+	mutex_unlock(&trusted_key_tee_mutex);
 }
 
 static const struct tee_client_device_id trusted_key_id_table[] = {
-- 
2.47.3

[RFC PATCH v3 08/11] rxrpc: Move key types to their own module

[Jarkko Sakkinen]

Once keys pin the kernel module implementing their key type, hosting rxrpc
and rxrpc_s keys inside the same module prevents unloading the transport
module while keys remain in keyrings or cached connections.

Split the key types and their preparse logic into a separate rxrpc_keys.ko
module that rxrpc.ko depends on, and flush key GC on rxrpc exit.

By doing this, rxrpc.ko can be removed while the persistent keys pin only
rxrpc_keys.ko.

Co-developed-by: David Howells <dhowells@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 net/Makefile            |   2 +-
 net/rxrpc/Kconfig       |  20 ++++
 net/rxrpc/Makefile      |   7 +-
 net/rxrpc/af_rxrpc.c    |  19 +--
 net/rxrpc/ar-internal.h |  25 +---
 net/rxrpc/key.c         | 122 ++++++-------------
 net/rxrpc/rxgk.c        |  66 ----------
 net/rxrpc/rxkad.c       |  47 --------
 net/rxrpc/security.c    | 184 ++++++++++++++++++++++++++++
 net/rxrpc/server_key.c  | 260 ++++++++++++++++++++++++----------------
 10 files changed, 410 insertions(+), 342 deletions(-)

diff --git a/net/Makefile b/net/Makefile
index 69c16a7851b4..97b2bd2ace7b 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -30,7 +30,7 @@ obj-$(CONFIG_LAPB)		+= lapb/
 obj-$(CONFIG_CAN)		+= can/
 obj-$(CONFIG_BT)		+= bluetooth/
 obj-$(CONFIG_SUNRPC)		+= sunrpc/
-obj-$(CONFIG_AF_RXRPC)		+= rxrpc/
+obj-$(CONFIG_RXRPC_KEYS)	+= rxrpc/
 obj-$(CONFIG_AF_KCM)		+= kcm/
 obj-$(CONFIG_STREAM_PARSER)	+= strparser/
 obj-$(CONFIG_ATM)		+= atm/
diff --git a/net/rxrpc/Kconfig b/net/rxrpc/Kconfig
index 2cebb666dc07..30135ae62bd6 100644
--- a/net/rxrpc/Kconfig
+++ b/net/rxrpc/Kconfig
@@ -3,6 +3,18 @@
 # RxRPC session sockets
 #
 
+config RXRPC_KEYS
+	tristate "RxRPC key types"
+	select CRYPTO
+	select CRYPTO_LIB_DES if RXKAD
+	select KEYS
+	help
+	  Say Y or M here to include support for rxrpc and rxrpc_s key types
+	  independently of RxRPC session sockets.
+
+	  To compile this as a module, choose M here: the module will be called
+	  rxrpc_keys.
+
 config AF_RXRPC
 	tristate "RxRPC session sockets"
 	depends on INET
@@ -10,6 +22,7 @@ config AF_RXRPC
 	select CRYPTO_LIB_DES if RXKAD
 	select KEYS
 	select NET_UDP_TUNNEL
+	select RXRPC_KEYS
 	help
 	  Say Y or M here to include support for RxRPC session sockets (just
 	  the transport part, not the presentation part: (un)marshalling is
@@ -54,6 +67,9 @@ config AF_RXRPC_DEBUG
 
 	  See Documentation/networking/rxrpc.rst.
 
+endif
+
+if RXRPC_KEYS
 
 config RXKAD
 	bool "RxRPC Kerberos security"
@@ -86,6 +102,10 @@ config RXGK
 
 	  See Documentation/networking/rxrpc.rst.
 
+endif
+
+if AF_RXRPC
+
 config RXPERF
 	tristate "RxRPC test service"
 	help
diff --git a/net/rxrpc/Makefile b/net/rxrpc/Makefile
index f994f9f30a29..196e0ce085ba 100644
--- a/net/rxrpc/Makefile
+++ b/net/rxrpc/Makefile
@@ -3,6 +3,7 @@
 # Makefile for Linux kernel RxRPC
 #
 
+obj-$(CONFIG_RXRPC_KEYS) += rxrpc_keys.o
 obj-$(CONFIG_AF_RXRPC) += rxrpc.o
 
 rxrpc-y := \
@@ -19,7 +20,6 @@ rxrpc-y := \
 	input_rack.o \
 	insecure.o \
 	io_thread.o \
-	key.o \
 	local_event.o \
 	local_object.o \
 	misc.o \
@@ -32,7 +32,6 @@ rxrpc-y := \
 	rtt.o \
 	security.o \
 	sendmsg.o \
-	server_key.o \
 	skbuff.o \
 	txbuf.o \
 	utils.o
@@ -45,5 +44,9 @@ rxrpc-$(CONFIG_RXGK) += \
 	rxgk_app.o \
 	rxgk_kdf.o
 
+rxrpc_keys-y := \
+	key.o \
+	server_key.o
+
 obj-$(CONFIG_RXPERF) += rxperf.o
 obj-$(CONFIG_KUNIT) += tests/
diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 9ab0f22c881e..6ccb99a992b9 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -1077,18 +1077,6 @@ static int __init af_rxrpc_init(void)
 		goto error_sock;
 	}
 
-	ret = register_key_type(&key_type_rxrpc);
-	if (ret < 0) {
-		pr_crit("Cannot register client key type\n");
-		goto error_key_type;
-	}
-
-	ret = register_key_type(&key_type_rxrpc_s);
-	if (ret < 0) {
-		pr_crit("Cannot register server key type\n");
-		goto error_key_type_s;
-	}
-
 	ret = rxrpc_sysctl_init();
 	if (ret < 0) {
 		pr_crit("Cannot register sysctls\n");
@@ -1098,10 +1086,6 @@ static int __init af_rxrpc_init(void)
 	return 0;
 
 error_sysctls:
-	unregister_key_type(&key_type_rxrpc_s);
-error_key_type_s:
-	unregister_key_type(&key_type_rxrpc);
-error_key_type:
 	sock_unregister(PF_RXRPC);
 error_sock:
 	proto_unregister(&rxrpc_proto);
@@ -1124,8 +1108,6 @@ static void __exit af_rxrpc_exit(void)
 {
 	_enter("");
 	rxrpc_sysctl_exit();
-	unregister_key_type(&key_type_rxrpc_s);
-	unregister_key_type(&key_type_rxrpc);
 	sock_unregister(PF_RXRPC);
 	proto_unregister(&rxrpc_proto);
 	unregister_pernet_device(&rxrpc_net_ops);
@@ -1139,6 +1121,7 @@ static void __exit af_rxrpc_exit(void)
 	destroy_workqueue(rxrpc_workqueue);
 	rxrpc_exit_security();
 	kmem_cache_destroy(rxrpc_call_jar);
+	key_gc_flush();
 	_leave("");
 }
 
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 865f05fe37ab..b5deab0e1ed9 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -273,18 +273,6 @@ struct rxrpc_security {
 	/* Clean up a security service */
 	void (*exit)(void);
 
-	/* Parse the information from a server key */
-	int (*preparse_server_key)(struct key_preparsed_payload *);
-
-	/* Clean up the preparse buffer after parsing a server key */
-	void (*free_preparse_server_key)(struct key_preparsed_payload *);
-
-	/* Destroy the payload of a server key */
-	void (*destroy_server_key)(struct key *);
-
-	/* Describe a server key */
-	void (*describe_server_key)(const struct key *, struct seq_file *);
-
 	/* initialise a connection's security */
 	int (*init_connection_security)(struct rxrpc_connection *,
 					struct rxrpc_key_token *);
@@ -1307,10 +1295,6 @@ extern const struct rxrpc_security rxrpc_no_security;
  */
 extern struct key_type key_type_rxrpc;
 
-int rxrpc_request_key(struct rxrpc_sock *, sockptr_t , int);
-int rxrpc_get_server_data_key(struct rxrpc_connection *, const void *, time64_t,
-			      u32);
-
 /*
  * local_event.c
  */
@@ -1492,6 +1476,11 @@ const struct rxrpc_security *rxrpc_get_incoming_security(struct rxrpc_sock *,
 							 struct sk_buff *);
 struct key *rxrpc_look_up_server_security(struct rxrpc_connection *,
 					  struct sk_buff *, u32, u32);
+int rxrpc_request_key(struct rxrpc_sock *rx, sockptr_t optval, int optlen);
+int rxrpc_get_server_data_key(struct rxrpc_connection *conn,
+			      const void *session_key, time64_t expiry,
+			      u32 kvno);
+int rxrpc_server_keyring(struct rxrpc_sock *rx, sockptr_t optval, int optlen);
 
 /*
  * sendmsg.c
@@ -1505,8 +1494,6 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *, struct msghdr *, size_t);
  */
 extern struct key_type key_type_rxrpc_s;
 
-int rxrpc_server_keyring(struct rxrpc_sock *, sockptr_t, int);
-
 /*
  * skbuff.c
  */
@@ -1633,7 +1620,7 @@ extern unsigned int rxrpc_debug;
 #define _leave(FMT,...)	kleave(FMT,##__VA_ARGS__)
 #define _debug(FMT,...)	kdebug(FMT,##__VA_ARGS__)
 
-#elif defined(CONFIG_AF_RXRPC_DEBUG)
+#elif defined(CONFIG_AF_RXRPC_DEBUG) && !defined(RXRPC_KEY_PROVIDER)
 #define RXRPC_DEBUG_KENTER	0x01
 #define RXRPC_DEBUG_KLEAVE	0x02
 #define RXRPC_DEBUG_KDEBUG	0x04
diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index cbd26da44951..49d0871beaf9 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -9,6 +9,7 @@
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#define RXRPC_KEY_PROVIDER
 
 #include <linux/module.h>
 #include <linux/net.h>
@@ -615,89 +616,6 @@ static void rxrpc_describe(const struct key *key, struct seq_file *m)
 	}
 }
 
-/*
- * grab the security key for a socket
- */
-int rxrpc_request_key(struct rxrpc_sock *rx, sockptr_t optval, int optlen)
-{
-	struct key *key;
-	char *description;
-
-	_enter("");
-
-	if (optlen <= 0 || optlen > PAGE_SIZE - 1 || rx->key)
-		return -EINVAL;
-
-	description = memdup_sockptr_nul(optval, optlen);
-	if (IS_ERR(description))
-		return PTR_ERR(description);
-
-	key = request_key_net(&key_type_rxrpc, description, sock_net(&rx->sk), NULL);
-	if (IS_ERR(key)) {
-		kfree(description);
-		_leave(" = %ld", PTR_ERR(key));
-		return PTR_ERR(key);
-	}
-
-	rx->key = key;
-	kfree(description);
-	_leave(" = 0 [key %x]", key->serial);
-	return 0;
-}
-
-/*
- * generate a server data key
- */
-int rxrpc_get_server_data_key(struct rxrpc_connection *conn,
-			      const void *session_key,
-			      time64_t expiry,
-			      u32 kvno)
-{
-	const struct cred *cred = current_cred();
-	struct key *key;
-	int ret;
-
-	struct {
-		u32 kver;
-		struct rxrpc_key_data_v1 v1;
-	} data;
-
-	_enter("");
-
-	key = key_alloc(&key_type_rxrpc, "x",
-			GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, cred, 0,
-			KEY_ALLOC_NOT_IN_QUOTA, NULL);
-	if (IS_ERR(key)) {
-		_leave(" = -ENOMEM [alloc %ld]", PTR_ERR(key));
-		return -ENOMEM;
-	}
-
-	_debug("key %d", key_serial(key));
-
-	data.kver = 1;
-	data.v1.security_index = RXRPC_SECURITY_RXKAD;
-	data.v1.ticket_length = 0;
-	data.v1.expiry = rxrpc_time64_to_u32(expiry);
-	data.v1.kvno = 0;
-
-	memcpy(&data.v1.session_key, session_key, sizeof(data.v1.session_key));
-
-	ret = key_instantiate_and_link(key, &data, sizeof(data), NULL, NULL);
-	if (ret < 0)
-		goto error;
-
-	conn->key = key;
-	_leave(" = 0 [%d]", key_serial(key));
-	return 0;
-
-error:
-	key_revoke(key);
-	key_put(key);
-	_leave(" = -ENOMEM [ins %d]", ret);
-	return -ENOMEM;
-}
-EXPORT_SYMBOL(rxrpc_get_server_data_key);
-
 /**
  * rxrpc_get_null_key - Generate a null RxRPC key
  * @keyname: The name to give the key.
@@ -893,3 +811,41 @@ static long rxrpc_read(const struct key *key,
 	_leave(" = %zu", size);
 	return size;
 }
+
+static int __init rxrpc_keys_init(void)
+{
+	int ret;
+
+	ret = register_key_type(&key_type_rxrpc);
+	if (ret < 0) {
+		pr_crit("Cannot register client key type\n");
+		goto error_key_type;
+	}
+
+	ret = register_key_type(&key_type_rxrpc_s);
+	if (ret < 0) {
+		pr_crit("Cannot register server key type\n");
+		goto error_key_type_s;
+	}
+
+	return 0;
+
+error_key_type_s:
+	unregister_key_type(&key_type_rxrpc);
+
+error_key_type:
+	return ret;
+}
+
+static void __exit rxrpc_keys_exit(void)
+{
+	unregister_key_type(&key_type_rxrpc_s);
+	unregister_key_type(&key_type_rxrpc);
+}
+
+module_init(rxrpc_keys_init);
+module_exit(rxrpc_keys_exit);
+
+MODULE_DESCRIPTION("RxRPC key types");
+MODULE_AUTHOR("David Howells <dhowells@redhat.com>");
+MODULE_LICENSE("GPL");
diff --git a/net/rxrpc/rxgk.c b/net/rxrpc/rxgk.c
index 77a67ace1d24..5fb3ce15cac4 100644
--- a/net/rxrpc/rxgk.c
+++ b/net/rxrpc/rxgk.c
@@ -14,68 +14,6 @@
 #include "ar-internal.h"
 #include "rxgk_common.h"
 
-/*
- * Parse the information from a server key
- */
-static int rxgk_preparse_server_key(struct key_preparsed_payload *prep)
-{
-	const struct krb5_enctype *krb5;
-	struct krb5_buffer *server_key = (void *)&prep->payload.data[2];
-	unsigned int service, sec_class, kvno, enctype;
-	int n = 0;
-
-	_enter("%zu", prep->datalen);
-
-	if (sscanf(prep->orig_description, "%u:%u:%u:%u%n",
-		   &service, &sec_class, &kvno, &enctype, &n) != 4)
-		return -EINVAL;
-
-	if (prep->orig_description[n])
-		return -EINVAL;
-
-	krb5 = crypto_krb5_find_enctype(enctype);
-	if (!krb5)
-		return -ENOPKG;
-
-	prep->payload.data[0] = (struct krb5_enctype *)krb5;
-
-	if (prep->datalen != krb5->key_len)
-		return -EKEYREJECTED;
-
-	server_key->len = prep->datalen;
-	server_key->data = kmemdup(prep->data, prep->datalen, GFP_KERNEL);
-	if (!server_key->data)
-		return -ENOMEM;
-
-	_leave(" = 0");
-	return 0;
-}
-
-static void rxgk_free_server_key(union key_payload *payload)
-{
-	struct krb5_buffer *server_key = (void *)&payload->data[2];
-
-	kfree_sensitive(server_key->data);
-}
-
-static void rxgk_free_preparse_server_key(struct key_preparsed_payload *prep)
-{
-	rxgk_free_server_key(&prep->payload);
-}
-
-static void rxgk_destroy_server_key(struct key *key)
-{
-	rxgk_free_server_key(&key->payload);
-}
-
-static void rxgk_describe_server_key(const struct key *key, struct seq_file *m)
-{
-	const struct krb5_enctype *krb5 = key->payload.data[0];
-
-	if (krb5)
-		seq_printf(m, ": %s", krb5->name);
-}
-
 /*
  * Handle rekeying the connection when we see our limits overrun or when the
  * far side decided to rekey.
@@ -1335,10 +1273,6 @@ const struct rxrpc_security rxgk_yfs = {
 	.no_key_abort			= RXGK_NOTAUTH,
 	.init				= rxgk_init,
 	.exit				= rxgk_exit,
-	.preparse_server_key		= rxgk_preparse_server_key,
-	.free_preparse_server_key	= rxgk_free_preparse_server_key,
-	.destroy_server_key		= rxgk_destroy_server_key,
-	.describe_server_key		= rxgk_describe_server_key,
 	.init_connection_security	= rxgk_init_connection_security,
 	.alloc_txbuf			= rxgk_alloc_txbuf,
 	.secure_packet			= rxgk_secure_packet,
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index ca9f0e82cb9a..2936c6effae3 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -47,50 +47,6 @@ struct rxkad_level2_hdr {
 static void rxkad_prime_packet_security(struct rxrpc_connection *conn,
 					const struct fcrypt_key *cipher);
 
-/*
- * Parse the information from a server key
- *
- * The data should be the 8-byte secret key.
- */
-static int rxkad_preparse_server_key(struct key_preparsed_payload *prep)
-{
-	struct des_ctx *des_key;
-	int err;
-
-	if (prep->datalen != 8)
-		return -EINVAL;
-
-	memcpy(&prep->payload.data[2], prep->data, 8);
-
-	des_key = kmalloc_obj(*des_key);
-	if (!des_key) {
-		_leave(" = -ENOMEM");
-		return -ENOMEM;
-	}
-
-	err = des_expand_key(des_key, prep->data, 8);
-	if (err) {
-		kfree_sensitive(des_key);
-		_leave(" = %d", err);
-		return err;
-	}
-
-	prep->payload.data[0] = des_key;
-	_leave(" = 0");
-	return 0;
-}
-
-static void rxkad_free_preparse_server_key(struct key_preparsed_payload *prep)
-{
-	kfree_sensitive(prep->payload.data[0]);
-}
-
-static void rxkad_destroy_server_key(struct key *key)
-{
-	kfree_sensitive(key->payload.data[0]);
-	key->payload.data[0] = NULL;
-}
-
 /*
  * initialise connection security
  */
@@ -1124,9 +1080,6 @@ const struct rxrpc_security rxkad = {
 	.no_key_abort			= RXKADUNKNOWNKEY,
 	.init				= rxkad_init,
 	.exit				= rxkad_exit,
-	.preparse_server_key		= rxkad_preparse_server_key,
-	.free_preparse_server_key	= rxkad_free_preparse_server_key,
-	.destroy_server_key		= rxkad_destroy_server_key,
 	.init_connection_security	= rxkad_init_connection_security,
 	.alloc_txbuf			= rxkad_alloc_txbuf,
 	.secure_packet			= rxkad_secure_packet,
diff --git a/net/rxrpc/security.c b/net/rxrpc/security.c
index 2bfbf2b2bb37..28883a70c9e8 100644
--- a/net/rxrpc/security.c
+++ b/net/rxrpc/security.c
@@ -10,6 +10,8 @@
 #include <linux/skbuff.h>
 #include <linux/udp.h>
 #include <linux/crypto.h>
+#include <linux/key-type.h>
+#include <linux/slab.h>
 #include <net/sock.h>
 #include <net/af_rxrpc.h>
 #include <keys/rxrpc-type.h>
@@ -208,3 +210,185 @@ struct key *rxrpc_look_up_server_security(struct rxrpc_connection *conn,
 	read_unlock(&conn->local->services_lock);
 	return key;
 }
+
+/*
+ * grab the security key for a socket
+ */
+int rxrpc_request_key(struct rxrpc_sock *rx, sockptr_t optval, int optlen)
+{
+	struct key *key;
+	char *description;
+
+	_enter("");
+
+	if (optlen <= 0 || optlen > PAGE_SIZE - 1 || rx->key)
+		return -EINVAL;
+
+	description = memdup_sockptr_nul(optval, optlen);
+	if (IS_ERR(description))
+		return PTR_ERR(description);
+
+	key = request_key_net(&key_type_rxrpc, description, sock_net(&rx->sk), NULL);
+	if (IS_ERR(key)) {
+		kfree(description);
+		_leave(" = %ld", PTR_ERR(key));
+		return PTR_ERR(key);
+	}
+
+	rx->key = key;
+	kfree(description);
+	_leave(" = 0 [key %x]", key->serial);
+	return 0;
+}
+
+/*
+ * generate a server data key
+ */
+int rxrpc_get_server_data_key(struct rxrpc_connection *conn,
+			      const void *session_key,
+			      time64_t expiry,
+			      u32 kvno)
+{
+	const struct cred *cred = current_cred();
+	struct {
+		u32 kver;
+		struct rxrpc_key_data_v1 v1;
+	} data;
+	struct key *key;
+	int ret;
+
+	_enter("");
+
+	key = key_alloc(&key_type_rxrpc, "x",
+			GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, cred, 0,
+			KEY_ALLOC_NOT_IN_QUOTA, NULL);
+	if (IS_ERR(key)) {
+		_leave(" = -ENOMEM [alloc %ld]", PTR_ERR(key));
+		return -ENOMEM;
+	}
+
+	_debug("key %d", key_serial(key));
+
+	data.kver = 1;
+	data.v1.security_index = RXRPC_SECURITY_RXKAD;
+	data.v1.ticket_length = 0;
+	data.v1.expiry = rxrpc_time64_to_u32(expiry);
+	data.v1.kvno = 0;
+
+	memcpy(&data.v1.session_key, session_key, sizeof(data.v1.session_key));
+
+	ret = key_instantiate_and_link(key, &data, sizeof(data), NULL, NULL);
+	if (ret < 0)
+		goto error;
+
+	conn->key = key;
+	_leave(" = 0 [%d]", key_serial(key));
+	return 0;
+
+error:
+	key_revoke(key);
+	key_put(key);
+	_leave(" = -ENOMEM [ins %d]", ret);
+	return -ENOMEM;
+}
+EXPORT_SYMBOL(rxrpc_get_server_data_key);
+
+/*
+ * grab the security keyring for a server socket
+ */
+int rxrpc_server_keyring(struct rxrpc_sock *rx, sockptr_t optval, int optlen)
+{
+	struct key *key;
+	char *description;
+
+	_enter("");
+
+	if (rx->securities)
+		return -EINVAL;
+
+	if (optlen <= 0 || optlen > PAGE_SIZE - 1)
+		return -EINVAL;
+
+	description = memdup_sockptr_nul(optval, optlen);
+	if (IS_ERR(description))
+		return PTR_ERR(description);
+
+	key = request_key(&key_type_keyring, description, NULL);
+	if (IS_ERR(key)) {
+		kfree(description);
+		_leave(" = %ld", PTR_ERR(key));
+		return PTR_ERR(key);
+	}
+
+	rx->securities = key;
+	kfree(description);
+	_leave(" = 0 [key %x]", key->serial);
+	return 0;
+}
+
+/**
+ * rxrpc_sock_set_security_keyring - Set the security keyring for a kernel service
+ * @sk: The socket to set the keyring on
+ * @keyring: The keyring to set
+ *
+ * Set the server security keyring on an rxrpc socket.  This is used to provide
+ * the encryption keys for a kernel service.
+ *
+ * Return: %0 if successful and a negative error code otherwise.
+ */
+int rxrpc_sock_set_security_keyring(struct sock *sk, struct key *keyring)
+{
+	struct rxrpc_sock *rx = rxrpc_sk(sk);
+	int ret = 0;
+
+	lock_sock(sk);
+	if (rx->securities)
+		ret = -EINVAL;
+	else if (rx->sk.sk_state != RXRPC_UNBOUND)
+		ret = -EISCONN;
+	else
+		rx->securities = key_get(keyring);
+	release_sock(sk);
+	return ret;
+}
+EXPORT_SYMBOL(rxrpc_sock_set_security_keyring);
+
+/**
+ * rxrpc_sock_set_manage_response - Set the manage-response flag for a kernel service
+ * @sk: The socket to set the keyring on
+ * @set: True to set, false to clear the flag
+ *
+ * Set the flag on an rxrpc socket to say that the caller wants to manage the
+ * RESPONSE packet and the user-defined data it may contain.  Setting this
+ * means that recvmsg() will return messages with RXRPC_CHALLENGED in the
+ * control message buffer containing information about the challenge.
+ *
+ * The user should respond to the challenge by passing RXRPC_RESPOND or
+ * RXRPC_RESPOND_ABORT control messages with sendmsg() to the same call.
+ * Supplementary control messages, such as RXRPC_RESP_RXGK_APPDATA, may be
+ * included to indicate the parts the user wants to supply.
+ *
+ * The server will be passed the response data with a RXRPC_RESPONDED control
+ * message when it gets the first data from each call.
+ *
+ * Note that this is only honoured by security classes that need auxiliary data
+ * (e.g. RxGK).  Those that don't offer the facility (e.g. RxKAD) respond
+ * without consulting userspace.
+ *
+ * Return: The previous setting.
+ */
+int rxrpc_sock_set_manage_response(struct sock *sk, bool set)
+{
+	struct rxrpc_sock *rx = rxrpc_sk(sk);
+	int ret;
+
+	lock_sock(sk);
+	ret = !!test_bit(RXRPC_SOCK_MANAGE_RESPONSE, &rx->flags);
+	if (set)
+		set_bit(RXRPC_SOCK_MANAGE_RESPONSE, &rx->flags);
+	else
+		clear_bit(RXRPC_SOCK_MANAGE_RESPONSE, &rx->flags);
+	release_sock(sk);
+	return ret;
+}
+EXPORT_SYMBOL(rxrpc_sock_set_manage_response);
diff --git a/net/rxrpc/server_key.c b/net/rxrpc/server_key.c
index 3efe104b1930..0be259dd0df0 100644
--- a/net/rxrpc/server_key.c
+++ b/net/rxrpc/server_key.c
@@ -9,6 +9,11 @@
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#define RXRPC_KEY_PROVIDER
+
+#include <crypto/des.h>
+#include <crypto/krb5.h>
+#include <linux/fips.h>
 
 #include <linux/module.h>
 #include <linux/net.h>
@@ -42,6 +47,7 @@ struct key_type key_type_rxrpc_s = {
 	.destroy	= rxrpc_destroy_s,
 	.describe	= rxrpc_describe_s,
 };
+EXPORT_SYMBOL(key_type_rxrpc_s);
 
 /*
  * Vet the description for an RxRPC server key.
@@ -60,156 +66,198 @@ static int rxrpc_vet_description_s(const char *desc)
 	return 0;
 }
 
+#ifdef CONFIG_RXKAD
 /*
- * Preparse a server secret key.
+ * Parse the information from a server key
+ *
+ * The data should be the 8-byte secret key.
  */
-static int rxrpc_preparse_s(struct key_preparsed_payload *prep)
+static int rxkad_preparse_server_key(struct key_preparsed_payload *prep)
 {
-	const struct rxrpc_security *sec;
-	unsigned int service, sec_class;
-	int n;
+	struct des_ctx *des_key;
+	int err;
+
+	if (fips_enabled)
+		return -ENOPKG;
+
+	if (prep->datalen != 8)
+		return -EINVAL;
+
+	memcpy(&prep->payload.data[2], prep->data, 8);
+
+	des_key = kmalloc_obj(*des_key);
+	if (!des_key) {
+		_leave(" = -ENOMEM");
+		return -ENOMEM;
+	}
+
+	err = des_expand_key(des_key, prep->data, 8);
+	if (err) {
+		kfree_sensitive(des_key);
+		_leave(" = %d", err);
+		return err;
+	}
+
+	prep->payload.data[0] = des_key;
+	_leave(" = 0");
+	return 0;
+}
+
+static void rxkad_free_preparse_server_key(struct key_preparsed_payload *prep)
+{
+	kfree_sensitive(prep->payload.data[0]);
+}
+
+static void rxkad_destroy_server_key(struct key *key)
+{
+	kfree_sensitive(key->payload.data[0]);
+	key->payload.data[0] = NULL;
+}
+#endif
+
+#ifdef CONFIG_RXGK
+/*
+ * Parse the information from a server key
+ */
+static int rxgk_preparse_server_key(struct key_preparsed_payload *prep)
+{
+	const struct krb5_enctype *krb5;
+	struct krb5_buffer *server_key = (void *)&prep->payload.data[2];
+	unsigned int service, sec_class, kvno, enctype;
+	int n = 0;
 
 	_enter("%zu", prep->datalen);
 
-	if (!prep->orig_description)
+	if (sscanf(prep->orig_description, "%u:%u:%u:%u%n",
+		   &service, &sec_class, &kvno, &enctype, &n) != 4)
 		return -EINVAL;
 
-	if (sscanf(prep->orig_description, "%u:%u%n", &service, &sec_class, &n) != 2)
+	if (prep->orig_description[n])
 		return -EINVAL;
 
-	sec = rxrpc_security_lookup(sec_class);
-	if (!sec)
+	krb5 = crypto_krb5_find_enctype(enctype);
+	if (!krb5)
 		return -ENOPKG;
 
-	prep->payload.data[1] = (struct rxrpc_security *)sec;
+	prep->payload.data[0] = (struct krb5_enctype *)krb5;
 
-	if (!sec->preparse_server_key)
-		return -EINVAL;
+	if (prep->datalen != krb5->key_len)
+		return -EKEYREJECTED;
+
+	server_key->len = prep->datalen;
+	server_key->data = kmemdup(prep->data, prep->datalen, GFP_KERNEL);
+	if (!server_key->data)
+		return -ENOMEM;
 
-	return sec->preparse_server_key(prep);
+	_leave(" = 0");
+	return 0;
 }
 
-static void rxrpc_free_preparse_s(struct key_preparsed_payload *prep)
+static void rxgk_free_server_key(union key_payload *payload)
 {
-	const struct rxrpc_security *sec = prep->payload.data[1];
+	struct krb5_buffer *server_key = (void *)&payload->data[2];
 
-	if (sec && sec->free_preparse_server_key)
-		sec->free_preparse_server_key(prep);
+	kfree_sensitive(server_key->data);
 }
 
-static void rxrpc_destroy_s(struct key *key)
+static void rxgk_free_preparse_server_key(struct key_preparsed_payload *prep)
 {
-	const struct rxrpc_security *sec = key->payload.data[1];
+	rxgk_free_server_key(&prep->payload);
+}
 
-	if (sec && sec->destroy_server_key)
-		sec->destroy_server_key(key);
+static void rxgk_destroy_server_key(struct key *key)
+{
+	rxgk_free_server_key(&key->payload);
 }
 
-static void rxrpc_describe_s(const struct key *key, struct seq_file *m)
+static void rxgk_describe_server_key(const struct key *key, struct seq_file *m)
 {
-	const struct rxrpc_security *sec = key->payload.data[1];
+	const struct krb5_enctype *krb5 = key->payload.data[0];
 
-	seq_puts(m, key->description);
-	if (sec && sec->describe_server_key)
-		sec->describe_server_key(key, m);
+	if (krb5)
+		seq_printf(m, ": %s", krb5->name);
 }
+#endif
 
 /*
- * grab the security keyring for a server socket
+ * Preparse a server secret key.
  */
-int rxrpc_server_keyring(struct rxrpc_sock *rx, sockptr_t optval, int optlen)
+static int rxrpc_preparse_s(struct key_preparsed_payload *prep)
 {
-	struct key *key;
-	char *description;
+	unsigned int service, sec_class;
+	int n;
 
-	_enter("");
+	_enter("%zu", prep->datalen);
 
-	if (rx->securities)
+	if (!prep->orig_description)
 		return -EINVAL;
 
-	if (optlen <= 0 || optlen > PAGE_SIZE - 1)
+	if (sscanf(prep->orig_description, "%u:%u%n", &service, &sec_class, &n) != 2)
 		return -EINVAL;
 
-	description = memdup_sockptr_nul(optval, optlen);
-	if (IS_ERR(description))
-		return PTR_ERR(description);
+	prep->payload.data[1] = (void *)(uintptr_t)sec_class;
 
-	key = request_key(&key_type_keyring, description, NULL);
-	if (IS_ERR(key)) {
-		kfree(description);
-		_leave(" = %ld", PTR_ERR(key));
-		return PTR_ERR(key);
+	switch (sec_class) {
+#ifdef CONFIG_RXKAD
+	case RXRPC_SECURITY_RXKAD:
+		return rxkad_preparse_server_key(prep);
+#endif
+#ifdef CONFIG_RXGK
+	case RXRPC_SECURITY_YFS_RXGK:
+		return rxgk_preparse_server_key(prep);
+#endif
+	default:
+		return -ENOPKG;
 	}
+}
 
-	rx->securities = key;
-	kfree(description);
-	_leave(" = 0 [key %x]", key->serial);
-	return 0;
+static void rxrpc_free_preparse_s(struct key_preparsed_payload *prep)
+{
+	switch ((uintptr_t)prep->payload.data[1]) {
+#ifdef CONFIG_RXKAD
+	case RXRPC_SECURITY_RXKAD:
+		rxkad_free_preparse_server_key(prep);
+		break;
+#endif
+#ifdef CONFIG_RXGK
+	case RXRPC_SECURITY_YFS_RXGK:
+		rxgk_free_preparse_server_key(prep);
+		break;
+#endif
+	default:
+		break;
+	}
 }
 
-/**
- * rxrpc_sock_set_security_keyring - Set the security keyring for a kernel service
- * @sk: The socket to set the keyring on
- * @keyring: The keyring to set
- *
- * Set the server security keyring on an rxrpc socket.  This is used to provide
- * the encryption keys for a kernel service.
- *
- * Return: %0 if successful and a negative error code otherwise.
- */
-int rxrpc_sock_set_security_keyring(struct sock *sk, struct key *keyring)
+static void rxrpc_destroy_s(struct key *key)
 {
-	struct rxrpc_sock *rx = rxrpc_sk(sk);
-	int ret = 0;
-
-	lock_sock(sk);
-	if (rx->securities)
-		ret = -EINVAL;
-	else if (rx->sk.sk_state != RXRPC_UNBOUND)
-		ret = -EISCONN;
-	else
-		rx->securities = key_get(keyring);
-	release_sock(sk);
-	return ret;
+	switch ((uintptr_t)key->payload.data[1]) {
+#ifdef CONFIG_RXKAD
+	case RXRPC_SECURITY_RXKAD:
+		rxkad_destroy_server_key(key);
+		break;
+#endif
+#ifdef CONFIG_RXGK
+	case RXRPC_SECURITY_YFS_RXGK:
+		rxgk_destroy_server_key(key);
+		break;
+#endif
+	default:
+		break;
+	}
 }
-EXPORT_SYMBOL(rxrpc_sock_set_security_keyring);
 
-/**
- * rxrpc_sock_set_manage_response - Set the manage-response flag for a kernel service
- * @sk: The socket to set the keyring on
- * @set: True to set, false to clear the flag
- *
- * Set the flag on an rxrpc socket to say that the caller wants to manage the
- * RESPONSE packet and the user-defined data it may contain.  Setting this
- * means that recvmsg() will return messages with RXRPC_CHALLENGED in the
- * control message buffer containing information about the challenge.
- *
- * The user should respond to the challenge by passing RXRPC_RESPOND or
- * RXRPC_RESPOND_ABORT control messages with sendmsg() to the same call.
- * Supplementary control messages, such as RXRPC_RESP_RXGK_APPDATA, may be
- * included to indicate the parts the user wants to supply.
- *
- * The server will be passed the response data with a RXRPC_RESPONDED control
- * message when it gets the first data from each call.
- *
- * Note that this is only honoured by security classes that need auxiliary data
- * (e.g. RxGK).  Those that don't offer the facility (e.g. RxKAD) respond
- * without consulting userspace.
- *
- * Return: The previous setting.
- */
-int rxrpc_sock_set_manage_response(struct sock *sk, bool set)
+static void rxrpc_describe_s(const struct key *key, struct seq_file *m)
 {
-	struct rxrpc_sock *rx = rxrpc_sk(sk);
-	int ret;
-
-	lock_sock(sk);
-	ret = !!test_bit(RXRPC_SOCK_MANAGE_RESPONSE, &rx->flags);
-	if (set)
-		set_bit(RXRPC_SOCK_MANAGE_RESPONSE, &rx->flags);
-	else
-		clear_bit(RXRPC_SOCK_MANAGE_RESPONSE, &rx->flags);
-	release_sock(sk);
-	return ret;
+	seq_puts(m, key->description);
+
+	switch ((uintptr_t)key->payload.data[1]) {
+#ifdef CONFIG_RXGK
+	case RXRPC_SECURITY_YFS_RXGK:
+		rxgk_describe_server_key(key, m);
+		break;
+#endif
+	default:
+		break;
+	}
 }
-EXPORT_SYMBOL(rxrpc_sock_set_manage_response);
-- 
2.47.3

[RFC PATCH v3 09/11] KEYS: Pin the backing kernel module for a key type

[Jarkko Sakkinen]

Add the owner field (populated with THIS_MODULE) to struct key_type.

Pin the module in key_alloc() and keyring_restrict(), and release it in
key_gc_unused_keys() and keyring_destroy(), which are wires to the keys
and restrictions.

Reject module owners still in MODULE_STATE_COMING so that the failed
module_init() paths cannot leave behind referenced keys.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 include/linux/key-type.h |  6 ++++-
 security/keys/gc.c       | 14 ++++++++++--
 security/keys/key.c      | 48 ++++++++++++++++++++++++++++++----------
 security/keys/keyring.c  | 36 +++++++++++++++++++++++++-----
 4 files changed, 83 insertions(+), 21 deletions(-)

diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index c4f329b6ae3f..9ecec31ad571 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -10,9 +10,11 @@
 
 #include <linux/key.h>
 #include <linux/errno.h>
+#include <linux/init.h>
 
 #ifdef CONFIG_KEYS
 
+struct module;
 struct kernel_pkey_query;
 struct kernel_pkey_params;
 
@@ -161,6 +163,7 @@ struct key_type {
 				     const void *in, const void *in2);
 
 	/* internal fields */
+	struct module		*owner;
 	atomic64_t		index_id;	/* Opaque keyring index ID */
 	struct list_head	link;		/* link in types list */
 	struct lock_class_key	lock_class;	/* key->sem lock class */
@@ -168,7 +171,8 @@ struct key_type {
 
 extern struct key_type key_type_keyring;
 
-extern int register_key_type(struct key_type *ktype);
+int __register_key_type(struct key_type *ktype, struct module *owner);
+#define register_key_type(ktype) __register_key_type(ktype, THIS_MODULE)
 extern void unregister_key_type(struct key_type *ktype);
 
 extern int key_payload_reserve(struct key *key, size_t datalen);
diff --git a/security/keys/gc.c b/security/keys/gc.c
index 2c3eddc0443a..94f01fb5050f 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/module.h>
 #include <linux/security.h>
 #include <keys/keyring-type.h>
 #include "internal.h"
@@ -148,6 +149,7 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
 	while (!list_empty(keys)) {
 		struct key *key =
 			list_entry(keys->next, struct key, graveyard_link);
+		struct module *owner = key->type->owner;
 		short state = key->state;
 
 		list_del(&key->graveyard_link);
@@ -161,8 +163,15 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
 #endif
 
 		/* Throw away the key data if the key is instantiated */
-		if (state == KEY_IS_POSITIVE && key->type->destroy)
-			key->type->destroy(key);
+		if (state == KEY_IS_POSITIVE) {
+			if (key->type->destroy)
+				key->type->destroy(key);
+		} else if (key->restrict_link) {
+			if (key->restrict_link->keytype)
+				module_put(key->restrict_link->keytype->owner);
+			key_put(key->restrict_link->key);
+			kfree(key->restrict_link);
+		}
 
 		security_key_free(key);
 
@@ -176,6 +185,7 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
 
 		memzero_explicit(key, sizeof(*key));
 		kmem_cache_free(key_jar, key);
+		module_put(owner);
 	}
 }
 
diff --git a/security/keys/key.c b/security/keys/key.c
index 3f1f0fc13710..0b2a03c511e5 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -7,6 +7,7 @@
 
 #include <linux/export.h>
 #include <linux/init.h>
+#include <linux/module.h>
 #include <linux/poison.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
@@ -234,19 +235,34 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 
 	key = ERR_PTR(-EINVAL);
 	if (!desc || !*desc)
-		goto error;
+		goto out;
+
+	/* Failed module initialization ignores outstanding module references. */
+	if ((type->owner && module_is_coming(type->owner)) ||
+	    !try_module_get(type->owner)) {
+		key = ERR_PTR(-ENOENT);
+		goto out;
+	}
+
+	if (restrict_link && restrict_link->keytype &&
+	    ((restrict_link->keytype->owner &&
+	      module_is_coming(restrict_link->keytype->owner)) ||
+	     !try_module_get(restrict_link->keytype->owner))) {
+		key = ERR_PTR(-ENOENT);
+		goto out_put_module;
+	}
 
 	if (type->vet_description) {
 		ret = type->vet_description(desc);
 		if (ret < 0) {
 			key = ERR_PTR(ret);
-			goto error;
+			goto out_put_restrict_module;
 		}
 	}
 
 	desclen = strlen(desc);
 	if (desclen >= KEY_MAX_DESC_SIZE)
-		goto error;
+		goto out_put_restrict_module;
 	quotalen = desclen + 1 + type->def_datalen;
 
 	/* get hold of the key tracking for this user */
@@ -323,8 +339,6 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 	refcount_inc(&key->domain_tag->usage);
 	atomic_inc(&user->nkeys);
 	key_alloc_serial(key);
-
-error:
 	return key;
 
 security_error:
@@ -338,7 +352,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 	}
 	key_user_put(user);
 	key = ERR_PTR(ret);
-	goto error;
+	goto out_put_restrict_module;
 
 no_memory_3:
 	kmem_cache_free(key_jar, key);
@@ -352,13 +366,22 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 	key_user_put(user);
 no_memory_1:
 	key = ERR_PTR(-ENOMEM);
-	goto error;
+	goto out_put_restrict_module;
 
 no_quota:
 	spin_unlock_irqrestore(&user->lock, irqflags);
 	key_user_put(user);
 	key = ERR_PTR(-EDQUOT);
-	goto error;
+
+out_put_restrict_module:
+	if (restrict_link && restrict_link->keytype)
+		module_put(restrict_link->keytype->owner);
+
+out_put_module:
+	module_put(type->owner);
+
+out:
+	return key;
 }
 EXPORT_SYMBOL(key_alloc);
 
@@ -1231,19 +1254,20 @@ int generic_key_instantiate(struct key *key, struct key_preparsed_payload *prep)
 EXPORT_SYMBOL(generic_key_instantiate);
 
 /**
- * register_key_type - Register a type of key.
+ * __register_key_type - Register a type of key.
  * @ktype: The new key type.
+ * @owner: Module owning the key type.
  *
  * Register a new key type.
  *
  * Returns 0 on success or -EEXIST if a type of this name already exists.
  */
-int register_key_type(struct key_type *ktype)
+int __register_key_type(struct key_type *ktype, struct module *owner)
 {
 	struct key_type *p;
 	int ret;
 
-	memset(&ktype->lock_class, 0, sizeof(ktype->lock_class));
+	ktype->owner = owner;
 
 	ret = -EEXIST;
 	down_write(&key_types_sem);
@@ -1264,7 +1288,7 @@ int register_key_type(struct key_type *ktype)
 	up_write(&key_types_sem);
 	return ret;
 }
-EXPORT_SYMBOL(register_key_type);
+EXPORT_SYMBOL(__register_key_type);
 
 /**
  * unregister_key_type - Unregister a type of key.
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 5281da9d5bf7..f90ab46ecfab 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -7,6 +7,7 @@
 
 #include <linux/export.h>
 #include <linux/init.h>
+#include <linux/module.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/security.h>
@@ -436,6 +437,9 @@ static void keyring_destroy(struct key *keyring)
 	if (keyring->restrict_link) {
 		struct key_restriction *keyres = keyring->restrict_link;
 
+		if (keyres->keytype)
+			module_put(keyres->keytype->owner);
+
 		key_put(keyres->key);
 		kfree(keyres);
 	}
@@ -540,6 +544,9 @@ struct key *keyring_alloc(const char *description, kuid_t uid, kgid_t gid,
 	if (!IS_ERR(keyring)) {
 		ret = key_instantiate_and_link(keyring, NULL, 0, dest, NULL);
 		if (ret < 0) {
+			if (restrict_link && restrict_link->keytype)
+				module_put(restrict_link->keytype->owner);
+			keyring->restrict_link = NULL;
 			key_put(keyring);
 			keyring = ERR_PTR(ret);
 		}
@@ -1062,7 +1069,7 @@ int keyring_restrict(key_ref_t keyring_ref, const char *type,
 
 		if (!restrict_type->lookup_restriction) {
 			ret = -ENOENT;
-			goto error;
+			goto out;
 		}
 
 		restrict_link = restrict_type->lookup_restriction(restriction);
@@ -1070,7 +1077,15 @@ int keyring_restrict(key_ref_t keyring_ref, const char *type,
 
 	if (IS_ERR(restrict_link)) {
 		ret = PTR_ERR(restrict_link);
-		goto error;
+		goto out;
+	}
+
+	if (restrict_link && restrict_link->keytype &&
+	    ((restrict_link->keytype->owner &&
+	      module_is_coming(restrict_link->keytype->owner)) ||
+	     !try_module_get(restrict_link->keytype->owner))) {
+		ret = -ENOENT;
+		goto out_free_restriction;
 	}
 
 	down_write(&keyring->sem);
@@ -1088,12 +1103,21 @@ int keyring_restrict(key_ref_t keyring_ref, const char *type,
 	up_write(&keyring_serialise_restrict_sem);
 	up_write(&keyring->sem);
 
-	if (ret < 0) {
+	if (ret < 0)
+		goto out_put_module;
+
+	goto out;
+
+out_put_module:
+	if (restrict_link && restrict_link->keytype)
+		module_put(restrict_link->keytype->owner);
+
+out_free_restriction:
+	if (restrict_link)
 		key_put(restrict_link->key);
-		kfree(restrict_link);
-	}
+	kfree(restrict_link);
 
-error:
+out:
 	if (restrict_type)
 		key_type_put(restrict_type);
 
-- 
2.47.3

[RFC PATCH v3 10/11] KEYS: Remove key_type_dead

[Jarkko Sakkinen]

unregister_key_type() no longer needs to retype live keys as keys and
restrictions pin the module while they reference it.

Remove key_type_dead and associated code from the keyring GC.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 security/keys/gc.c       | 136 ++-------------------------------------
 security/keys/internal.h |   4 --
 security/keys/key.c      |   9 +--
 security/keys/keyring.c  |  50 --------------
 4 files changed, 9 insertions(+), 190 deletions(-)

diff --git a/security/keys/gc.c b/security/keys/gc.c
index 94f01fb5050f..8e2c83ef29bc 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -40,22 +40,8 @@ static void key_gc_timer_func(struct timer_list *);
 static DEFINE_TIMER(key_gc_timer, key_gc_timer_func);
 
 static time64_t key_gc_next_run = TIME64_MAX;
-static struct key_type *key_gc_dead_keytype;
-
 static unsigned long key_gc_flags;
 #define KEY_GC_KEY_EXPIRED	0	/* A key expired and needs unlinking */
-#define KEY_GC_REAP_KEYTYPE	1	/* A keytype is being unregistered */
-#define KEY_GC_REAPING_KEYTYPE	2	/* Cleared when keytype reaped */
-
-
-/*
- * Any key whose type gets unregistered will be re-typed to this if it can't be
- * immediately unlinked.
- */
-struct key_type key_type_dead = {
-	.name = ".dead",
-};
-
 /*
  * Schedule a garbage collection run.
  * - time precision isn't particularly important
@@ -67,7 +53,7 @@ void key_schedule_gc(time64_t gc_at)
 
 	kenter("%lld", gc_at - now);
 
-	if (gc_at <= now || test_bit(KEY_GC_REAP_KEYTYPE, &key_gc_flags)) {
+	if (gc_at <= now) {
 		kdebug("IMMEDIATE");
 		schedule_work(&key_gc_work);
 	} else if (gc_at < key_gc_next_run) {
@@ -111,36 +97,6 @@ static void key_gc_timer_func(struct timer_list *unused)
 	key_schedule_gc_links();
 }
 
-/*
- * Reap keys of dead type.
- *
- * We use three flags to make sure we see three complete cycles of the garbage
- * collector: the first to mark keys of that type as being dead, the second to
- * collect dead links and the third to clean up the dead keys.  We have to be
- * careful as there may already be a cycle in progress.
- *
- * The caller must be holding key_types_sem.
- */
-void key_gc_keytype(struct key_type *ktype)
-{
-	kenter("%s", ktype->name);
-
-	key_gc_dead_keytype = ktype;
-	set_bit(KEY_GC_REAPING_KEYTYPE, &key_gc_flags);
-	smp_mb();
-	set_bit(KEY_GC_REAP_KEYTYPE, &key_gc_flags);
-
-	kdebug("schedule");
-	schedule_work(&key_gc_work);
-
-	kdebug("sleep");
-	wait_on_bit(&key_gc_flags, KEY_GC_REAPING_KEYTYPE,
-		    TASK_UNINTERRUPTIBLE);
-
-	key_gc_dead_keytype = NULL;
-	kleave("");
-}
-
 /*
  * Garbage collect a list of unreferenced, detached keys
  */
@@ -199,30 +155,20 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
 static void key_garbage_collector(struct work_struct *work)
 {
 	static LIST_HEAD(graveyard);
-	static u8 gc_state;		/* Internal persistent state */
+	u8 gc_state = 0;
 #define KEY_GC_REAP_AGAIN	0x01	/* - Need another cycle */
 #define KEY_GC_REAPING_LINKS	0x02	/* - We need to reap links */
-#define KEY_GC_REAPING_DEAD_1	0x10	/* - We need to mark dead keys */
-#define KEY_GC_REAPING_DEAD_2	0x20	/* - We need to reap dead key links */
-#define KEY_GC_REAPING_DEAD_3	0x40	/* - We need to reap dead keys */
-#define KEY_GC_FOUND_DEAD_KEY	0x80	/* - We found at least one dead key */
 
 	struct rb_node *cursor;
 	struct key *key;
 	time64_t new_timer, limit, expiry;
 
-	kenter("[%lx,%x]", key_gc_flags, gc_state);
+	kenter("[%lx]", key_gc_flags);
 
 	limit = ktime_get_real_seconds();
 
-	/* Work out what we're going to be doing in this pass */
-	gc_state &= KEY_GC_REAPING_DEAD_1 | KEY_GC_REAPING_DEAD_2;
-	gc_state <<= 1;
 	if (test_and_clear_bit(KEY_GC_KEY_EXPIRED, &key_gc_flags))
 		gc_state |= KEY_GC_REAPING_LINKS;
-
-	if (test_and_clear_bit(KEY_GC_REAP_KEYTYPE, &key_gc_flags))
-		gc_state |= KEY_GC_REAPING_DEAD_1;
 	kdebug("new pass %x", gc_state);
 
 	new_timer = TIME64_MAX;
@@ -244,18 +190,6 @@ static void key_garbage_collector(struct work_struct *work)
 			goto found_unreferenced_key;
 		}
 
-		if (unlikely(gc_state & KEY_GC_REAPING_DEAD_1)) {
-			if (key->type == key_gc_dead_keytype) {
-				gc_state |= KEY_GC_FOUND_DEAD_KEY;
-				set_bit(KEY_FLAG_DEAD, &key->flags);
-				key->perm = 0;
-				goto skip_dead_key;
-			} else if (key->type == &key_type_keyring &&
-				   key->restrict_link) {
-				goto found_restricted_keyring;
-			}
-		}
-
 		expiry = key->expiry;
 		if (expiry != TIME64_MAX) {
 			if (!(key->type->flags & KEY_TYPE_INSTANT_REAP))
@@ -267,21 +201,10 @@ static void key_garbage_collector(struct work_struct *work)
 			}
 		}
 
-		if (unlikely(gc_state & KEY_GC_REAPING_DEAD_2))
-			if (key->type == key_gc_dead_keytype)
-				gc_state |= KEY_GC_FOUND_DEAD_KEY;
-
-		if ((gc_state & KEY_GC_REAPING_LINKS) ||
-		    unlikely(gc_state & KEY_GC_REAPING_DEAD_2)) {
-			if (key->type == &key_type_keyring)
-				goto found_keyring;
-		}
-
-		if (unlikely(gc_state & KEY_GC_REAPING_DEAD_3))
-			if (key->type == key_gc_dead_keytype)
-				goto destroy_dead_key;
+		if ((gc_state & KEY_GC_REAPING_LINKS) &&
+		    key->type == &key_type_keyring)
+			goto found_keyring;
 
-	skip_dead_key:
 		if (spin_is_contended(&key_serial_lock) || need_resched())
 			goto contended;
 	}
@@ -307,8 +230,7 @@ static void key_garbage_collector(struct work_struct *work)
 		key_schedule_gc(new_timer);
 	}
 
-	if (unlikely(gc_state & KEY_GC_REAPING_DEAD_2) ||
-	    !list_empty(&graveyard)) {
+	if (!list_empty(&graveyard)) {
 		/* Make sure that all pending keyring payload destructions are
 		 * fulfilled and that people aren't now looking at dead or
 		 * dying keys that they don't have a reference upon or a link
@@ -316,32 +238,10 @@ static void key_garbage_collector(struct work_struct *work)
 		 */
 		kdebug("gc sync");
 		synchronize_rcu();
-	}
-
-	if (!list_empty(&graveyard)) {
 		kdebug("gc keys");
 		key_gc_unused_keys(&graveyard);
 	}
 
-	if (unlikely(gc_state & (KEY_GC_REAPING_DEAD_1 |
-				 KEY_GC_REAPING_DEAD_2))) {
-		if (!(gc_state & KEY_GC_FOUND_DEAD_KEY)) {
-			/* No remaining dead keys: short circuit the remaining
-			 * keytype reap cycles.
-			 */
-			kdebug("dead short");
-			gc_state &= ~(KEY_GC_REAPING_DEAD_1 | KEY_GC_REAPING_DEAD_2);
-			gc_state |= KEY_GC_REAPING_DEAD_3;
-		} else {
-			gc_state |= KEY_GC_REAP_AGAIN;
-		}
-	}
-
-	if (unlikely(gc_state & KEY_GC_REAPING_DEAD_3)) {
-		kdebug("dead wake");
-		clear_and_wake_up_bit(KEY_GC_REAPING_KEYTYPE, &key_gc_flags);
-	}
-
 	if (gc_state & KEY_GC_REAP_AGAIN)
 		schedule_work(&key_gc_work);
 	kleave(" [end %x]", gc_state);
@@ -359,14 +259,6 @@ static void key_garbage_collector(struct work_struct *work)
 	gc_state |= KEY_GC_REAP_AGAIN;
 	goto maybe_resched;
 
-	/* We found a restricted keyring and need to update the restriction if
-	 * it is associated with the dead key type.
-	 */
-found_restricted_keyring:
-	spin_unlock(&key_serial_lock);
-	keyring_restriction_gc(key, key_gc_dead_keytype);
-	goto maybe_resched;
-
 	/* We found a keyring and we need to check the payload for links to
 	 * dead or expired keys.  We don't flag another reap immediately as we
 	 * have to wait for the old payload to be destroyed by RCU before we
@@ -376,18 +268,4 @@ static void key_garbage_collector(struct work_struct *work)
 	spin_unlock(&key_serial_lock);
 	keyring_gc(key, limit);
 	goto maybe_resched;
-
-	/* We found a dead key that is still referenced.  Reset its type and
-	 * destroy its payload with its semaphore held.
-	 */
-destroy_dead_key:
-	spin_unlock(&key_serial_lock);
-	kdebug("destroy key %d", key->serial);
-	down_write(&key->sem);
-	key->type = &key_type_dead;
-	if (key_gc_dead_keytype->destroy)
-		key_gc_dead_keytype->destroy(key);
-	memset(&key->payload, KEY_DESTROY, sizeof(key->payload));
-	up_write(&key->sem);
-	goto maybe_resched;
 }
diff --git a/security/keys/internal.h b/security/keys/internal.h
index b7b622bc36a1..005d609ae2df 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -38,7 +38,6 @@ struct iovec;
 	no_printk(KERN_DEBUG FMT"\n", ##__VA_ARGS__)
 #endif
 
-extern struct key_type key_type_dead;
 extern struct key_type key_type_user;
 extern struct key_type key_type_logon;
 
@@ -165,12 +164,9 @@ extern void key_change_session_keyring(struct callback_head *twork);
 extern struct work_struct key_gc_work;
 extern unsigned key_gc_delay;
 extern void keyring_gc(struct key *keyring, time64_t limit);
-extern void keyring_restriction_gc(struct key *keyring,
-				   struct key_type *dead_type);
 void key_set_expiry(struct key *key, time64_t expiry);
 extern void key_schedule_gc(time64_t gc_at);
 extern void key_schedule_gc_links(void);
-extern void key_gc_keytype(struct key_type *ktype);
 
 extern int key_task_permission(const key_ref_t key_ref,
 			       const struct cred *cred,
diff --git a/security/keys/key.c b/security/keys/key.c
index 0b2a03c511e5..df6af8066212 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -1294,18 +1294,14 @@ EXPORT_SYMBOL(__register_key_type);
  * unregister_key_type - Unregister a type of key.
  * @ktype: The key type.
  *
- * Unregister a key type and mark all the extant keys of this type as dead.
- * Those keys of this type are then destroyed to get rid of their payloads and
- * they and their links will be garbage collected as soon as possible.
+ * Unregister a key type.
  */
 void unregister_key_type(struct key_type *ktype)
 {
 	down_write(&key_types_sem);
 	list_del_init(&ktype->link);
-	downgrade_write(&key_types_sem);
-	key_gc_keytype(ktype);
 	pr_notice("Key type %s unregistered\n", ktype->name);
-	up_read(&key_types_sem);
+	up_write(&key_types_sem);
 }
 EXPORT_SYMBOL(unregister_key_type);
 
@@ -1320,7 +1316,6 @@ void __init key_init(void)
 
 	/* add the special key types */
 	list_add_tail(&key_type_keyring.link, &key_types_list);
-	list_add_tail(&key_type_dead.link, &key_types_list);
 	list_add_tail(&key_type_user.link, &key_types_list);
 	list_add_tail(&key_type_logon.link, &key_types_list);
 
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index f90ab46ecfab..b51abf698aa9 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -1793,53 +1793,3 @@ void keyring_gc(struct key *keyring, time64_t limit)
 	up_write(&keyring->sem);
 	kleave(" [gc]");
 }
-
-/*
- * Garbage collect restriction pointers from a keyring.
- *
- * Keyring restrictions are associated with a key type, and must be cleaned
- * up if the key type is unregistered. The restriction is altered to always
- * reject additional keys so a keyring cannot be opened up by unregistering
- * a key type.
- *
- * Not called with any keyring locks held. The keyring's key struct will not
- * be deallocated under us as only our caller may deallocate it.
- *
- * The caller is required to hold key_types_sem and dead_type->sem. This is
- * fulfilled by key_gc_keytype() holding the locks on behalf of
- * key_garbage_collector(), which it invokes on a workqueue.
- */
-void keyring_restriction_gc(struct key *keyring, struct key_type *dead_type)
-{
-	struct key_restriction *keyres;
-
-	kenter("%x{%s}", keyring->serial, keyring->description ?: "");
-
-	/*
-	 * keyring->restrict_link is only assigned at key allocation time
-	 * or with the key type locked, so the only values that could be
-	 * concurrently assigned to keyring->restrict_link are for key
-	 * types other than dead_type. Given this, it's ok to check
-	 * the key type before acquiring keyring->sem.
-	 */
-	if (!dead_type || !keyring->restrict_link ||
-	    keyring->restrict_link->keytype != dead_type) {
-		kleave(" [no restriction gc]");
-		return;
-	}
-
-	/* Lock the keyring to ensure that a link is not in progress */
-	down_write(&keyring->sem);
-
-	keyres = keyring->restrict_link;
-
-	keyres->check = restrict_link_reject;
-
-	key_put(keyres->key);
-	keyres->key = NULL;
-	keyres->keytype = NULL;
-
-	up_write(&keyring->sem);
-
-	kleave(" [restriction gc]");
-}
-- 
2.47.3

[RFC PATCH v3 11/11] KEYS: key_garbage_collector(): process in chunks

[Jarkko Sakkinen]

key_garbage_collector() has a design that is not resistant enough for
workqueue lockups. The duration scales up relative to the number of items
in the serial tree, which causes a reoccuring theme of being guilty of
causing workqueue lockups.

Resolve the problem by processing in chunks:

1. Declare a static variable key_gc_work, which owns the garbage collection
   in-flight state, and key_schedule_gc_work() for re-scheduling the work.
2. Define KEY_GC_CHUNK_SIZE, set to 64 keys, as the bound for the number
   of items scanned and destroyed.
3. Define KEY_GC_DESTROYING to mark resume to destruction without repeating
   the RCU grace period.
4. Define KEY_GC_REAP_REQUESTED for preserving full-pass requests across
   the continuations.

keygcbench results without the patch:

unused:   min=413.359 avg=1817.240 max=3221.120 var=1970880.024
expired:  min=0.002 avg=1360.124 max=2481.990 var=1055093.994
combined: min=0.002 avg=1542.970 max=3221.120 var=1471557.422

keygcbench results with the patch:

unused:   min=0.005 avg=0.126 max=54.173 var=0.946
expired:  min=0.002 avg=0.210 max=216.470 var=7.894
combined: min=0.002 avg=0.158 max=216.470 var=3.627

These results translate to:

- >99.9% drop in the combined variance.
- 3.0% increase in the total task time.

The value 64 for KEY_GC_CHUNK_SIZE was cherry-picked from the set {1, 2,
..., 1024} by running the benchmark for each chunk size three times.

At the far left variance is 97% lower but at the same time total time
grows by 133%, when compared to the selected chunk size. The size was
picked based on getting a reasonable variance with a minimal throughput
decline.

Fixes: 0c061b5707ab ("KEYS: Correctly destroy key payloads when their keytype is removed")
Link: https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd-test.git/tree/package/keygcbench
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 security/keys/gc.c       | 166 ++++++++++++++++++++++++++-------------
 security/keys/internal.h |   3 +-
 security/keys/key.c      |   4 +-
 security/keys/keyring.c  |   2 +-
 4 files changed, 117 insertions(+), 58 deletions(-)

diff --git a/security/keys/gc.c b/security/keys/gc.c
index 8e2c83ef29bc..9fbd1d94274e 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -11,27 +11,28 @@
 #include <keys/keyring-type.h>
 #include "internal.h"
 
-/*
- * Delay between key revocation/expiry in seconds
- */
-unsigned key_gc_delay = 5 * 60;
+#define KEY_GC_CHUNK_SIZE 64
 
 /*
  * Reaper for unused keys.
  */
 static void key_garbage_collector(struct work_struct *work);
-DECLARE_WORK(key_gc_work, key_garbage_collector);
 
-/**
- * key_gc_flush - Drain the garbage collector RCU callbacks
+static struct {
+	time64_t start;			/* expiry range start */
+	time64_t end;			/* expiry range end */
+	struct rb_node *cursor;		/* serial tree position */
+	u8 state;			/* state flags */
+	atomic_t seq;
+	struct work_struct work;	/* work item */
+} key_gc_work = {
+	.work = __WORK_INITIALIZER(key_gc_work.work, key_garbage_collector),
+};
+
+/*
+ * Delay between key revocation/expiry in seconds
  */
-void key_gc_flush(void)
-{
-	do {
-		rcu_barrier();
-	} while (flush_work(&key_gc_work));
-}
-EXPORT_SYMBOL_GPL(key_gc_flush);
+unsigned int key_gc_delay = 5 * 60;
 
 /*
  * Reaper for links from keyrings to dead keys.
@@ -42,6 +43,42 @@ static DEFINE_TIMER(key_gc_timer, key_gc_timer_func);
 static time64_t key_gc_next_run = TIME64_MAX;
 static unsigned long key_gc_flags;
 #define KEY_GC_KEY_EXPIRED	0	/* A key expired and needs unlinking */
+#define KEY_GC_REAP_REQUESTED	1	/* Another complete pass was requested */
+
+/*
+ * Schedule a new GC pass or re-schedule an ongoing GC pass.
+ */
+void key_schedule_gc_work(bool expired)
+{
+	unsigned long flags = BIT(KEY_GC_REAP_REQUESTED);
+
+	if (expired)
+		flags |= BIT(KEY_GC_KEY_EXPIRED);
+
+	/* Publish all request flags together, after the caller's updates. */
+	set_mask_bits(&key_gc_flags, 0, flags);
+	schedule_work(&key_gc_work.work);
+}
+
+/**
+ * key_gc_flush - Drain the garbage collector RCU callbacks
+ */
+void key_gc_flush(void)
+{
+	int seq;
+
+	do {
+		while (flush_work(&key_gc_work.work) ||
+		       work_pending(&key_gc_work.work))
+			cpu_relax();
+		seq = atomic_read_acquire(&key_gc_work.seq);
+		rcu_barrier();
+	} while (flush_work(&key_gc_work.work) ||
+		 work_pending(&key_gc_work.work) ||
+		 atomic_read_acquire(&key_gc_work.seq) != seq);
+}
+EXPORT_SYMBOL_GPL(key_gc_flush);
+
 /*
  * Schedule a garbage collection run.
  * - time precision isn't particularly important
@@ -55,7 +92,8 @@ void key_schedule_gc(time64_t gc_at)
 
 	if (gc_at <= now) {
 		kdebug("IMMEDIATE");
-		schedule_work(&key_gc_work);
+		/* Catch expirations that became due during the scan. */
+		key_schedule_gc_work(true);
 	} else if (gc_at < key_gc_next_run) {
 		kdebug("DEFERRED");
 		key_gc_next_run = gc_at;
@@ -77,15 +115,6 @@ void key_set_expiry(struct key *key, time64_t expiry)
 	}
 }
 
-/*
- * Schedule a dead links collection run.
- */
-void key_schedule_gc_links(void)
-{
-	set_bit(KEY_GC_KEY_EXPIRED, &key_gc_flags);
-	schedule_work(&key_gc_work);
-}
-
 /*
  * Some key's cleanup time was met after it expired, so we need to get the
  * reaper to go through a cycle finding expired keys.
@@ -94,7 +123,7 @@ static void key_gc_timer_func(struct timer_list *unused)
 {
 	kenter("");
 	key_gc_next_run = TIME64_MAX;
-	key_schedule_gc_links();
+	key_schedule_gc_work(true);
 }
 
 /*
@@ -102,7 +131,9 @@ static void key_gc_timer_func(struct timer_list *unused)
  */
 static noinline void key_gc_unused_keys(struct list_head *keys)
 {
-	while (!list_empty(keys)) {
+	unsigned int chunk = KEY_GC_CHUNK_SIZE;
+
+	while (!list_empty(keys) && chunk--) {
 		struct key *key =
 			list_entry(keys->next, struct key, graveyard_link);
 		struct module *owner = key->type->owner;
@@ -155,35 +186,46 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
 static void key_garbage_collector(struct work_struct *work)
 {
 	static LIST_HEAD(graveyard);
-	u8 gc_state = 0;
+	typeof(key_gc_work) *gc = &key_gc_work;
 #define KEY_GC_REAP_AGAIN	0x01	/* - Need another cycle */
 #define KEY_GC_REAPING_LINKS	0x02	/* - We need to reap links */
+#define KEY_GC_DESTROYING	0x04	/* - We need to destroy keys */
 
-	struct rb_node *cursor;
+	unsigned int chunk = KEY_GC_CHUNK_SIZE;
+	unsigned long flags;
 	struct key *key;
-	time64_t new_timer, limit, expiry;
+	time64_t expiry;
 
-	kenter("[%lx]", key_gc_flags);
+	kenter("[%lx,%x]", key_gc_flags, gc->state);
 
-	limit = ktime_get_real_seconds();
+	if (gc->state & KEY_GC_DESTROYING)
+		goto destroy_keys;
+	if (gc->cursor) {
+		spin_lock(&key_serial_lock);
+		goto continue_scanning;
+	}
 
-	if (test_and_clear_bit(KEY_GC_KEY_EXPIRED, &key_gc_flags))
-		gc_state |= KEY_GC_REAPING_LINKS;
-	kdebug("new pass %x", gc_state);
+	flags = xchg(&key_gc_flags, 0);
+	gc->start = ktime_get_real_seconds();
+	gc->state = 0;
+	if (flags & BIT(KEY_GC_KEY_EXPIRED))
+		gc->state |= KEY_GC_REAPING_LINKS;
+	kdebug("new pass %x", gc->state);
 
-	new_timer = TIME64_MAX;
+	gc->end = TIME64_MAX;
 
 	/* As only this function is permitted to remove things from the key
 	 * serial tree, if cursor is non-NULL then it will always point to a
 	 * valid node in the tree - even if lock got dropped.
 	 */
 	spin_lock(&key_serial_lock);
-	cursor = rb_first(&key_serial_tree);
+	gc->cursor = rb_first(&key_serial_tree);
 
 continue_scanning:
-	while (cursor) {
-		key = rb_entry(cursor, struct key, serial_node);
-		cursor = rb_next(cursor);
+	while (gc->cursor && chunk) {
+		key = rb_entry(gc->cursor, struct key, serial_node);
+		gc->cursor = rb_next(gc->cursor);
+		chunk--;
 
 		if (!test_bit_acquire(KEY_FLAG_USER_ALIVE, &key->flags)) {
 			/* Clobber key->user after final put seen. */
@@ -194,14 +236,14 @@ static void key_garbage_collector(struct work_struct *work)
 		if (expiry != TIME64_MAX) {
 			if (!(key->type->flags & KEY_TYPE_INSTANT_REAP))
 				expiry += key_gc_delay;
-			if (expiry > limit && expiry < new_timer) {
+			if (expiry > gc->start && expiry < gc->end) {
 				kdebug("will expire %x in %lld",
-				       key_serial(key), key->expiry - limit);
-				new_timer = key->expiry;
+				       key_serial(key), key->expiry - gc->start);
+				gc->end = key->expiry;
 			}
 		}
 
-		if ((gc_state & KEY_GC_REAPING_LINKS) &&
+		if ((gc->state & KEY_GC_REAPING_LINKS) &&
 		    key->type == &key_type_keyring)
 			goto found_keyring;
 
@@ -213,7 +255,11 @@ static void key_garbage_collector(struct work_struct *work)
 	spin_unlock(&key_serial_lock);
 
 maybe_resched:
-	if (cursor) {
+	if (gc->cursor) {
+		if (!chunk) {
+			schedule_work(&key_gc_work.work);
+			goto out;
+		}
 		cond_resched();
 		spin_lock(&key_serial_lock);
 		goto continue_scanning;
@@ -225,9 +271,9 @@ static void key_garbage_collector(struct work_struct *work)
 	 */
 	kdebug("pass complete");
 
-	if (new_timer != TIME64_MAX) {
-		new_timer += key_gc_delay;
-		key_schedule_gc(new_timer);
+	if (gc->end != TIME64_MAX) {
+		gc->end += key_gc_delay;
+		key_schedule_gc(gc->end);
 	}
 
 	if (!list_empty(&graveyard)) {
@@ -238,13 +284,27 @@ static void key_garbage_collector(struct work_struct *work)
 		 */
 		kdebug("gc sync");
 		synchronize_rcu();
+	}
+
+	gc->state |= KEY_GC_DESTROYING;
+
+destroy_keys:
+	if (!list_empty(&graveyard)) {
 		kdebug("gc keys");
 		key_gc_unused_keys(&graveyard);
+		if (!list_empty(&graveyard)) {
+			schedule_work(&key_gc_work.work);
+			goto out;
+		}
 	}
-
-	if (gc_state & KEY_GC_REAP_AGAIN)
-		schedule_work(&key_gc_work);
-	kleave(" [end %x]", gc_state);
+	gc->state &= ~KEY_GC_DESTROYING;
+
+	if ((gc->state & KEY_GC_REAP_AGAIN) ||
+	    test_bit(KEY_GC_REAP_REQUESTED, &key_gc_flags))
+		schedule_work(&key_gc_work.work);
+out:
+	atomic_inc_return(&gc->seq);
+	kleave(" [end %x]", gc->state);
 	return;
 
 	/* We found an unreferenced key - once we've removed it from the tree,
@@ -256,7 +316,7 @@ static void key_garbage_collector(struct work_struct *work)
 	spin_unlock(&key_serial_lock);
 
 	list_add_tail(&key->graveyard_link, &graveyard);
-	gc_state |= KEY_GC_REAP_AGAIN;
+	gc->state |= KEY_GC_REAP_AGAIN;
 	goto maybe_resched;
 
 	/* We found a keyring and we need to check the payload for links to
@@ -266,6 +326,6 @@ static void key_garbage_collector(struct work_struct *work)
 	 */
 found_keyring:
 	spin_unlock(&key_serial_lock);
-	keyring_gc(key, limit);
+	keyring_gc(key, gc->start);
 	goto maybe_resched;
 }
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 005d609ae2df..976f16baf632 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -161,12 +161,11 @@ extern bool lookup_user_key_possessed(const struct key *key,
 extern long join_session_keyring(const char *name);
 extern void key_change_session_keyring(struct callback_head *twork);
 
-extern struct work_struct key_gc_work;
 extern unsigned key_gc_delay;
 extern void keyring_gc(struct key *keyring, time64_t limit);
 void key_set_expiry(struct key *key, time64_t expiry);
 extern void key_schedule_gc(time64_t gc_at);
-extern void key_schedule_gc_links(void);
+void key_schedule_gc_work(bool expired);
 
 extern int key_task_permission(const key_ref_t key_ref,
 			       const struct cred *cred,
diff --git a/security/keys/key.c b/security/keys/key.c
index df6af8066212..374f65c12ac8 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -693,7 +693,7 @@ void key_put(struct key *key)
 			}
 			/* Mark key as safe for GC after key->user done. */
 			clear_bit_unlock(KEY_FLAG_USER_ALIVE, &key->flags);
-			schedule_work(&key_gc_work);
+			key_schedule_gc_work(false);
 		}
 	}
 }
@@ -1214,7 +1214,7 @@ void key_invalidate(struct key *key)
 		down_write_nested(&key->sem, 1);
 		if (!test_and_set_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
 			notify_key(key, NOTIFY_KEY_INVALIDATED, 0);
-			key_schedule_gc_links();
+			key_schedule_gc_work(true);
 		}
 		up_write(&key->sem);
 	}
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index b51abf698aa9..393e6d3fb4bd 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -266,7 +266,7 @@ void key_remove_domain(struct key_tag *domain_tag)
 {
 	domain_tag->removed = true;
 	if (!key_put_tag(domain_tag))
-		key_schedule_gc_links();
+		key_schedule_gc_work(true);
 }
 
 /*
-- 
2.47.3

Re: [RFC PATCH v3 00/11] KEYS: Sanitize the garbage collector

[Jarkko Sakkinen]

On Thu, Nov 05, 2026 at 02:52:59AM +0200, Jarkko Sakkinen wrote:
> 1. Replace key_type_dead retyping with module reference pinning so live
>    keys and restrictions keep their backing key_type module loaded until
>    key_gc_unused_keys() or keyring_destroy() completes.
> 2. Process serial-tree scans and graveyard destruction in fixed 64-key
>    chunks to a bound workqueue execution latency without hurting overall
>    throughput for a significant amount.
> 
> The first part has a user-space visible effect: rmmod returns -EBUSY while
> keys or keyring restrictions still pin the backing module.
> 
> Internal key caches or transport state bottlenecks are addressed by either
> clearing their private caches on last-client teardown or splitting key
> types into dedicated provider modules.

<samba-technical@lists.samba.org>: host
    lists-mx.samba.org[2a01:4f8:192:486::1:0] said: 550-Please subscribe the
    address jarkko@kernel.org to the samba-technical 550-mailing list before
    posting. See <https://lists.samba.org/mailman/listinfo/samba-technical>.
    550-You can also write a mail to samba-technical-owner@lists.samba.org and
    ask 550 to get on the list of accepted non-members to be able to send mail
    here. (in reply to end of DATA command)

This was accidentally pulled by:

	./scripts/get_maintainer.pl \
		--nogit \
		--nogit-fallback \
		--norolestats v3-00?[1-9]-*.patch | \
		awk '{print "Cc: " $0}' | \
		sed -i '/^Subject:/r /dev/stdin' v3-0000-cover-letter.patch

I'll address this in the next version but perhaps that email should be
removed from MAINTAINERS? 

Br, Jarkko