Re: [RFC PATCH 14/20] kho: Add crash-kernel-safe radix tree presence check

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] In-place conversion destructively modifies the old kernel's memory, corrupting the crash dump.
- [High] Unbounded per-node `memremap()` calls can quickly exhaust the crash kernel's limited memory resources.
- [High] Lack of physical address validation before `memremap()` risks triggering Machine Check Exceptions (MCE).
- [Medium] `kho_radix_crash_contains_page()` fails to identify subpages of higher-order folios.
- [Medium] The exported API lacks a teardown function, resulting in permanent vmap memory leaks.
--

commit c1bba78846e7698e1b8a124f8432407d41f69b0c
Author: Jork Loeser <jloeser@linux.microsoft.com>
Subject: kho: Add crash-kernel-safe radix tree presence check

In the crash kernel, the old kernel's memory is outside the direct map.
This patch adds a read-only radix tree variant that memremaps nodes during
init so that subsequent page presence checks can traverse the tree with
plain pointer dereferencing.

> diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> index 2e2b4e73f00d3..0dfdf0f9781ef 100644
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
[ ... ]
> @@ -396,6 +397,129 @@ void kho_radix_destroy_tree(struct kho_radix_tree *tree)
>  }
>  EXPORT_SYMBOL_GPL(kho_radix_destroy_tree);
>  
> +/*
> + * Convert a crash tree node's children from PA to VA in-place via memremap().
> + * On failure, already-remapped pages are not cleaned up -- the crash kernel
> + * is short-lived and will reboot after dump collection, so the leak is
> + * inconsequential.
> + */
> +static int kho_radix_crash_convert_node(struct kho_radix_node *node,
> +					unsigned int level)
> +{
> +	struct kho_radix_node *child;
> +	unsigned int i;
> +	int err;
> +
> +	for (i = 0; i < (1 << KHO_TABLE_SIZE_LOG2); i++) {
> +		if (!node->table[i])
> +			continue;
> +
> +		/* Validate: PA must have bit 63 clear and be page-aligned */
> +		if ((node->table[i] & BIT_ULL(63)) ||
> +		    (node->table[i] & (PAGE_SIZE - 1)))
> +			return -EINVAL;
> +
> +		child = memremap(node->table[i], PAGE_SIZE, MEMREMAP_WB);

[Severity: High]
Since this recursively calls memremap() for every populated node in the tree,
could this exhaust the crash kernel's limited memory resources? A fully
populated tree might create thousands of individual virtual mappings, consuming
significant memory for tracking structures like vm_struct and page tables.

Also, the untrusted physical addresses read from the crashed kernel's memory
are passed blindly to memremap(). If the old kernel crashed due to memory
corruption, could these pointers contain garbage values pointing to non-RAM
regions like MMIO, potentially triggering a Machine Check Exception (MCE) when
accessed?

> +		if (!child)
> +			return -ENOMEM;
> +
> +		/* Overwrite PA with VA in-place */
> +		node->table[i] = (u64)(uintptr_t)child;

[Severity: High]
Because memremap() creates a direct mapping to the underlying physical memory,
does assigning the virtual address here destructively modify the old kernel's
physical memory? This seems like it would corrupt the crash dump, causing
offline analysis tools like crash or makedumpfile to encounter meaningless
virtual addresses instead of the original physical addresses.

> +
> +		/* Recurse for intermediate levels; level 1 children are leaves */
> +		if (level > 1) {
> +			err = kho_radix_crash_convert_node(child, level - 1);
> +			if (err)
> +				return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * kho_radix_crash_init - Initialize a crash-kernel view of a KHO radix tree.
[ ... ]
> + */
> +int kho_radix_crash_init(struct kho_radix_crash_tree *tree, phys_addr_t root_pa)
> +{
> +	struct kho_radix_node *root;
> +	int err;
> +
> +	tree->root = NULL;
> +
> +	if (!root_pa || (root_pa & (PAGE_SIZE - 1)))
> +		return -EINVAL;
> +
> +	root = memremap(root_pa, PAGE_SIZE, MEMREMAP_WB);
> +	if (!root)
> +		return -ENOMEM;
> +
> +	err = kho_radix_crash_convert_node(root, KHO_TREE_MAX_DEPTH - 1);
> +	if (err)
> +		return err;
> +
> +	tree->root = root;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(kho_radix_crash_init);

[Severity: Medium]
This API is exported to modules and allocates mappings via memremap(), but
there is no corresponding kho_radix_crash_destroy() provided. Even if the
crash kernel is short-lived, should kernel APIs exported to modules enforce
symmetrical resource lifecycle management to avoid permanent vmap memory
leaks if a module is unloaded or fails initialization?

> +
> +/**
> + * kho_radix_crash_contains_page - Check if a page is in a crash-kernel radix tree.
[ ... ]
> + * Note: This function checks specifically for the presence of the page at the
> + * given order. If a larger order page that encompasses this page is preserved,
> + * this function will return false.
> + *
> + * Return: true if the page is present in the tree, false otherwise.
> + */
> +bool kho_radix_crash_contains_page(struct kho_radix_crash_tree *tree,
> +				   unsigned long pfn, unsigned int order)
> +{
> +	unsigned long key = kho_encode_radix_key(PFN_PHYS(pfn), order);

[Severity: Medium]
Since this searches for an exact match of the provided pfn and order, doesn't
this fail to identify subpages of higher-order folios? If a page was
preserved as part of a higher-order folio, an order-0 query for a PFN within
that folio will fail to find the key and incorrectly return false. 

Would callers like pfn_is_ram() be forced to manually iterate through all
possible orders up to MAX_PAGE_ORDER to reliably determine if a page is
preserved?

> +	struct kho_radix_node *node = tree->root;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528004204.1484584-1-jloeser@linux.microsoft.com?part=14