Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The `acpi_nfit_init` function fails to restore list elements on error paths, leading to memory leaks and complete state corruption for existing NVDIMM devices during hot-updates. -- --- Patch [5]: [PATCH v2 5/5] ACPI: NFIT: bound the platform capability mask --- commit bbbfd46d3c5bf4ec32e49479ec84620dc0f64973 Author: Pengpeng Hou <[email protected]> ACPI: NFIT: bound the platform capability mask The platform capability subtable parser reads highest_capability and capabilities without first requiring the fixed structure to be present. This commit adds checks to prevent out-of-bounds reads and undefined shifts. > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > index 84c70f1941c57..7638d4dc8c6da 100644 > --- a/drivers/acpi/nfit/core.c > +++ b/drivers/acpi/nfit/core.c > @@ -987,7 +987,14 @@ static bool add_platform_cap(struct acpi_nfit_desc > *acpi_desc, > struct device *dev = acpi_desc->dev; > u32 mask; > > - mask = (1 << (pcap->highest_capability + 1)) - 1; > + if (pcap->header.length < sizeof(*pcap)) > + return false; [Severity: High] This is a pre-existing issue, but does adding this new early return expose a state corruption bug during hot-updates? In acpi_nfit_init(), existing NFIT resources are moved to a local stack variable before parsing: acpi_nfit_init() { ... list_cut_position(&prev.spas, &acpi_desc->spas, acpi_desc->spas.prev); ... If the newly added capability length check fails, it appears the table parsing aborts: if (IS_ERR(data)) { dev_dbg(dev, "nfit table parsing error: %ld\n", PTR_ERR(data)); rc = PTR_ERR(data); goto out_unlock; } Which jumps directly to the exit label: out_unlock: mutex_unlock(&acpi_desc->init_mutex); return rc; } Since the elements in the local prev lists are never restored to acpi_desc on this error path, do these devm-managed items become permanently leaked with dangling list pointers when prev goes out of scope? > + > + if (pcap->highest_capability >= 31) > + mask = U32_MAX; > + else > + mask = (1U << (pcap->highest_capability + 1)) - 1; > + > acpi_desc->platform_cap = pcap->capabilities & mask; > dev_dbg(dev, "cap: %#x\n", acpi_desc->platform_cap); > return true; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
