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