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

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

[Jarkko Sakkinen]

1. Remove key_type_dead and replace it with the regular module semantics.
2. Replace variable size workqueue items with fixed-size 64 key chunks.

The first change has user space visible effect: rmmod semantics will change
from always succeeding into returning -EBUSY when keys or restrictions have
the LKM pinned.

CHANGES
=======

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
========

Jarkko Sakkinen (11):
  KEYS: Add key_gc_flush() for draining RCU callbacks
  nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()
  dns_resolver: Clear .dns_resolver cache when the last client unloads
  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: Track subsystem keys instead of re-registering
    key_type
  KEYS: trusted: TEE: Disable sysfs bind attributes
  KEYS: trusted: TEE: Synchronize session lifetime
  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 ++++++-------
 drivers/nvme/common/keyring.c             |   1 +
 fs/afs/main.c                             |   4 +
 fs/nfs/dns_resolve.h                      |   7 +-
 fs/nfs/nfs4idmap.c                        |  21 ++
 fs/smb/client/cifs_debug.c                |   2 +-
 fs/smb/client/cifs_spnego.c               |   8 +
 fs/smb/client/cifsacl.c                   |   8 +
 fs/smb/client/cifsfs.c                    |   8 +-
 fs/smb/client/cifsglob.h                  |   2 +-
 fs/smb/client/cifsproto.h                 |   8 +
 fs/smb/client/misc.c                      |  19 +-
 include/linux/dns_resolver.h              |  13 +-
 include/linux/key-type.h                  |   6 +-
 include/linux/key.h                       |   2 +
 net/ceph/ceph_common.c                    |   7 +
 net/dns_resolver/dns_key.c                |  28 +++
 security/keys/gc.c                        | 266 +++++++++-------------
 security/keys/internal.h                  |   7 +-
 security/keys/key.c                       |  61 +++--
 security/keys/keyring.c                   |  85 +++----
 security/keys/trusted-keys/trusted_core.c |  15 +-
 security/keys/trusted-keys/trusted_tee.c  |  87 +++++--
 23 files changed, 450 insertions(+), 376 deletions(-)

--
2.47.3

[RFC PATCH v2 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 live keys pin the key type until key_gc_unused_keys(), callers that
clear an internal cache before module unload need a synchronous barrier so
module references are dropped before delete_module() checks the refcount.

Add key_gc_flush() for draining RCU callbacks and flush key_gc_work until
no further garbage collection passes remain queued.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 include/linux/key.h |  2 ++
 security/keys/gc.c  | 14 ++++++++++++++
 2 files changed, 16 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..3cfbf00085b3 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -21,6 +21,20 @@ 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 - Wait for pending key unlinks and garbage collection.
+ *
+ * Drain RCU callbacks from keyring edits and wait for the key garbage
+ * collector work item to finish reaping unreferenced keys.
+ */
+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 v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Jarkko Sakkinen]

Call key_put() before returning -EKEYREVOKED, as the reference count gets
incremented by key_lookup().

Cc: stable@vger.kernel.org # v6.12+
Fixes: 5bc46b49c828 ("nvme-tcp: check for invalidated or revoked key")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/nvme/common/keyring.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c
index 32d16c53133b..87cb51e1f5a7 100644
--- a/drivers/nvme/common/keyring.c
+++ b/drivers/nvme/common/keyring.c
@@ -35,6 +35,7 @@ struct key *nvme_tls_key_lookup(key_serial_t key_id)
 	}
 	if (nvme_tls_psk_revoked(key)) {
 		pr_err("key id %08x revoked\n", key_id);
+		key_put(key);
 		return ERR_PTR(-EKEYREVOKED);
 	}
 	return key;
-- 
2.47.3

[RFC PATCH v2 03/11] dns_resolver: Clear .dns_resolver cache when the last client unloads

[Jarkko Sakkinen]

dns_query() caches resolved dns_resolver keys into the module-owned
.dns_resolver keyring until the TTL expires or module_exit() revokes the
keyring.

Once live keys pin their key type, cached entries keep dns_resolver.ko
pinned after its client modules unload.

Track active kernel client modules with dns_resolver_cache_get() and
dns_resolver_cache_put(), and do the necessary plumbing for:

1. nfsv4
2. cifs
3. libceph
4. kafs

When the last client unloads, clear .dns_resolver and flush key garbage
collection so dns_resolver.ko can unload immediately.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 fs/afs/main.c                |  4 ++++
 fs/nfs/dns_resolve.h         |  7 ++++++-
 fs/smb/client/cifsfs.c       |  4 ++++
 include/linux/dns_resolver.h | 13 ++++++++++---
 net/ceph/ceph_common.c       |  7 +++++++
 net/dns_resolver/dns_key.c   | 28 ++++++++++++++++++++++++++++
 6 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/fs/afs/main.c b/fs/afs/main.c
index 7a883c59976f..16ae947d371e 100644
--- a/fs/afs/main.c
+++ b/fs/afs/main.c
@@ -12,6 +12,7 @@
 #include <linux/sched.h>
 #include <linux/random.h>
 #include <linux/proc_fs.h>
+#include <linux/dns_resolver.h>
 #define CREATE_TRACE_POINTS
 #include "internal.h"
 
@@ -183,6 +184,7 @@ static int __init afs_init(void)
 	if (ret < 0)
 		goto error_net;
 
+	dns_resolver_cache_get();
 	/* register the filesystems */
 	ret = afs_fs_init();
 	if (ret < 0)
@@ -199,6 +201,7 @@ static int __init afs_init(void)
 error_proc:
 	afs_fs_exit();
 error_fs:
+	dns_resolver_cache_put();
 	unregister_pernet_device(&afs_net_ops);
 error_net:
 	destroy_workqueue(afs_lock_manager);
@@ -232,6 +235,7 @@ static void __exit afs_exit(void)
 	destroy_workqueue(afs_wq);
 	afs_clean_up_permit_cache();
 	rcu_barrier();
+	dns_resolver_cache_put();
 }
 
 module_exit(afs_exit);
diff --git a/fs/nfs/dns_resolve.h b/fs/nfs/dns_resolve.h
index fe3b172c4de1..4b9ecfb4f7fe 100644
--- a/fs/nfs/dns_resolve.h
+++ b/fs/nfs/dns_resolve.h
@@ -9,13 +9,18 @@
 
 
 #ifdef CONFIG_NFS_USE_KERNEL_DNS
+#include <linux/dns_resolver.h>
+
 static inline int nfs_dns_resolver_init(void)
 {
+	dns_resolver_cache_get();
 	return 0;
 }
 
 static inline void nfs_dns_resolver_destroy(void)
-{}
+{
+	dns_resolver_cache_put();
+}
 
 static inline int nfs_dns_resolver_cache_init(struct net *net)
 {
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index bf199a5ca084..9788052599ce 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -31,6 +31,7 @@
 #include <linux/xattr.h>
 #include <linux/mm.h>
 #include <linux/key-type.h>
+#include <linux/dns_resolver.h>
 #include <linux/fileattr.h>
 #include <uapi/linux/magic.h>
 #include <net/ipv6.h>
@@ -2160,6 +2161,7 @@ init_cifs(void)
 		goto out_cifs_swn_init;
 
 	cifs_proc_init();
+	dns_resolver_cache_get();
 
 	rc = register_filesystem(&cifs_fs_type);
 	if (rc)
@@ -2174,6 +2176,7 @@ init_cifs(void)
 	return 0;
 
 out_clean_proc:
+	dns_resolver_cache_put();
 	cifs_proc_clean();
 	exit_cifs_idmap();
 out_cifs_swn_init:
@@ -2242,6 +2245,7 @@ exit_cifs(void)
 	destroy_workqueue(serverclose_wq);
 	destroy_workqueue(cfid_put_wq);
 	destroy_workqueue(cifsiod_wq);
+	dns_resolver_cache_put();
 }
 
 MODULE_AUTHOR("Steve French");
diff --git a/include/linux/dns_resolver.h b/include/linux/dns_resolver.h
index 976cbbdb2832..cc11e09b0df3 100644
--- a/include/linux/dns_resolver.h
+++ b/include/linux/dns_resolver.h
@@ -27,8 +27,15 @@
 #include <uapi/linux/dns_resolver.h>
 
 struct net;
-extern int dns_query(struct net *net, const char *type, const char *name, size_t namelen,
-		     const char *options, char **_result, time64_t *_expiry,
-		     bool invalidate);
+#if IS_ENABLED(CONFIG_DNS_RESOLVER)
+int dns_query(struct net *net, const char *type, const char *name, size_t namelen,
+	      const char *options, char **_result, time64_t *_expiry,
+	      bool invalidate);
+void dns_resolver_cache_get(void);
+void dns_resolver_cache_put(void);
+#else
+static inline void dns_resolver_cache_get(void) { }
+static inline void dns_resolver_cache_put(void) { }
+#endif
 
 #endif /* _LINUX_DNS_RESOLVER_H */
diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c
index a797c7360e3c..df62a269cb73 100644
--- a/net/ceph/ceph_common.c
+++ b/net/ceph/ceph_common.c
@@ -8,6 +8,7 @@
 #include <linux/in6.h>
 #include <linux/key.h>
 #include <keys/ceph-type.h>
+#include <linux/dns_resolver.h>
 #include <linux/module.h>
 #include <linux/mount.h>
 #include <linux/nsproxy.h>
@@ -890,6 +891,9 @@ static int __init init_ceph_lib(void)
 	if (ret < 0)
 		goto out_msgr;
 
+#ifdef CONFIG_CEPH_LIB_USE_DNS_RESOLVER
+	dns_resolver_cache_get();
+#endif
 	pr_info("loaded (mon/osd proto %d/%d)\n",
 		CEPH_MONC_PROTOCOL, CEPH_OSDC_PROTOCOL);
 
@@ -913,6 +917,9 @@ static void __exit exit_ceph_lib(void)
 	ceph_msgr_exit();
 	ceph_crypto_shutdown();
 	ceph_debugfs_cleanup();
+#ifdef CONFIG_CEPH_LIB_USE_DNS_RESOLVER
+	dns_resolver_cache_put();
+#endif
 }
 
 module_init(init_ceph_lib);
diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index 451247864a63..63f4fac0dbc8 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/keyctl.h>
 #include <linux/err.h>
+#include <linux/mutex.h>
 #include <linux/seq_file.h>
 #include <linux/dns_resolver.h>
 #include <keys/dns_resolver-type.h>
@@ -31,6 +32,8 @@ module_param_named(debug, dns_resolver_debug, uint, 0644);
 MODULE_PARM_DESC(debug, "DNS Resolver debugging mask");
 
 const struct cred *dns_resolver_cache;
+static DEFINE_MUTEX(dns_resolver_cache_mutex);
+static unsigned int dns_resolver_cache_users;
 
 #define	DNS_ERRORNO_OPTION	"dnserror"
 
@@ -306,6 +309,31 @@ static long dns_resolver_read(const struct key *key,
 	return user_read(key, buffer, buflen);
 }
 
+void dns_resolver_cache_get(void)
+{
+	mutex_lock(&dns_resolver_cache_mutex);
+	dns_resolver_cache_users++;
+	mutex_unlock(&dns_resolver_cache_mutex);
+}
+EXPORT_SYMBOL_GPL(dns_resolver_cache_get);
+
+void dns_resolver_cache_put(void)
+{
+	int ret;
+
+	mutex_lock(&dns_resolver_cache_mutex);
+	if (--dns_resolver_cache_users == 0 && dns_resolver_cache) {
+		ret = keyring_clear(dns_resolver_cache->thread_keyring);
+		if (ret)
+			pr_warn_ratelimited("dns_resolver: Failed to clear cache: %d\n",
+					    ret);
+		else
+			key_gc_flush();
+	}
+	mutex_unlock(&dns_resolver_cache_mutex);
+}
+EXPORT_SYMBOL_GPL(dns_resolver_cache_put);
+
 struct key_type key_type_dns_resolver = {
 	.name		= "dns_resolver",
 	.flags		= KEY_TYPE_NET_DOMAIN | KEY_TYPE_INSTANT_REAP,
-- 
2.47.3

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

[Jarkko Sakkinen]

id_resolver_cache->thread_keyring caches id_resolver_legacy keys until
nfs_idmap_quit() tears down the keyring from module_exit(). Once live
keys pin the key type, cached keys keep nfsv4 pinned so module_exit() is
never reached after all mounts are gone.

Track active idmap instances under a mutex and clear the idmap cache
keyring when the last client is 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..9e3a2255be1e 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;
 
+/* Serialize idmap admission with the last client's cache clear. */
+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 v2 05/11] smb: client: Clear idmap/SPNEGO caches when the last session is unloaded

[Jarkko Sakkinen]

cifs caches cifs.idmap and cifs.spnego keys in module-owned thread
keyrings that are only destroyed in module_exit(). Once live keys pin
the key type, cached entries keep cifs pinned after the last SMB session
is torn down.

Serialize 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_debug.c  |  2 +-
 fs/smb/client/cifs_spnego.c |  8 ++++++++
 fs/smb/client/cifsacl.c     |  8 ++++++++
 fs/smb/client/cifsfs.c      |  4 ++--
 fs/smb/client/cifsglob.h    |  2 +-
 fs/smb/client/cifsproto.h   |  8 ++++++++
 fs/smb/client/misc.c        | 19 +++++++++++++++++--
 7 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index 577f0ddede90..2eecd2516dfe 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -790,7 +790,7 @@ static int cifs_stats_proc_show(struct seq_file *m, void *v)
 	struct cifs_tcon *tcon;
 
 	seq_printf(m, "Resources in use\nCIFS Session: %d\n",
-			sesInfoAllocCount.counter);
+			cifs_ses_alloc_count.counter);
 	seq_printf(m, "Share (unique mount targets): %d\n",
 			tconInfoAllocCount.counter);
 	seq_printf(m, "SMB Request/Response Buffer: %d Pool size: %d\n",
diff --git a/fs/smb/client/cifs_spnego.c b/fs/smb/client/cifs_spnego.c
index 44c407275680..3bd272c7d9fe 100644
--- a/fs/smb/client/cifs_spnego.c
+++ b/fs/smb/client/cifs_spnego.c
@@ -242,6 +242,14 @@ init_cifs_spnego(void)
 	return ret;
 }
 
+void cifs_spnego_clear_cache(void)
+{
+	int ret;
+
+	ret = keyring_clear(spnego_cred->thread_keyring);
+	WARN_ON(ret);
+}
+
 void
 exit_cifs_spnego(void)
 {
diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
index 346899da91eb..7504cedf6417 100644
--- a/fs/smb/client/cifsacl.c
+++ b/fs/smb/client/cifsacl.c
@@ -590,6 +590,14 @@ init_cifs_idmap(void)
 	return ret;
 }
 
+void cifs_idmap_clear_cache(void)
+{
+	int ret;
+
+	ret = keyring_clear(root_cred->thread_keyring);
+	WARN_ON(ret);
+}
+
 void
 exit_cifs_idmap(void)
 {
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 9788052599ce..f8a9a6f7dc49 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -84,7 +84,7 @@ DEFINE_SPINLOCK(GlobalMid_Lock); /* protects above & list operations on midQ ent
 /*
  *  Global counters, updated atomically
  */
-atomic_t sesInfoAllocCount;
+atomic_t cifs_ses_alloc_count;
 atomic_t tconInfoAllocCount;
 atomic_t tcpSesNextId;
 atomic_t tcpSesAllocCount;
@@ -2024,7 +2024,7 @@ init_cifs(void)
 /*
  *  Initialize Global counters
  */
-	atomic_set(&sesInfoAllocCount, 0);
+	atomic_set(&cifs_ses_alloc_count, 0);
 	atomic_set(&tconInfoAllocCount, 0);
 	atomic_set(&tcpSesNextId, 0);
 	atomic_set(&tcpSesAllocCount, 0);
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index d3ca688b6aba..c6e8d1340517 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -2095,7 +2095,7 @@ extern spinlock_t GlobalMid_Lock; /* protects above & list operations on midQ en
 /*
  *  Global counters, updated atomically
  */
-extern atomic_t sesInfoAllocCount;
+extern atomic_t cifs_ses_alloc_count;
 extern atomic_t tconInfoAllocCount;
 extern atomic_t tcpSesNextId;
 extern atomic_t tcpSesAllocCount;
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..4503434942f9 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(&cifs_ses_alloc_count);
+		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(&cifs_ses_alloc_count)) {
+		cifs_idmap_clear_cache();
+		cifs_spnego_clear_cache();
+		key_gc_flush();
+	}
+	mutex_unlock(&cifs_session_mutex);
 }
 
 struct cifs_tcon *
-- 
2.47.3

[RFC PATCH v2 06/11] s390/cert_store: Track subsystem keys instead of re-registering key_type

[Jarkko Sakkinen]

cert_store refreshes its keyring by unregistering and re-registering
key_type_cert_store_key so key_type_dead garbage collection sweeps old
keys, while leaving userspace-linked keys behind.

The removal of key_type_dead will eliminate that sweep.

Register key_type_cert_store_key once during init before creating sysfs
attributes, and track subsystem-created keys explicitly so refresh
unlinks 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 v2 07/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 | 1 +
 1 file changed, 1 insertion(+)

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

[RFC PATCH v2 08/11] KEYS: trusted: TEE: Synchronize session lifetime

[Jarkko Sakkinen]

Even with sysfs bind attributes suppressed on trusted_key_driver, the
parent TEE device can still remove trusted_key_device while live trusted
keys keep trusted.ko loaded, and init_trusted() updated static calls
only after backend init() had already registered key_type_trusted.

Update the static calls before invoking backend init() and reset them on
failure. In the TEE backend, guard session context state with a mutex,
publish pvt_data.dev before registering key_type_trusted, sanitize
pvt_data on probe failure and remove, and reject seal/unseal/get_random
with -ENODEV when no active TEE session is bound.

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 5d1288189e5c..a2ed16e0db3f 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 v2 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       |  3 +++
 security/keys/key.c      | 48 ++++++++++++++++++++++++++++++----------
 security/keys/keyring.c  | 33 ++++++++++++++++++++++-----
 4 files changed, 71 insertions(+), 19 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 3cfbf00085b3..f4ae3e57a4be 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"
@@ -151,6 +152,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);
@@ -179,6 +181,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..1561406b68fa 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);
 	}
@@ -1062,7 +1066,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 +1074,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 +1100,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 v2 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 f4ae3e57a4be..334b86f2c1ca 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -43,22 +43,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
@@ -70,7 +56,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) {
@@ -114,36 +100,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
  */
@@ -195,30 +151,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;
@@ -240,18 +186,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))
@@ -263,21 +197,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;
 	}
@@ -303,8 +226,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
@@ -312,32 +234,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);
@@ -355,14 +255,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
@@ -372,18 +264,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 1561406b68fa..294b13829d88 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -1790,53 +1790,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 v2 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       | 153 ++++++++++++++++++++++++++-------------
 security/keys/internal.h |   3 +-
 security/keys/key.c      |   4 +-
 security/keys/keyring.c  |   2 +-
 4 files changed, 105 insertions(+), 57 deletions(-)

diff --git a/security/keys/gc.c b/security/keys/gc.c
index 334b86f2c1ca..a7da604bb6af 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -11,16 +11,53 @@
 #include <keys/keyring-type.h>
 #include "internal.h"
 
+#define KEY_GC_CHUNK_SIZE 64
+
+/*
+ * Reaper for unused keys.
+ */
+static void key_garbage_collector(struct work_struct *work);
+
+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 */
+	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
  */
-unsigned key_gc_delay = 5 * 60;
+unsigned int key_gc_delay = 5 * 60;
 
 /*
- * Reaper for unused keys.
+ * Reaper for links from keyrings to dead keys.
  */
-static void key_garbage_collector(struct work_struct *work);
-DECLARE_WORK(key_gc_work, key_garbage_collector);
+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 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 - Wait for pending key unlinks and garbage collection.
@@ -32,19 +69,10 @@ void key_gc_flush(void)
 {
 	do {
 		rcu_barrier();
-	} while (flush_work(&key_gc_work));
+	} while (flush_work(&key_gc_work.work));
 }
 EXPORT_SYMBOL_GPL(key_gc_flush);
 
-/*
- * Reaper for links from keyrings to dead keys.
- */
-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 unsigned long key_gc_flags;
-#define KEY_GC_KEY_EXPIRED	0	/* A key expired and needs unlinking */
 /*
  * Schedule a garbage collection run.
  * - time precision isn't particularly important
@@ -58,7 +86,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;
@@ -80,15 +109,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.
@@ -97,7 +117,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);
 }
 
 /*
@@ -105,7 +125,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;
@@ -151,35 +173,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. */
@@ -190,14 +223,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;
 
@@ -209,7 +242,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);
+			return;
+		}
 		cond_resched();
 		spin_lock(&key_serial_lock);
 		goto continue_scanning;
@@ -221,9 +258,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)) {
@@ -234,13 +271,25 @@ 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);
+			return;
+		}
 	}
+	gc->state &= ~KEY_GC_DESTROYING;
 
-	if (gc_state & KEY_GC_REAP_AGAIN)
-		schedule_work(&key_gc_work);
-	kleave(" [end %x]", gc_state);
+	if ((gc->state & KEY_GC_REAP_AGAIN) ||
+	    test_bit(KEY_GC_REAP_REQUESTED, &key_gc_flags))
+		schedule_work(&key_gc_work.work);
+	kleave(" [end %x]", gc->state);
 	return;
 
 	/* We found an unreferenced key - once we've removed it from the tree,
@@ -252,7 +301,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
@@ -262,6 +311,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 294b13829d88..ee6327641356 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 v2 00/11] KEYS: Sanitize the garbage collector

[Jarkko Sakkinen]

On Tue, Nov 03, 2026 at 05:02:05AM +0200, Jarkko Sakkinen wrote:
> CHANGES
> =======
> 
> 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.

I spent hours poking dns_resolver. What I have now is I think somewhat
acceptable or at least any other angle I tried was complete failure :-)

Br, Jarkko

Re: [RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Christoph Hellwig]

On Tue, Nov 03, 2026 at 05:02:07AM +0200, Jarkko Sakkinen wrote:
> Call key_put() before returning -EKEYREVOKED, as the reference count gets
> incremented by key_lookup().

Looks good, but I only got this one patch out of a 11 patch series,
so without the full context it's hard to fully review it.

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

[David Howells]

Jarkko Sakkinen <jarkko@kernel.org> wrote:

> The first change has user space visible effect: rmmod semantics will change
> from always succeeding into returning -EBUSY when keys or restrictions have
> the LKM pinned.

Please don't do that.  That will cause the AF_RXRPC module to get pinned in
place by outstanding connections that haven't timed out yet.

David