On Thu, 2026-07-16 at 14:39 +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, 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
> message),
> 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 | 2 +-
> kpartx/kpartx.c | 52 ++++++++++++++++++++++++++++++++------------
> --
> 2 files changed, 37 insertions(+), 17 deletions(-)
>
> diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
> index 45dac585..c50d6368 100644
> --- a/kpartx/devmapper.c
> +++ b/kpartx/devmapper.c
> @@ -646,7 +646,7 @@ int dm_find_part(const char *parent, const char
> *delim, int part,
> if (!format_partname(name, namesiz, parent, delim, part)) {
> if (verbose)
> fprintf(stderr, "partname too small\n");
> - return 0;
> + return -1;
> }
>
> r = dm_map_present(name, part_uuid);
> diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
> index cfd82128..72f0bb70 100644
> --- a/kpartx/kpartx.c
> +++ b/kpartx/kpartx.c
> @@ -437,7 +437,7 @@ main(int argc, char **argv){
> case UPDATE:
> /* ADD and UPDATE share the same code that
> adds new partitions. */
> for (j = 0, c = 0; j < n; j++) {
> - char *part_uuid, *reason;
> + char *part_uuid = NULL, *reason;
>
> if (slices[j].size == 0)
> continue;
> @@ -454,10 +454,20 @@ main(int argc, char **argv){
> exit(1);
> }
>
> - op = (dm_find_part(mapname, delim, j
> + 1, uuid,
> - partname,
> sizeof(partname),
> - &part_uuid,
> verbose) ?
> - DM_DEVICE_RELOAD :
> DM_DEVICE_CREATE);
> + int res = dm_find_part(mapname,
> delim, j + 1, uuid,
> + partname,
> sizeof(partname),
> + &part_uuid,
> verbose);
> + if (res < 0) {
> + fprintf(stderr,
> + "partition name too
> long for partition %d, skipping\n",
> + j + 1);
Please move this message to dm_find_part() as well.
Thanks
Martin
> + r++;
> + continue;
> + } else if (res == 0) {
> + op = DM_DEVICE_CREATE;
> + } else {
> + op = DM_DEVICE_RELOAD;
> + }
>
> if (part_uuid && uuid) {
> if (check_uuid(uuid,
> part_uuid, &reason) != 0) {
> @@ -500,7 +510,7 @@ main(int argc, char **argv){
> d = c;
> while (c) {
> for (j = 0; j < n; j++) {
> - char *part_uuid, *reason;
> + char *part_uuid = NULL,
> *reason;
> int k = slices[j].container
> - 1;
>
> if (slices[j].size == 0)
> @@ -526,11 +536,21 @@ main(int argc, char **argv){
> exit(1);
> }
>
> - op = (dm_find_part(mapname,
> delim, j + 1, uuid,
> - partname,
> -
> sizeof(partname),
> -
> &part_uuid, verbose) ?
> - DM_DEVICE_RELOAD :
> DM_DEVICE_CREATE);
> + int res =
> dm_find_part(mapname, delim, j + 1, uuid,
> + part
> name,
> + size
> of(partname),
> + &par
> t_uuid, verbose);
> + if (res < 0) {
> + fprintf(stderr,
> + "partition
> name too long for partition %d, skipping\n",
> + j + 1);
> + r++;
> + continue;
> + } else if (res == 0) {
> + op =
> DM_DEVICE_CREATE;
> + } else {
> + op =
> DM_DEVICE_RELOAD;
> + }
>
> if (part_uuid && uuid) {
> if (check_uuid(uuid,
> part_uuid, &reason) != 0) {
> @@ -570,11 +590,11 @@ main(int argc, char **argv){
> }
>
> for (j = MAXSLICES-1; j >= 0; j--) {
> - char *part_uuid, *reason;
> - if (slices[j].size ||
> - !dm_find_part(mapname, delim, j
> + 1, uuid,
> - partname,
> sizeof(partname),
> - &part_uuid,
> verbose))
> + char *part_uuid = NULL, *reason;
> + int res = dm_find_part(mapname,
> delim, j + 1, uuid,
> + partname,
> sizeof(partname),
> + &part_uuid,
> verbose);
> + if (slices[j].size || res <= 0)
> continue;
>
> if (part_uuid && uuid) {
--
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)
