Re: [PATCH v5 8/9] dax/kmem: add sysfs interface for atomic whole-device hotplug

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The protection against deadlocks during driver unbind can be completely bypassed if the underlying memory blocks are manually onlined via the core memory sysfs, causing state desynchronization.
- [Medium] any_hotremove_failed is irrevocably set on recoverable sysfs hotremove failures, leading to a permanent leak of kmem_name.
--

commit fa89899811083ada324ea3db8204b1c135005af5
Author: Gregory Price <gourry@gourry.net>
Subject: dax/kmem: add sysfs interface for atomic whole-device hotplug

This patch adds a new 'state' sysfs attribute for dax_kmem devices to enable
atomic whole-device hotplug operations. It also updates the unbind logic to
prevent deadlocks by omitting memory removal if the memory blocks are
currently online.

> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
> index a45e50def5371..340486586d82c 100644
> --- a/drivers/dax/kmem.c
> +++ b/drivers/dax/kmem.c

[ ... ]

> +static int dax_kmem_do_hotremove(struct dev_dax *dev_dax,
> +				 struct dax_kmem_data *data)
> +{

[ ... ]

> +	rc = offline_and_remove_memory_ranges(ranges, nr_ranges);
> +	kfree(ranges);
> +	if (rc) {
> +		any_hotremove_failed = true;
> +		dev_err(dev, "hotremove failed, device left online: %d\n", rc);
> +		return rc;
> +	}

[Severity: Medium]
Does this permanently leak kmem_name on recoverable sysfs failures?

If a user attempts to unplug a device via sysfs and the hotremove fails
transiently (e.g., due to memory being in use), any_hotremove_failed is set
to true. Even if the user retries and the unplug later succeeds, this global
flag is never cleared, which will prevent freeing kmem_name during module
unload.

[ ... ]

> @@ -348,20 +490,26 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax)
>  #ifdef CONFIG_MEMORY_HOTREMOVE
>  static void dev_dax_kmem_remove(struct dev_dax *dev_dax)
>  {

[ ... ]

> +	if (dax_kmem_state_is_online(data->state)) {
> +		dev_warn(dev, "Hotplug regions stuck online until reboot\n");
> +		any_hotremove_failed = true;
> +		return;
> +	} else if (data->state == MMOP_OFFLINE &&
> +	    dax_kmem_do_hotremove(dev_dax, data)) {
> +		dev_warn(dev, "Unplug failed, resources leaked until reboot\n");
>  		return;
>  	}

[Severity: High]
Can this deadlock during unbind if the memory state is desynchronized?

If the device is configured with auto_online_blocks=offline, data->state is
initially MMOP_OFFLINE. If a user manually onlines the underlying memory
blocks via the generic /sys/devices/system/memory/memoryX/state sysfs
interface, data->state remains MMOP_OFFLINE.

During unbind, dev_dax_kmem_remove() will evaluate
dax_kmem_state_is_online(data->state) as false and proceed to call
dax_kmem_do_hotremove() instead of safely aborting. This attempts to offline
the blocks while holding the device unbind lock, bypassing the intended
deadlock protection.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624145744.3532049-1-gourry@gourry.net?part=8