Re: [RFC PATCH 18/20] mshv: Exclude Hyper-V donated pages from crash dump collection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Radix tree key encoding mismatch prevents exclusion of Hyper-V owned pages during crash dump.
- [High] Failure to clear the Hyper-V crash MSR P2 when KHO is disabled or fails to initialize leaves a stale physical address, leading to memory corruption during crash dump collection.
- [Medium] The kdump initialization path calls a KHO API without being gated by `kho_is_enabled()`, violating strict KHO subsystem guidelines.
--

commit a672e76ca918e68a4de01d571d5f2e72810eec75
Author: Jork Loeser <jloeser@linux.microsoft.com>

mshv: Exclude Hyper-V donated pages from crash dump collection

Pages donated to Hyper-V must not be read during crash dump collection.
They are not ordinary RAM and accessing them can hang or corrupt the
crash kernel.

> diff --git a/drivers/hv/mshv_page_preserve.c b/drivers/hv/mshv_page_preserve.c
> index bc3a3a688f5b2..e16fb946790d5 100644
> --- a/drivers/hv/mshv_page_preserve.c
> +++ b/drivers/hv/mshv_page_preserve.c

[ ... ]

> @@ -327,6 +328,57 @@ static int __init alloc_tree(void)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_CRASH_DUMP
> +static struct kho_radix_crash_tree crash_preserved_pages_tree;
> +
> +/**
> + * restore_crash_tree() - Set up the crash tree for dump-time page exclusion.
> + *
> + * In the crash kernel, the old kernel's memory is not in the direct map.
> + * The old kernel stashes the radix tree root PA in Hyper-V crash MSR P2
> + * so we can retrieve it without touching the old kernel's FDT.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +static int __init restore_crash_tree(void)
> +{
> +	phys_addr_t root_pa;
> +
> +	root_pa = hv_get_msr(HV_MSR_CRASH_P2);
> +	if (!root_pa)
> +		return -ENOENT;
> +
> +	/*
> +	 * The MSR may contain stale data from a previous
> +	 * hyperv_report_panic().  Sanity-check that it looks like a
> +	 * page-aligned physical address within the architectural limit.
> +	 */
> +	if (!PAGE_ALIGNED(root_pa) || root_pa >> MAX_POSSIBLE_PHYSMEM_BITS) {
> +		pr_warn("Invalid crash tree root PA: 0x%llx\n",
> +			(unsigned long long)root_pa);
> +		return -EINVAL;
> +	}
> +
> +	return kho_radix_crash_init(&crash_preserved_pages_tree, root_pa);
> +}
> +
> +static bool mshv_vmcore_pfn_is_ram(struct vmcore_cb *cb, unsigned long pfn)
> +{
> +	/*
> +	 * MSHV-owned pages must not be read during crash dump collection.
> +	 * Currently all pages are registered at order 0. If higher-order
> +	 * registrations are added, this lookup will need to handle them
> +	 * (e.g. by querying multiple orders or using a range-based API).
> +	 */
> +	return !kho_radix_crash_contains_page(&crash_preserved_pages_tree,
> +					      pfn, 0);

[Severity: High]
Is the key inserted into the radix tree matched correctly here?

If pages are inserted into the tree using the raw page PFN (e.g.,
page_to_pfn()), this lookup might always miss. The
kho_radix_crash_contains_page() function internally encodes the query key
using kho_encode_radix_key(PFN_PHYS(pfn), order), which adds high-order
metadata bits.

Will this cause the crash kernel to fail to exclude Hyper-V donated pages,
treating them as normal RAM?

> +}
> +
> +static struct vmcore_cb mshv_vmcore_cb = {
> +	.pfn_is_ram = mshv_vmcore_pfn_is_ram,
> +};
> +#endif
> +
>  static struct notifier_block reboot_notifier = {
>  	.notifier_call = reboot_cb,
>  	.priority = 0,
> @@ -347,6 +399,24 @@ int __init mshv_preserve_init(void)
>  {
>  	int err;
>  
> +#ifdef CONFIG_CRASH_DUMP
> +	if (is_kdump_kernel()) {
> +		/*
> +		 * Crash kernel only needs the pfn_is_ram callback to exclude
> +		 * MSHV-owned pages from the dump.  No page restoration, no
> +		 * reboot notifier — the crash kernel reboots after collection.
> +		 */
> +		err = restore_crash_tree();
> +		if (err) {
> +			pr_err("Could not set up crash page tree: %d; MSHV pages may appear in dump\n",
> +			       err);
> +			return 0;
> +		}
> +		register_vmcore_cb(&mshv_vmcore_cb);
> +		return 0;
> +	}
> +#endif
> +

[Severity: Medium]
Should this path be gated by kho_is_enabled() as well?

The KHO subsystem documentation mandates checking kho_is_enabled() at the
entry point of any code path that uses KHO APIs. Here, restore_crash_tree()
(which calls the KHO API kho_radix_crash_init()) is invoked before checking
if KHO is enabled.

>  	if (!kho_is_enabled()) {
>  		pr_err("KHO is disabled; page deposits will fail.\n");
>  		return 0;

[Severity: High]
If the system kexecs into a new kernel where KHO is disabled (or initialization
fails), this function returns early without clearing the HV_MSR_CRASH_P2 MSR.

Since Hyper-V crash MSRs persist across kexecs, could the new kernel leave a
stale physical address in P2?

If this new kernel panics, the kdump kernel will read the stale MSR. Because
the address is page-aligned and within bounds, it passes sanity checks in
restore_crash_tree(), and kho_radix_crash_init() will recursively map and
parse the arbitrary memory as a radix tree.

By executing node->table[i] = (u64)(uintptr_t)child, might this actively write
virtual addresses into the crashed kernel's physical memory, corrupting the
crash dump or triggering panics in the kdump kernel due to mapping invalid
addresses?

>  	}
>  
>  	err = restore_tree();

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