[PATCH v2 5/8] cmd: ubi: reorganize command messages

[Simon Glass]

Hi Weijie,

On 2026-04-21T08:35:39, Weijie Gao <weijie.gao at mediatek.com> wrote:
> cmd: ubi: reorganize command messages
>
> This patch moves normal subcommand messages into the main command function.
> This will allow current and potential api functions being called with clean
> output.
>
> A new function ubi_require_volume() is added for finding and printing error
> message if volume not found. The original ubi_find_volume() will be silent
> for being an api function.
>
> Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
>
> cmd/ubi.c | 92 ++++++++++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 65 insertions(+), 27 deletions(-)

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -514,7 +518,7 @@ int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
> -     vol = ubi_find_volume(volume);
> +     vol = ubi_require_volume(volume);

Won't this print messages, e.g. ubi_volume_read() is also an API function?

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -402,7 +406,7 @@ int ubi_volume_begin_write(const char *volume, const void *buf, size_t size,
> -     vol = ubi_find_volume(volume);
> +     vol = ubi_require_volume(volume);

Same issue: ubi_volume_begin_write() is an exported API function.

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -806,31 +806,63 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> +             if (argc == 3) {
> +                     struct ubi_volume *vol;
> +
> +                     vol = ubi_find_volume(argv[2]);
> +                     if (!vol)
> +                             return 0;
> +
> +                     ret = ubi_remove_vol(argv[2]);
> +                     if (!ret) {
> +                             printf("Removed UBI volume %s (id %d)\n",
> +                                    vol->name, vol->vol_id);
> +                     }

Use-after-free: ubi_remove_vol() frees the volume structure, then you
access vol->name and vol->vol_id  - please save the vol_id before
calling ubi_remove_vol(), or print the volume name from argv[2].

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -877,7 +909,13 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> +                     ret = ubi_volume_read(argv[3], (void *)addr, 0, size);
> +                     if (!ret) {
> +                             printf("%lld bytes read from volume %s to 0x%lx\n",
> +                                    size, argv[3], addr);
> +                     }

When size is 0, ubi_volume_read() uses vol->used_bytes internally, but
the success message prints 0. The old message was printed inside
ubi_volume_read() after size was adjusted, so it reported the true
value.

Regards,
Simon