[RFC PATCH] drivers: nvdimm: validate dataoffset

[RFC PATCH] drivers: nvdimm: validate dataoffset

[Shyam Saini]

On some platforms cold reboot may not zero out DRAM and
as a consequence, the next kernel boot with a new pmem layout
change may end up using stale superblocks. This can cause
altmap to run out of memory and consequently the pmem drivers
may fail to probe.

    failed to free all reserved pages
   [trimmed]
    Call trace:
     memunmap_pages+0x2f8/0x358
     memremap_pages+0x590/0x728
     devm_memremap_pages+0x30/0x88
     pmem_attach_disk+0x178/0x488
     nd_pmem_probe+0x60/0xe0
     nvdimm_bus_probe+0xec/0x208
     really_probe+0xc0/0x2e8
     __driver_probe_device+0xc4/0x168
     driver_probe_device+0x48/0x218
     __device_attach_driver+0x130/0x160
     bus_for_each_drv+0x94/0xf0
     __device_attach+0xfc/0x1a0
     device_initial_probe+0x18/0x28
     bus_probe_device+0x40/0xa0
     device_add+0x270/0x498
     nd_async_device_register+0x20/0x70
     async_run_entry_fn+0x30/0x158
     process_scheduled_works+0x248/0x4b0
     worker_thread+0x284/0x3a0
     kthread+0xe0/0x180
     ret_from_fork+0x10/0x20
    ---[ end trace 0000000000000000 ]---
    nd_pmem: probe of pfnX.1 failed with error -12

Fix this by discarding superblock if there is a mismatch b/w
dataoffset value from the preserved superblock and the
dataoffset from the resource.

While this fix is not robust but it can be combined with userspace
namespace destruction and recreation to remove all the stale pmems
metadata and reconfigure the fresh data using ndctl

Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
---
 drivers/nvdimm/pfn_devs.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

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
@@ -439,12 +439,12 @@ static bool nd_supported_alignment(unsigned long align)
  */
 int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
 {
-	u64 checksum, offset;
+	u64 checksum, offset, offset_res;
 	struct resource *res;
 	enum nd_pfn_mode mode;
-	resource_size_t res_size;
+	resource_size_t res_start, res_size;
 	struct nd_namespace_io *nsio;
-	unsigned long align, start_pad, end_trunc;
+	unsigned long align, start_pad, end_trunc, npfns;
 	struct nd_pfn_sb *pfn_sb = nd_pfn->pfn_sb;
 	struct nd_namespace_common *ndns = nd_pfn->ndns;
 	const uuid_t *parent_uuid = nd_dev_to_uuid(&ndns->dev);
@@ -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;
+
 	if (offset >= res_size) {
 		dev_err(&nd_pfn->dev, "pfn array size exceeds capacity of %s\n",
 				dev_name(&ndns->dev));
-- 
2.43.0

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

[sashiko-bot]

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