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

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)