[PATCH 0/4] CRASH_ZEROIZE: Wipe secrets before kdump

[PATCH 0/4] CRASH_ZEROIZE: Wipe secrets before kdump

[Jan Sebastian Götte]

I'm using linux on an embedded target in a Hardware Security Module-like
application. One requirement is that I want the system to be able to
quickly erase its memory when it detects physical tampering. I'm
approaching that by using kdump to load into a small payload that
instead of dumping RAM, erases RAM from start to end. However, writing
all of RAM, especially on an embedded target, is rather slow. For this
reason, I propose the mechanism in this patch series:

Add CONFIG_CRASH_ZEROIZE (default off), which when enabled makes various
subsystems handling secret data do a quick, targeted wipe of these
secrets before kdump. This behavior might also be interesting in cases
where you run a normal kdump kernel but you still want to keep things
like fde crypto keys out of these dumps.

CONFIG_CRASH_ZEROIZE is a best effort, defense in depth solution. There
are circumstances, such as when a panic is triggered after memory
corruption, or when a panic interrupts some operation that mutates data
structures under locks, when the kernel cannot safely wipe some memory
areas. The handlers proposed in this series will just print a warning
and skip the affected areas in this case.

This series introduces two handlers as a starting point: One for kernel
keyrings, and one for secretmem. Future places where such handlers could
be added would be for example drivers for crypto accelerators. 

The patch series applies on top of linux-next but should work on 7.0.0,
too. I've tested the patches on a Arduino uno Q (Qualcomm QRB2210)
embedded target.

Jan Sebastian Götte (4):
  of/kexec: fix typo in comment (usable-memory-range)
  kexec: add CRASH_ZEROIZE to wipe secrets before kdump
  mm/secretmem: zeroize secret pages before kdump
  security/keys: zeroize key payloads before kdump

 drivers/of/kexec.c                        |  2 +-
 include/linux/crash_core.h                |  5 +++
 include/linux/key-type.h                  |  9 ++++
 kernel/Kconfig.kexec                      |  8 ++++
 kernel/crash_core.c                       | 18 ++++++++
 mm/secretmem.c                            | 50 +++++++++++++++++++++++
 security/keys/big_key.c                   | 15 +++++++
 security/keys/encrypted-keys/encrypted.c  | 12 ++++++
 security/keys/key.c                       | 44 ++++++++++++++++++++
 security/keys/trusted-keys/trusted_core.c | 14 +++++++
 security/keys/user_defined.c              | 11 +++++
 11 files changed, 187 insertions(+), 1 deletion(-)

-- 
2.53.0

[PATCH 1/4] of/kexec: fix typo in comment (usable-memory-range)

[Jan Sebastian Götte]

Signed-off-by: Jan Sebastian Götte <linux@jaseg.de>
---
 drivers/of/kexec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index 029903b986cb..36635f6cb900 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -24,7 +24,7 @@
 
 /*
  * Additional space needed for the FDT buffer so that we can add initrd,
- * bootargs, kaslr-seed, rng-seed, useable-memory-range and elfcorehdr.
+ * bootargs, kaslr-seed, rng-seed, usable-memory-range and elfcorehdr.
  */
 #define FDT_EXTRA_SPACE 0x1000
 
-- 
2.53.0

[PATCH 2/4] kexec: add CRASH_ZEROIZE to wipe secrets before kdump

[Jan Sebastian Götte]

When kdump is used to capture system memory after a panic(), any secret
keys currently in RAM end up in the dump. Add an opt-in atomic notifier
chain, crash_zeroize_notifier_list, invoked late into __crash_kexec().
Subsystems holding secrets can register a callback to scrub them.

Callbacks are run after machine_crash_shutdown() has already stopped the
other CPUs and disabled preemption. Callbacks must not wait on locks,
which will never be released.

This is a best-effort, defence-in-depth measure, not a guarantee.
Secrets in flight on the stack, in registers, in DMA buffers, or in
other places in memory are out of scope.

Signed-off-by: Jan Sebastian Götte <linux@jaseg.de>
---
 include/linux/crash_core.h |  5 +++++
 kernel/Kconfig.kexec       |  8 ++++++++
 kernel/crash_core.c        | 18 ++++++++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index bc087124cd78..5c7207c0bba1 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -5,6 +5,7 @@
 #include <linux/linkage.h>
 #include <linux/elfcore.h>
 #include <linux/elf.h>
+#include <linux/notifier.h>
 
 struct kimage;
 
@@ -34,6 +35,10 @@ static inline void arch_kexec_protect_crashkres(void) { }
 static inline void arch_kexec_unprotect_crashkres(void) { }
 #endif
 
+#ifdef CONFIG_CRASH_ZEROIZE
+extern struct atomic_notifier_head crash_zeroize_notifier_list;
+#endif
+
 #ifndef arch_crash_handle_hotplug_event
 static inline void arch_crash_handle_hotplug_event(struct kimage *image, void *arg) { }
 #endif
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 15632358bcf7..92ab0a69c8ec 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -179,4 +179,12 @@ config CRASH_MAX_MEMORY_RANGES
 	  the computation behind the value provided through the
 	  /sys/kernel/crash_elfcorehdr_size attribute.
 
+config CRASH_ZEROIZE
+	bool "Zeroize secrets on panic"
+	depends on CRASH_DUMP
+	help
+	  Wipe secrets (e.g. kernel keyring and memfd_secret pages) on crash or panic.
+
+	  If unsure, say N.
+
 endmenu
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 2b36aa9fade0..d3a7763e2759 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -23,6 +23,7 @@
 #include <linux/objtool.h>
 #include <linux/delay.h>
 #include <linux/panic.h>
+#include <linux/timekeeping.h>
 
 #include <asm/page.h>
 #include <asm/sections.h>
@@ -33,6 +34,22 @@
 /* Per cpu memory for storing cpu states in case of system crash. */
 note_buf_t __percpu *crash_notes;
 
+#ifdef CONFIG_CRASH_ZEROIZE
+ATOMIC_NOTIFIER_HEAD(crash_zeroize_notifier_list);
+EXPORT_SYMBOL_GPL(crash_zeroize_notifier_list);
+
+static void crash_zeroize(void)
+{
+	ktime_t zeroize_start = ktime_get();
+
+	pr_info("Wiping sensitive secrets...\n");
+	atomic_notifier_call_chain(&crash_zeroize_notifier_list, 0, NULL);
+	pr_info("Done in %lld us\n", ktime_us_delta(ktime_get(), zeroize_start));
+}
+#else
+static inline void crash_zeroize(void) { }
+#endif /* CONFIG_CRASH_ZEROIZE */
+
 /* time to wait for possible DMA to finish before starting the kdump kernel
  * when a CMA reservation is used
  */
@@ -142,6 +159,7 @@ void __noclone __crash_kexec(struct pt_regs *regs)
 			crash_save_vmcoreinfo();
 			machine_crash_shutdown(&fixed_regs);
 			crash_cma_clear_pending_dma();
+			crash_zeroize();
 			machine_kexec(kexec_crash_image);
 		}
 		kexec_unlock();
-- 
2.53.0

[PATCH 3/4] mm/secretmem: zeroize secret pages before kdump

[Jan Sebastian Götte]

Register a CRASH_ZEROIZE notifier that wipes secretmem folios. As a
result, when CONFIG_CRASH_ZEROIZE is set, secretmem areas will be
cleared before the kdump kernel is kexec'ed.

Zeroization runs after the other CPUs have been stopped, so the page
cache cannot be mutated concurrently and the xarray may be walked
without taking the i_pages lock. This is a best effort, defense in depth
measure. s_inode_list_lock is taken with trylock only. If a CPU was
stopped mid-modification the list may be inconsistent, and this late
into the panic path, there's nothing we can do about it.

Signed-off-by: Jan Sebastian Götte <linux@jaseg.de>
---
 mm/secretmem.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/mm/secretmem.c b/mm/secretmem.c
index d29865075b6e..53f629d6633c 100644
--- a/mm/secretmem.c
+++ b/mm/secretmem.c
@@ -13,9 +13,11 @@
 #include <linux/bitops.h>
 #include <linux/printk.h>
 #include <linux/pagemap.h>
+#include <linux/notifier.h>
 #include <linux/syscalls.h>
 #include <linux/pseudo_fs.h>
 #include <linux/secretmem.h>
+#include <linux/crash_core.h>
 #include <linux/set_memory.h>
 #include <linux/sched/signal.h>
 
@@ -187,6 +189,50 @@ static const struct inode_operations secretmem_iops = {
 
 static struct vfsmount *secretmem_mnt;
 
+#ifdef CONFIG_CRASH_ZEROIZE
+/* Called far into vpanic from crash_core.c with other CPUs stopped and
+ * preemption disabled
+ */
+static int secretmem_crash_zeroize(struct notifier_block *nb, unsigned long
+		action, void *data)
+{
+	struct super_block *sb;
+	struct inode *inode;
+
+	if (!secretmem_mnt)
+		return NOTIFY_DONE;
+	sb = secretmem_mnt->mnt_sb;
+
+	/* If the list was modified in the exact moment we panic'ed, it might be
+	 * in an inconsistent state that would be unsafe to iterate. If we can't
+	 * get the lock, too bad, that's all we can do here.
+	 */
+	if (!spin_trylock(&sb->s_inode_list_lock)) {
+		pr_crit("crash_zeroize: can't acquire secretmem superblock lock.\n"
+			 "crash_zeroize: skipping zeroizing secretmem.\n");
+		return NOTIFY_DONE;
+	}
+
+	list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
+		XA_STATE(xas, &inode->i_mapping->i_pages, 0);
+		struct folio *folio;
+
+		/* no need for locks if we're burning down the house :) */
+		xas_for_each(&xas, folio, ULONG_MAX) {
+			if (xas_retry(&xas, folio) || xa_is_value(folio))
+				continue;
+			inode->i_mapping->a_ops->free_folio(folio);
+		}
+	}
+	/* off to kexec()! */
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block secretmem_zeroize_nb = {
+	.notifier_call = secretmem_crash_zeroize
+};
+#endif /* CONFIG_CRASH_ZEROIZE */
+
 static struct file *secretmem_file_create(unsigned long flags)
 {
 	struct file *file;
@@ -263,6 +309,10 @@ static int __init secretmem_init(void)
 	if (IS_ERR(secretmem_mnt))
 		return PTR_ERR(secretmem_mnt);
 
+#ifdef CONFIG_CRASH_ZEROIZE
+	atomic_notifier_chain_register(&crash_zeroize_notifier_list, &secretmem_zeroize_nb);
+#endif
+
 	return 0;
 }
 fs_initcall(secretmem_init);
-- 
2.53.0

[PATCH 4/4] security/keys: zeroize key payloads before kdump

[Jan Sebastian Götte]

When CONFIG_CRASH_ZEROIZE is set, try to erase key payloads on panic
before jumping to the kdump kernel.

CRASH_ZEROIZE notifiers run during panic() with other CPUs stopped and
preemption disabled. In this state, we can't rely on free()'ing being
safe, so we define a new zeroize key op.

Implement the zeroize op for the user/logon, encrypted, trusted, and
big_key types.

Signed-off-by: Jan Sebastian Götte <linux@jaseg.de>
---
 include/linux/key-type.h                  |  9 +++++
 security/keys/big_key.c                   | 15 ++++++++
 security/keys/encrypted-keys/encrypted.c  | 12 +++++++
 security/keys/key.c                       | 44 +++++++++++++++++++++++
 security/keys/trusted-keys/trusted_core.c | 14 ++++++++
 security/keys/user_defined.c              | 11 ++++++
 6 files changed, 105 insertions(+)

diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index bb97bd3e5af4..ff0944ce368f 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -122,6 +122,15 @@ struct key_type {
 	/* clear the data from a key (optional) */
 	void (*destroy)(struct key *key);
 
+	/* scrub the key material without free'ing (optional)
+	 * - used from CONFIG_CRASH_ZEROIZE during panic to keep keys out of
+	 *   crash dumps
+	 * - called from the panic path with other CPUs stopped and preemption
+	 *   disabled
+	 * - must not sleep, allocate, free or take locks
+	 */
+	void (*zeroize)(struct key *key);
+
 	/* describe a key */
 	void (*describe)(const struct key *key, struct seq_file *p);
 
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index 268f702df380..ad8537dda70f 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -35,6 +35,8 @@ struct big_key_payload {
  */
 #define BIG_KEY_FILE_THRESHOLD (sizeof(struct inode) + sizeof(struct dentry))
 
+static void big_key_zeroize(struct key *key);
+
 /*
  * big_key defined keys take an arbitrary string as the description and an
  * arbitrary blob of data as the payload
@@ -46,6 +48,7 @@ struct key_type key_type_big_key = {
 	.instantiate		= generic_key_instantiate,
 	.revoke			= big_key_revoke,
 	.destroy		= big_key_destroy,
+	.zeroize		= big_key_zeroize,
 	.describe		= big_key_describe,
 	.read			= big_key_read,
 	.update			= big_key_update,
@@ -279,6 +282,18 @@ long big_key_read(const struct key *key, char *buffer, size_t buflen)
 	return ret;
 }
 
+static void big_key_zeroize(struct key *key)
+{
+	struct big_key_payload *payload = to_big_key_payload(key->payload);
+
+	if (payload->data) {
+		if (payload->length > BIG_KEY_FILE_THRESHOLD)
+			memzero_explicit(payload->data, CHACHA20POLY1305_KEY_SIZE);
+		else
+			memzero_explicit(payload->data, payload->length);
+	}
+}
+
 /*
  * Register key type
  */
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 59cb77b237b3..9f56fa9b4aaf 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -970,11 +970,23 @@ static void encrypted_destroy(struct key *key)
 	kfree_sensitive(key->payload.data[0]);
 }
 
+static void encrypted_zeroize(struct key *key)
+{
+	struct encrypted_key_payload *epayload = key->payload.data[0];
+
+	if (!epayload)
+		return;
+
+	memzero_explicit(epayload->payload_data,
+			 epayload->payload_datalen + epayload->datablob_len);
+}
+
 struct key_type key_type_encrypted = {
 	.name = "encrypted",
 	.instantiate = encrypted_instantiate,
 	.update = encrypted_update,
 	.destroy = encrypted_destroy,
+	.zeroize = encrypted_zeroize,
 	.describe = user_describe,
 	.read = encrypted_read,
 };
diff --git a/security/keys/key.c b/security/keys/key.c
index b34a64d81d47..5673dcc8c5d2 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -12,6 +12,7 @@
 #include <linux/slab.h>
 #include <linux/security.h>
 #include <linux/workqueue.h>
+#include <linux/crash_core.h>
 #include <linux/random.h>
 #include <linux/err.h>
 #include "internal.h"
@@ -1268,6 +1269,44 @@ void unregister_key_type(struct key_type *ktype)
 }
 EXPORT_SYMBOL(unregister_key_type);
 
+#ifdef CONFIG_CRASH_ZEROIZE
+/* Called far into vpanic from crash_core.c with other CPUs stopped and
+ * preemption disabled
+ */
+static int key_crash_zeroize(struct notifier_block *nb, unsigned long action,
+		void *data)
+{
+	struct rb_node *node;
+
+	/* If we can't acquire the lock, the rbtree might be in an inconsistent
+	 * state. That's all we can do then, as there's no point to waiting
+	 * at this stage.
+	 */
+	if (!spin_trylock(&key_serial_lock)) {
+		pr_crit("crash_zeroize: can't acquire key_serial_lock. skipping keyrings.\n");
+		return NOTIFY_DONE;
+	}
+
+	for (node = rb_first(&key_serial_tree); node; node = rb_next(node)) {
+		struct key *key = rb_entry(node, struct key, serial_node);
+
+		if (key->type == &key_type_keyring ||
+		    key->state == KEY_IS_UNINSTANTIATED)
+			continue;
+
+		/* custom zeroize since free'ing isn't safe at this point */
+		if (key->type->zeroize)
+			key->type->zeroize(key);
+	}
+	/* off to kexec()! */
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block key_crash_zeroize_nb = {
+	.notifier_call = key_crash_zeroize
+};
+#endif /* CONFIG_CRASH_ZEROIZE */
+
 /*
  * Initialise the key management state.
  */
@@ -1290,4 +1329,9 @@ void __init key_init(void)
 
 	rb_insert_color(&root_key_user.node,
 			&key_user_tree);
+
+#ifdef CONFIG_CRASH_ZEROIZE
+	atomic_notifier_chain_register(&crash_zeroize_notifier_list,
+			&key_crash_zeroize_nb);
+#endif
 }
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index 0509d9955f2a..f159faeafe23 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -325,11 +325,25 @@ static void trusted_destroy(struct key *key)
 	kfree_sensitive(key->payload.data[0]);
 }
 
+static void trusted_zeroize(struct key *key)
+{
+	struct trusted_key_payload *p = key->payload.data[0];
+
+	if (!p)
+		return;
+
+	memzero_explicit(p->key, sizeof(p->key));
+	memzero_explicit(p->blob, sizeof(p->blob));
+	p->key_len = 0;
+	p->blob_len = 0;
+}
+
 struct key_type key_type_trusted = {
 	.name = "trusted",
 	.instantiate = trusted_instantiate,
 	.update = trusted_update,
 	.destroy = trusted_destroy,
+	.zeroize = trusted_zeroize,
 	.describe = user_describe,
 	.read = trusted_read,
 };
diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index 6f88b507f927..ade95dc2481d 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -15,6 +15,7 @@
 #include "internal.h"
 
 static int logon_vet_description(const char *desc);
+static void user_zeroize(struct key *key);
 
 /*
  * user defined keys take an arbitrary string as the description and an
@@ -28,6 +29,7 @@ struct key_type key_type_user = {
 	.update			= user_update,
 	.revoke			= user_revoke,
 	.destroy		= user_destroy,
+	.zeroize		= user_zeroize,
 	.describe		= user_describe,
 	.read			= user_read,
 };
@@ -48,6 +50,7 @@ struct key_type key_type_logon = {
 	.update			= user_update,
 	.revoke			= user_revoke,
 	.destroy		= user_destroy,
+	.zeroize		= user_zeroize,
 	.describe		= user_describe,
 	.vet_description	= logon_vet_description,
 };
@@ -152,6 +155,14 @@ void user_destroy(struct key *key)
 
 EXPORT_SYMBOL_GPL(user_destroy);
 
+static void user_zeroize(struct key *key)
+{
+	struct user_key_payload *upayload = key->payload.data[0];
+
+	if (upayload)
+		memzero_explicit(upayload->data, upayload->datalen);
+}
+
 /*
  * describe the user key
  */
-- 
2.53.0

Re: [PATCH 2/4] kexec: add CRASH_ZEROIZE to wipe secrets before kdump

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Deadlock in `crash_zeroize()` due to `ktime_get()` during kdump panic path
- [Low] Missing registration wrapper for conditionally compiled `crash_zeroize_notifier_list`

Pre-existing issues:
- [High] Pre-existing deadlock risk in `crash_save_vmcoreinfo` on 32-bit platforms
--

--- Patch [2]: [PATCH 2/4] kexec: add CRASH_ZEROIZE to wipe secrets before kdump ---
commit 3005bf8ed21843217c4ae3efeb7db1241b960e62
Author: Jan Sebastian Götte <linux@jaseg.de>

kexec: add CRASH_ZEROIZE to wipe secrets before kdump

When kdump is used to capture system memory after a panic(), any secret
keys currently in RAM end up in the dump. Add an opt-in atomic notifier
chain, crash_zeroize_notifier_list, invoked late into __crash_kexec().
Subsystems holding secrets can register a callback to scrub them.

> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h

[ ... ]

> @@ -34,6 +35,10 @@ static inline void arch_kexec_protect_crashkres(void) { }
>  static inline void arch_kexec_unprotect_crashkres(void) { }
>  #endif
>  
> +#ifdef CONFIG_CRASH_ZEROIZE
> +extern struct atomic_notifier_head crash_zeroize_notifier_list;
> +#endif

[Severity: Low]
Is it possible to provide a registration wrapper instead of exporting the
list directly? 

If developers try to register a callback using 
atomic_notifier_chain_register() when CONFIG_CRASH_ZEROIZE is disabled, they
will encounter a build failure unless they wrap the call in #ifdef guards.
Adding a wrapper like register_crash_zeroize_notifier() with a static inline
stub for the !CONFIG case would avoid this.

> +
>  #ifndef arch_crash_handle_hotplug_event
>  static inline void arch_crash_handle_hotplug_event(struct kimage *image, void *arg) { }
>  #endif

[ ... ]

> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c

[ ... ]

> @@ -33,6 +34,22 @@
>  /* Per cpu memory for storing cpu states in case of system crash. */
>  note_buf_t __percpu *crash_notes;
>  
> +#ifdef CONFIG_CRASH_ZEROIZE
> +ATOMIC_NOTIFIER_HEAD(crash_zeroize_notifier_list);
> +EXPORT_SYMBOL_GPL(crash_zeroize_notifier_list);
> +
> +static void crash_zeroize(void)
> +{
> +	ktime_t zeroize_start = ktime_get();

[Severity: High]
Can this ktime_get() call cause a deadlock? 

When panic() triggers kdump, it calls machine_crash_shutdown() to stop all
other CPUs. Since crash_zeroize() executes immediately after this shutdown
step, if any other CPU was executing a timekeeping update and holding
tk_core.seq for writing when it was permanently stopped by an NMI/IPI, the
panicking CPU will spin forever in read_seqcount_begin().

This would entirely prevent the crash dump from being collected. Should this
use a lockless accessor like ktime_get_mono_fast_ns() instead?

> +
> +	pr_info("Wiping sensitive secrets...\n");
> +	atomic_notifier_call_chain(&crash_zeroize_notifier_list, 0, NULL);
> +	pr_info("Done in %lld us\n", ktime_us_delta(ktime_get(), zeroize_start));
> +}
> +#else
> +static inline void crash_zeroize(void) { }
> +#endif /* CONFIG_CRASH_ZEROIZE */
> +

[ ... ]

> @@ -142,6 +159,7 @@ void __noclone __crash_kexec(struct pt_regs *regs)
>  			crash_save_vmcoreinfo();

[Severity: High]
This is a pre-existing issue, but does crash_save_vmcoreinfo() carry a
similar deadlock risk on 32-bit platforms?

Looking at crash_save_vmcoreinfo(), it calls ktime_get_real_seconds(), which
relies on the tk_core.seq sequence lock on 32-bit architectures. 

If the panicking CPU interrupted a timekeeping update, or if another CPU was
stopped while holding the lock, this call will spin forever and block kdump.
Using __ktime_get_real_seconds() would bypass the sequence lock.

>  			machine_crash_shutdown(&fixed_regs);
>  			crash_cma_clear_pending_dma();
> +			crash_zeroize();
>  			machine_kexec(kexec_crash_image);
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731162739.158320-1-linux@jaseg.de?part=2

Re: [PATCH 3/4] mm/secretmem: zeroize secret pages before kdump

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Deadlock risk during kdump due to taking a spinlock in a panic notifier.
- [Low] Missing unlock of the `s_inode_list_lock` after successfully acquiring it.
--

--- Patch [3]: [PATCH 3/4] mm/secretmem: zeroize secret pages before kdump ---
commit e5fefbd7f9b60fca2237ceaf3f2f3cb2aa75df03
Author: Jan Sebastian Götte <linux@jaseg.de>

mm/secretmem: zeroize secret pages before kdump
    
Register a CRASH_ZEROIZE notifier that wipes secretmem folios. As a
result, when CONFIG_CRASH_ZEROIZE is set, secretmem areas will be
cleared before the kdump kernel is kexec'ed.

> diff --git a/mm/secretmem.c b/mm/secretmem.c
> index 5f57ac4720d32..07f3905f47cbd 100644
> --- a/mm/secretmem.c
> +++ b/mm/secretmem.c

[ ... ]

> @@ -187,6 +189,50 @@ static const struct inode_operations secretmem_iops = {
>  
>  static struct vfsmount *secretmem_mnt;
>  
> +#ifdef CONFIG_CRASH_ZEROIZE
> +/* Called far into vpanic from crash_core.c with other CPUs stopped and
> + * preemption disabled
> + */
> +static int secretmem_crash_zeroize(struct notifier_block *nb, unsigned long
> +		action, void *data)
> +{
> +	struct super_block *sb;
> +	struct inode *inode;
> +
> +	if (!secretmem_mnt)
> +		return NOTIFY_DONE;
> +	sb = secretmem_mnt->mnt_sb;
> +
> +	/* If the list was modified in the exact moment we panic'ed, it might be
> +	 * in an inconsistent state that would be unsafe to iterate. If we can't
> +	 * get the lock, too bad, that's all we can do here.
> +	 */
> +	if (!spin_trylock(&sb->s_inode_list_lock)) {
> +		pr_crit("crash_zeroize: can't acquire secretmem superblock lock.\n"
> +			 "crash_zeroize: skipping zeroizing secretmem.\n");
> +		return NOTIFY_DONE;
> +	}
> +
> +	list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> +		XA_STATE(xas, &inode->i_mapping->i_pages, 0);
> +		struct folio *folio;
> +
> +		/* no need for locks if we're burning down the house :) */
> +		xas_for_each(&xas, folio, ULONG_MAX) {
> +			if (xas_retry(&xas, folio) || xa_is_value(folio))
> +				continue;
> +			inode->i_mapping->a_ops->free_folio(folio);

[Severity: Critical]
Can this free_folio callback deadlock the panicking CPU?

The free_folio operation maps to secretmem_free_folio(), which invokes
set_direct_map_default_noflush(). On x86, this eventually calls
__change_page_attr_set_clr(), which unconditionally acquires cpa_lock
(when debug_pagealloc is not enabled):

arch/x86/mm/pat/set_memory.c:__change_page_attr_set_clr() {
    ...
    if (!debug_pagealloc_enabled())
        spin_lock(&cpa_lock);
    ...
}

Because this crash notifier runs after other CPUs have been forcibly halted
(e.g., via NMIs), if any of the stopped CPUs were interrupted while holding
cpa_lock (such as during module load or vmalloc), won't the panicking CPU
spin forever here and prevent the crash dump kernel from booting?

> +		}
> +	}
> +	/* off to kexec()! */
> +	return NOTIFY_DONE;

[Severity: Low]
Should we unlock sb->s_inode_list_lock before returning?

While this is the panic path and the system is about to kexec into the crash
kernel, leaving a successfully acquired spin_trylock unbalanced can trigger
warnings in static analysis tools or confuse readers.

> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731162739.158320-1-linux@jaseg.de?part=3

Re: [PATCH 4/4] security/keys: zeroize key payloads before kdump

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Torn payload updates and in-place frees in `big_key_update` cause Use-After-Free and Out-Of-Bounds writes in `big_key_zeroize` during panic.
- [High] Sensitive key types (like NVMe TLS PSK and Ceph keys) are missing zeroization callbacks and will leak in crash dumps.
- [High] Keys removed from the rbtree by the garbage collector but not yet destroyed are missed by the crash zeroization mechanism.
- [High] Keys whose payloads are currently being instantiated are skipped during panic zeroization due to a TOCTOU race with `key->state`.
- [High] The entire crash zeroization mechanism can be reliably bypassed if `key_serial_lock` is held when panic occurs.
--

--- Patch [4]: [PATCH 4/4] security/keys: zeroize key payloads before kdump ---
commit 6f552c7707db0ce9c5a1ed7eeec0a2afb30b26a3
Author: Jan Sebastian Götte <linux@jaseg.de>

security/keys: zeroize key payloads before kdump

When CONFIG_CRASH_ZEROIZE is set, try to erase key payloads on panic before
jumping to the kdump kernel. It implements the zeroize op for the user/logon,
encrypted, trusted, and big_key types.

> diff --git a/security/keys/big_key.c b/security/keys/big_key.c
> index 268f702df380e..ad8537dda70fa 100644
> --- a/security/keys/big_key.c
> +++ b/security/keys/big_key.c

[ ... ]

> @@ -279,6 +282,18 @@ long big_key_read(const struct key *key, char *buffer, size_t buflen)
>  	return ret;
>  }
>  
> +static void big_key_zeroize(struct key *key)
> +{
> +	struct big_key_payload *payload = to_big_key_payload(key->payload);
> +
> +	if (payload->data) {
> +		if (payload->length > BIG_KEY_FILE_THRESHOLD)
> +			memzero_explicit(payload->data, CHACHA20POLY1305_KEY_SIZE);
> +		else
> +			memzero_explicit(payload->data, payload->length);
> +	}
> +}

[Severity: High]
Can this result in a use-after-free or out-of-bounds write if a panic occurs
during big_key_update?

In big_key_update(), the key payload is modified in-place:

security/keys/big_key.c:big_key_update() {
        ...
        if (key_is_positive(key))
                big_key_destroy(key);

        return generic_key_instantiate(key, prep);
}

generic_key_instantiate() updates the payload union sequentially. If a panic
occurs exactly when the payload pointer is freed, or when the new pointer is
written but payload->length is still the old length, this zeroize function
will read the torn state asynchronously and potentially write zeroes to freed
memory or beyond the bounds of the new buffer.

> diff --git a/security/keys/key.c b/security/keys/key.c
> index 091ee084bc300..4f0e8e579cebd 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c

[ ... ]

> @@ -1268,6 +1269,44 @@ void unregister_key_type(struct key_type *ktype)
>  }
>  EXPORT_SYMBOL(unregister_key_type);
>  
> +#ifdef CONFIG_CRASH_ZEROIZE
> +/* Called far into vpanic from crash_core.c with other CPUs stopped and
> + * preemption disabled
> + */
> +static int key_crash_zeroize(struct notifier_block *nb, unsigned long action,
> +		void *data)
> +{
> +	struct rb_node *node;
> +
> +	/* If we can't acquire the lock, the rbtree might be in an inconsistent
> +	 * state. That's all we can do then, as there's no point to waiting
> +	 * at this stage.
> +	 */
> +	if (!spin_trylock(&key_serial_lock)) {
> +		pr_crit("crash_zeroize: can't acquire key_serial_lock. skipping keyrings.\n");
> +		return NOTIFY_DONE;
> +	}

[Severity: High]
Does this completely bypass zeroization if another CPU holds the lock when
the panic occurs?

Any legitimate action on a separate CPU that briefly acquires key_serial_lock
will cause the panicking CPU to fail this trylock. Could this be used to
reliably bypass the crash zeroize protection for all keys if the panic is
triggered during that window?

> +
> +	for (node = rb_first(&key_serial_tree); node; node = rb_next(node)) {
> +		struct key *key = rb_entry(node, struct key, serial_node);

[Severity: High]
Are keys that are pending destruction missed by this loop?

When a key is unreferenced, key_garbage_collector() removes it from the
key_serial_tree and adds it to a local graveyard list before freeing it:

security/keys/gc.c:key_garbage_collector() {
        ...
        rb_erase(&key->serial_node, &key_serial_tree);
        spin_unlock(&key_serial_lock);

        list_add_tail(&key->graveyard_link, &graveyard);
        ...
}

If a panic happens between the removal from the tree and the actual
destruction (which can involve sleeping for synchronize_rcu), the key
material is still in memory but seems to be skipped here.

> +
> +		if (key->type == &key_type_keyring ||
> +		    key->state == KEY_IS_UNINSTANTIATED)
> +			continue;

[Severity: High]
Can this skip newly instantiated keys due to a race condition?

In __key_instantiate_and_link(), the payload is populated before the state
is updated:

security/keys/key.c:__key_instantiate_and_link() {
        ...
        ret = key->type->instantiate(key, prep);

        if (ret == 0) {
                atomic_inc(&key->user->nikeys);
                mark_key_instantiated(key, 0);
        ...
}

If a panic occurs in the window after the instantiate callback copies the
secret material but before the state changes from KEY_IS_UNINSTANTIATED,
the key material is already present but will be skipped by this check.

> +
> +		/* custom zeroize since free'ing isn't safe at this point */
> +		if (key->type->zeroize)
> +			key->type->zeroize(key);

[Severity: High]
Will this silently leak sensitive keys that lack a zeroize callback?

Several sensitive key types appear to be missing the zeroize callback
definition in this patch, such as nvme_tls_psk_key_type in
drivers/nvme/common/keyring.c and key_type_ceph in net/ceph/crypto.c.

Since this loop skips any type without a zeroize callback, those
cryptographic keys will bypass the crash zeroize mechanism and remain
in crash dumps.

> +	}
> +	/* off to kexec()! */
> +	return NOTIFY_DONE;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731162739.158320-1-linux@jaseg.de?part=4