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
