Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] dax_kmem_do_hotplug() unconditionally attempts to hotplug memory for all ranges without verifying successful resource reservation, leading to a permanent memory leak and potential use-after-free. --
commit 5d9cdda5cefdb148a54897a713788ec98f55e42a Author: Gregory Price <[email protected]> dax/kmem: extract hotplug/hotremove helper functions Refactor kmem _probe() _remove() by extracting init, cleanup, hotplug, and hot-remove logic into separate helper functions: > diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c > index 0a184c0878ddf..a45e50def5371 100644 > --- a/drivers/dax/kmem.c > +++ b/drivers/dax/kmem.c [ ... ] > @@ -63,14 +63,195 @@ static void kmem_put_memory_types(void) > mt_put_memory_types(&kmem_memory_types); > } > > +/** > + * dax_kmem_do_hotplug - hotplug memory for dax kmem device > + * @dev_dax: the dev_dax instance > + * @data: the dax_kmem_data structure with resource tracking > + * > + * Hotplugs all ranges in the dev_dax region as system memory. > + * > + * Returns the number of successfully mapped ranges, or negative error. > + */ > +static int dax_kmem_do_hotplug(struct dev_dax *dev_dax, > + struct dax_kmem_data *data, > + int online_type) > +{ > + struct device *dev = &dev_dax->dev; > + int i, rc, onlined = 0; > + mhp_t mhp_flags; > + > + for (i = 0; i < dev_dax->nr_range; i++) { > + struct range range; > + > + rc = dax_kmem_range(dev_dax, i, &range); > + if (rc) > + continue; > + > + mhp_flags = MHP_NID_IS_MGID; > + if (dev_dax->memmap_on_memory) > + mhp_flags |= MHP_MEMMAP_ON_MEMORY; > + > + /* > + * Ensure that future kexec'd kernels will not treat > + * this as RAM automatically. > + */ > + rc = __add_memory_driver_managed(data->mgid, range.start, > + range_len(&range), kmem_name, mhp_flags, > + online_type); [Severity: High] Does this unconditionally add memory even for ranges that failed to reserve resources? If dax_kmem_init_resources() succeeds on the first range but fails on a subsequent range, it leaves data->res[i] = NULL and continues because mapped > 0. Then dax_kmem_do_hotplug() iterates over all ranges again and unconditionally calls __add_memory_driver_managed() for the range that failed reservation. If memory addition succeeds, the memory is brought online without an I/O resource reservation. Later, during device removal, dax_kmem_do_hotremove() skips ranges where !data->res[i], which permanently leaks the online memory. When the backing device is physically removed, any access to this memory could cause a crash. Should there be an if (!data->res[i]) continue; check here to ensure we only hotplug memory for successfully reserved regions? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
