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 <[email protected]> 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/[email protected]?part=18
