Re: [PATCH v3] kpartx: fix crash and truncated device creation with long -p delimiter

On Thu, 2026-07-16 at 16:20 +0800, [email protected] wrote:
> From: Kou Wenqi <[email protected]>
> 
> When the -p delimiter is long enough to make the formatted partition
> name exceed PARTNAME_SIZE (128 bytes), three issues occur:
> 
> 1. format_partname() fails but snprintf has already written a
> truncated
>    name into the buffer. dm_find_part() returns 0 and the caller
>    proceeds to dm_addmap() with the truncated name, creating a device
>    that was never intended.
> 
> 2. dm_find_part() returns early without setting *part_uuid. The
>    uninitialized local variable part_uuid then gets passed to
>    check_uuid() -> strchr(), causing a SIGSEGV.
> 
> 3. The callers cannot distinguish between "partition not found,
> create
>    new" and "name construction failed" since both return 0.
> 
> Fix by:
> - Having dm_find_part() return -1 when format_partname() fails, with
>   an error message printed by dm_find_part() itself, so that callers
>   can test the return value directly (negative for error,
>   0 for not found, positive for found)
> - In the ADD/UPDATE loops (both main and container), checking the
>   return value: negative means name too long (skip with error count),
>   0 means not found (create), positive means found (reload)
> - In the DELETE loop, only proceeding with removal when
> dm_find_part()
>   returns positive (partition found), skipping when it returns zero
>   (not found) or negative (error)
> - Initializing part_uuid to NULL in all three partition loop bodies
>   (ADD/UPDATE main loop, container partition loop, DELETE loop) so
>   that the "if (part_uuid && uuid)" guard correctly skips the UUID
>   check when dm_find_part() returns early
> 
> Reproduce steps:
> 
>   # Create test image
>   dd if=/dev/zero of=/tmp/vhlg-test.img bs=1M count=10
>   parted /tmp/vhlg-test.img mklabel msdos
>   parted /tmp/vhlg-test.img mkpart primary ext4 1MiB 5MiB
> 
>   # Reproduce
>   kpartx -a -p $(python3 -c "print('A'*200)") /tmp/vhlg-test.img
> 
>   # Cleanup
>   kpartx -d /tmp/vhlg-test.img
>   rm -f /tmp/vhlg-test.img
> 
> Signed-off-by: Kou Wenqi <[email protected]>
> ---
>  kpartx/devmapper.c |  5 ++---
>  kpartx/kpartx.c    | 46 ++++++++++++++++++++++++++++++--------------
> --
>  2 files changed, 32 insertions(+), 19 deletions(-)
> 

Thanks again. This looks quite good. However, what about this?

enum {
	DFP_DEVICE_CREATE = DM_DEVICE_CREATE,
	DFP_DEVICE_RELOAD = DM_DEVICE_RELOAD,
	DFP_ERR = -1,
};

Then use values of this enum as return values of dm_find_part().
If it returns DFP_ERR, bail out; otherwise use the return value as
"op", like this:

        if ((op = dm_find_part(...)) == DFP_ERR)
                continue;

If you want to be extra careful, you could add something like
this at the beginning of dm_find_part():

BUILD_BUG_ON(DM_DEVICE_CREATE < 0 || DM_DEVICE_RELOAD < 0);

But I think we can trust that these values will always be positive.

Martin