[PATCH] dm: core: Unlink the device when bind fails without DM_DEVICE_REMOVE

[PATCH] dm: core: Unlink the device when bind fails without DM_DEVICE_REMOVE

[Yuho Choi]

device_bind_common() adds the new device to its parent's list of
children and to its uclass before it calls the bind methods. If one of
them fails, the error path takes the device off both lists again, but
only when DM_DEVICE_REMOVE is enabled. The free(dev) at the end of the
error path is unconditional, so without DM_DEVICE_REMOVE the freed
device stays on both lists.

DM_DEVICE_REMOVE is disabled by default in SPL, and a failing bind is
not unusual there: lists_bind_fdt() takes -ENODEV from a bind method as
the driver refusing the node and goes on with the next driver. For
example cadence_ttc_bind() refuses TTC nodes that have a #pwm-cells
property so that the PWM driver binds them instead. Any later walk of
the parent's children or of the uclass then finds a device that was
freed. With the simple malloc() the memory is not reused, but the
refused device can still be found and probed through its uclass; with
a full malloc() the memory is handed out again.

Taking the device off the two lists only needs two list_del() calls, so
do it regardless of DM_DEVICE_REMOVE. The remaining clean-up, which only
frees memory, stays conditional as before.

Fixes: 5a87c4174d18 ("dm: core: Drop device removal error path when not supported")
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
 drivers/core/device.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/core/device.c b/drivers/core/device.c
index 6024534ff93..ea7beeb662d 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -206,10 +206,13 @@ fail_bind:
 			dm_warn("Failed to unbind dev '%s' on error path\n",
 				dev->name);
 		}
+	} else {
+		/* dev is freed below, so it must not stay in the uclass */
+		list_del(&dev->uclass_node);
 	}
 fail_uclass_bind:
+	list_del(&dev->sibling_node);
 	if (CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)) {
-		list_del(&dev->sibling_node);
 		if (dev_get_flags(dev) & DM_FLAG_ALLOC_PARENT_PDATA) {
 			free(dev_get_parent_plat(dev));
 			dev_set_parent_plat(dev, NULL);
-- 
2.43.0

Re: [PATCH] dm: core: Unlink the device when bind fails without DM_DEVICE_REMOVE

[Simon Glass]

Hi Yuho,

On 2026-10-26T03:09:22, Yuho Choi <oss.patchbox@gmail.com> wrote:
> dm: core: Unlink the device when bind fails without DM_DEVICE_REMOVE
>
> device_bind_common() adds the new device to its parent's list of
> children and to its uclass before it calls the bind methods. If one of
> them fails, the error path takes the device off both lists again, but
> only when DM_DEVICE_REMOVE is enabled. The free(dev) at the end of the
> error path is unconditional, so without DM_DEVICE_REMOVE the freed
> device stays on both lists.
>
> DM_DEVICE_REMOVE is disabled by default in SPL, and a failing bind is
> not unusual there: lists_bind_fdt() takes -ENODEV from a bind method as
> the driver refusing the node and goes on with the next driver. For
> example cadence_ttc_bind() refuses TTC nodes that have a #pwm-cells
> property so that the PWM driver binds them instead. Any later walk of
> the parent's children or of the uclass then finds a device that was
> freed. With the simple malloc() the memory is not reused, but the
> refused device can still be found and probed through its uclass; with
> a full malloc() the memory is handed out again.
>
> Taking the device off the two lists only needs two list_del() calls, so
> [...]
>
>  drivers/core/device.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

> diff --git a/drivers/core/device.c b/drivers/core/device.c
> @@ -206,10 +206,13 @@ fail_bind:
>  			dm_warn("Failed to unbind dev '%s' on error path\n",
>  				dev->name);
>  		}
> +	} else {
> +		/* dev is freed below, so it must not stay in the uclass */
> +		list_del(&dev->uclass_node);
>  	}

uclass_unbind_device() only does list_del(&dev->uclass_node), so both
branches now do the same thing. Please can you move
uclass_unbind_device() out of the #if in uclass.c (and drop the inline
stub in uclass-internal.h which returns 0 without doing anything),
then call it here unconditionally? That avoids the else and keeps the
uclass list handling in uclass.c. What do you think?

Regards,
Simon

Re: [PATCH] dm: core: Unlink the device when bind fails without DM_DEVICE_REMOVE

[Yuho Choi]

On Tue, 27 Oct 2026 at 09:28, Simon Glass <sjg@chromium.org> wrote:
>
> uclass_unbind_device() only does list_del(&dev->uclass_node), so both
> branches now do the same thing. Please can you move
> uclass_unbind_device() out of the #if in uclass.c (and drop the inline
> stub in uclass-internal.h which returns 0 without doing anything),
> then call it here unconditionally? That avoids the else and keeps the
> uclass list handling in uclass.c. What do you think?

Hi Simon,

Thanks for the suggestion. That makes sense. I'll move
uclass_unbind_device() out of the conditional compilation and call it
unconditionally, while keeping the sibling_node cleanup as is.

I'll send a v2 with these changes.

Best regards,
Yuho