Re: [PATCH v7 07/10] dax/kmem: resolve default online type at probe time

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Memory leak during device removal if any DAX ranges are too small after alignment.
--

--- Patch [7]: [PATCH v7 07/10] dax/kmem: resolve default online type at probe time ---
commit 4d3b2bc139f20f3d91088883d6dbe39863e79701
Author: Gregory Price <[email protected]>

dax/kmem: resolve default online type at probe time
    
Resolve the default online type in kmem at probe time explicitly
instead of implicitly inheriting the system default by calling
add_memory_driver_managed().

> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
> index 592171ec10f49..38ed5c4e9c83d 100644
> --- a/drivers/dax/kmem.c
> +++ b/drivers/dax/kmem.c

[ ... ]

While reviewing this, I noticed a potential resource leak in another part of
the file:

> 		rc = dax_kmem_range(dev_dax, i, &range);
> 		if (rc)
> 			continue;
> 
> 		/* range was never added during probe */
> 		if (!data->res[i]) {
> 			success++;
> 			continue;
> 		}

[Severity: High]
This is a pre-existing issue, but does this logic skip the success counter
increment when a memory range is too small?

If dax_kmem_range() returns -ENOSPC during probe for a small range, the
same check here in dev_dax_kmem_remove() will execute continue early.

This bypasses the success++ operation, which means the success counter
will remain strictly less than dev_dax->nr_range. 

> 	if (success >= dev_dax->nr_range) {
> 		memory_group_unregister(data->mgid);
> 		kfree(data->res_name);
> 		kfree(data);
> 		dev_set_drvdata(dev, NULL);
> 		/* ... */
> 		clear_node_memory_type(node, NULL);
> 	}

Could this bypass the cleanup block entirely, permanently leaking the memory
group and driver data structures every time a device with unaligned or small
memory ranges is unbound?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7