On Fri, 2026-07-17 at 11:34 +0800, [email protected] wrote: > From: Kou Wenqi <[email protected]> > > On Thu, 2026-07-16 at 15:49 +0200, [email protected] wrote: > > 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, > > }; > > Good idea. I've defined this enum in kpartx/devmapper.h (which now > also includes <libdevmapper.h> to make the header self-contained), > and dm_find_part() uses these enum values explicitly in all return > statements. > > > 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; > > Done. In the ADD/UPDATE loops, the return value is assigned directly > to 'op', and DFP_ERR is handled with r++ and continue. In the DELETE > loop, I check against DFP_DEVICE_RELOAD to proceed with removal. > > > 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. > > I've omitted this check, as I agree that DM_DEVICE_CREATE and > DM_DEVICE_RELOAD are defined by libdevmapper as positive constants > and are unlikely to change. > > Changes since v3: > - Defined enum { DFP_DEVICE_CREATE, DFP_DEVICE_RELOAD, DFP_ERR } > in kpartx/devmapper.h > - dm_find_part() returns enum values explicitly in all return paths > - Callers assign return value directly to 'op' and check for > DFP_ERR > - kpartx/devmapper.h now includes <libdevmapper.h> directly > > Here is the new patch. > > 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 DFP_ERR when format_partname() fails, > with an error message printed by dm_find_part() itself > - Defining an enum to represent the return values of dm_find_part(): > > enum { > DFP_DEVICE_CREATE = DM_DEVICE_CREATE, > DFP_DEVICE_RELOAD = DM_DEVICE_RELOAD, > DFP_ERR = -1 > }; > > This enum is placed in kpartx/devmapper.h, which now also includes > <libdevmapper.h> directly to make the header self-contained. > dm_find_part() uses these enum values explicitly in all return > statements. > - In the ADD/UPDATE loops (both main and container), assigning the > return value directly to 'op' and checking for DFP_ERR to handle > errors > - In the DELETE loop, checking the return value against > DFP_DEVICE_RELOAD to proceed with removal, skipping on error or > not-found > - 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]>
Thanks a lot! Reviewed-by: Martin Wilck <[email protected]> I also added a test case for this problem in the "test-kpartx" script. The code is currently on my "tip" branch: https://github.com/openSUSE/multipath-tools/tree/tip -- Dr. Martin Wilck <[email protected]> SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg, Germany Geschäftsführer: Jochen Jaser, Andrew McDonald (HRB 36809,AG Nürnberg)
