Re: [PATCH v5 7/9] dax/kmem: extract hotplug/hotremove helper functions

[sashiko-bot@kernel.org]

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 <gourry@gourry.net>

    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/20260624145744.3532049-1-gourry@gourry.net?part=7