Re: [RFC PATCH] drivers: nvdimm: validate dataoffset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] nvdimm: false rejection of valid PMEM namespaces due to hardcoded MAX_STRUCT_PAGE_SIZE
- [High] nvdimm: offset validation ignores memremap_compat_align()
--

commit eb5279626b7f080ae471e489ece4609d9d792c1b
Author: Shyam Saini <shyamsaini@linux.microsoft.com>

drivers: nvdimm: validate dataoffset

This commit adds a dataoffset validation to prevent probing failures caused
by stale superblocks after a cold reboot where DRAM is not zeroed. It checks
for a mismatch between the preserved superblock dataoffset and the resource's
dataoffset, rejecting mismatched ones.

> diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
> index 8fa9c16aba7ea..667115ce88e40 100644
> --- a/drivers/nvdimm/pfn_devs.c
> +++ b/drivers/nvdimm/pfn_devs.c
[ ... ]
> @@ -577,6 +577,12 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
>  	nsio = to_nd_namespace_io(&ndns->dev);
>  	res = &nsio->res;
>  	res_size = resource_size(res);
> +	res_start = res->start;
> +	npfns = PHYS_PFN(res_size - SZ_8K);
> +	offset_res = ALIGN(res_start + SZ_8K + (MAX_STRUCT_PAGE_SIZE * npfns), align) - res_start;
> +	if ((mode == PFN_MODE_PMEM) && offset_res != offset)
> +		return -ENODEV;
> +

[Severity: High]
Does this code falsely reject valid PMEM namespaces that were created with a
custom struct page size or on architectures with a larger alignment?

The offset_res calculation hardcodes MAX_STRUCT_PAGE_SIZE instead of using
the superblock's recorded page_struct_size.

If the namespace was initialized with a larger struct page size (for example,
with KMSAN where sizeof(struct page) > 64), offset_res will be calculated as
smaller than the actual stored offset.

Additionally, the original offset calculation during namespace creation in
nd_pfn_init() uses max(nd_pfn->align, memremap_compat_align()):

drivers/nvdimm/pfn_devs.c:nd_pfn_init() {
    ...
    align = max(nd_pfn->align, memremap_compat_align());
    ...
}

Because the recalculation here uses only 'align' without considering
memremap_compat_align(), could it cause a mismatch on architectures where
memremap_compat_align() is larger than nd_pfn->align (like PowerPC with 64K
pages, where memremap_compat_align() is 16MB)?

Will this mismatch check incorrectly evaluate to true for these valid PMEM
namespaces and fail to probe them?

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