Re: [PATCH v1 11/13] mshv: unlock SNP pages on panic for crashdump collection

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lockless iteration of a non-RCU list in a panic notifier exposes the system to poison pointer dereferences, causing a nested panic and preventing crashdump collection.
--

--- Patch [11]: [PATCH v1 11/13] mshv: unlock SNP pages on panic for crashdump collection ---
commit da7cefab011e489222d854cd80ab1f7f2ab3f297
Author: Wei Hu <[email protected]>

mshv: unlock SNP pages on panic for crashdump collection

This commit adds a panic notifier to unmap and share SEV-SNP guest pages
back to the host, allowing kexec to collect a valid root vmcore after a
crash. The notifier walks the partition table and unlocks memory regions
if the hypervisor's own crashdump support is inactive.

> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index 0fbd2158968da..4d08d547704e4 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
[ ... ]
> @@ -2898,6 +2898,74 @@ static int __init mshv_init_vmm_caps(struct device *dev)
>  	return 0;
>  }
>  
> +#if defined(CONFIG_X86_64) && IS_ENABLED(CONFIG_CRASH_DUMP)
> +static void mshv_panic_unlock_snp(struct mshv_partition *vm)
> +{
> +	struct mshv_mem_region *memreg;
> +	int ret;
> +
> +	hlist_for_each_entry(memreg, &vm->pt_mem_regions, hnode) {

[Severity: High]
Is it safe to iterate pt_mem_regions locklessly here?

Since panic notifiers can run at any time, they might interrupt a concurrent
hlist_del() operation on this list. If that happens, the list removal poisons
the node pointers. This hlist_for_each_entry() traversal would then
dereference a poisoned pointer, causing a recursive panic.

A recursive panic would instantly halt the machine and abort the kexec
crashdump process, defeating the purpose of this fallback path.

Does this list need to be converted to use RCU to safely traverse it during
a panic, or is there another way to safely identify these regions without
risking a page fault?

> +		mshv_region_unmap(memreg);
> +		ret = mshv_region_share(memreg);
> +		if (ret)
> +			pt_err(vm, "Unlock snp failed. ret:0x%x gfn:%llx numpfns:%lld\n",
> +			       ret, memreg->start_gfn, memreg->nr_pages);
> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11