[PATCH 03/11] cmd: ubi: unbind the block device when detaching UBI

[Simon Glass]

Hi Daniel,

On 2026-09-29T00:04:16, Daniel Golle <daniel at makrotopia.org> wrote:
> cmd: ubi: unbind the block device when detaching UBI
>
> The ubi_blk device is parented to the MTD device UBI was attached to,
> but ubi_detach() leaves it bound. After 'ubi part A; ubi detach; ubi
> part B' the device still hangs off A's MTD while UBI sits on B, so the
> driver-model tree describes a parent that is no longer in use and A's
> MTD can no longer be removed. Reads happen to keep working only because
> get_ubi_device() always returns ubi_devices[0].
>
> Return the new device from ubi_bind(), remember it, and unbind it in
> ubi_detach() so the next attach binds a fresh one under the right
> parent. Tracking the device also replaces the scan of every UCLASS_BLK
> device that was used to find it.
>
> Fixes: dec405d1653a ("cmd: ubi: create a ubi_blk device when attaching UBI")
> Signed-off-by: Daniel Golle <daniel at makrotopia.org>
>
> cmd/ubi.c               | 73 +++++++++++++++++++++++++++++--------------------
>  drivers/mtd/ubi/block.c |  4 ++-
>  include/ubi_uboot.h     |  4 +--
>  3 files changed, 49 insertions(+), 32 deletions(-)

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -650,6 +651,47 @@ static int ubi_set_skip_check(const char *volume, bool skip_check)
> +#if CONFIG_IS_ENABLED(UBI_BLOCK)
> +static struct udevice *ubi_blk_dev;
> +
> +static void ubi_blk_bind_once(void)
> +{
> +     struct udevice *parent;
> +     int ret;
> +
> +     /*
> +      * A single ubi_blk device serves all volumes of the attached UBI
> +      * device, the volume being selected through the block descriptor's
> +      * hwpart. It is parented to the MTD device UBI sits on, so it must
> +      * not outlive the attach that created it.
> +      */
> +     if (ubi_blk_dev)
> +             return;

Driver model can free this device without cmd/ubi.c knowing. If
anything unbinds the MTD parent (e.g. 'unbind' on the flash device),
DM unbinds the ubi_blk child too, leaving ubi_blk_dev pointing at
freed memory. The next ubi_detach() passes it to device_remove(), and
the next ubi_part() returns early and never binds a new device.

Patch 11 makes this easier to hit. With UTF_DM the DM tree is torn
down after each test, so if imagemap_test_ubiblock() fails an
assertion between ubi_part() and the final ubi_detach(), the next
run's initial ubi_detach() uses the stale pointer. The UCLASS_BLK scan
you are removing does not have this problem, since it can only find
devices that still exist.

Please can you keep the scan, and use it in ubi_blk_unbind() too to
find the device to unbind? That still fixes the wrong-parent problem
without global state. What do you think?

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -650,6 +651,47 @@ static int ubi_set_skip_check(const char *volume, bool skip_check)
> +     device_remove(ubi_blk_dev, DM_REMOVE_NORMAL);
> +     device_unbind(ubi_blk_dev);
> +     ubi_blk_dev = NULL;

Both return values are ignored. If device_remove() fails,
device_unbind() returns -EINVAL for an activated device, and the
pointer is cleared anyway, so the device stays bound under the old
parent, which is the case this patch is trying to fix. Please at least
log the error.

Regards,
Simon