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

On Wed, 2026-07-15 at 17:36 +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:
> - Clearing the name buffer in format_partname() on overflow, so that
>   callers can detect the failure by checking partname[0]
> - In the ADD/UPDATE loops, after dm_find_part() returns 0, checking
>   whether partname is empty and skipping the partition with an error
>   message instead of proceeding with dm_addmap()
> - 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

Thanks for the patch! It looks good to me, but I'd suggest to add an
error return to dm_find_part() of testing partname[0] in the callers.

Regards
Martin

> 
> 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 |  4 +++-
>  kpartx/kpartx.c    | 44 ++++++++++++++++++++++++++++++++------------
>  2 files changed, 35 insertions(+), 13 deletions(-)
> 
> diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
> index 45dac585..9eb6eeca 100644
> --- a/kpartx/devmapper.c
> +++ b/kpartx/devmapper.c
> @@ -113,8 +113,10 @@ strip_slash (char * device)
>  static int format_partname(char *buf, size_t bufsiz,
>  			   const char *mapname, const char *delim,
> int part)
>  {
> -	if (safe_snprintf(buf, bufsiz, "%s%s%d", mapname, delim,
> part))
> +	if (safe_snprintf(buf, bufsiz, "%s%s%d", mapname, delim,
> part)) {
> +		buf[0] = '\0';
>  		return 0;
> +	}
>  	strip_slash(buf);
>  	return 1;
>  }
> diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
> index cfd82128..5972a251 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);
> +				if (!dm_find_part(mapname, delim, j
> + 1, uuid,
> +							partname,
> sizeof(partname),
> +							&part_uuid,
> verbose)) {
> +					if (partname[0] == '\0') {
> +						fprintf(stderr,
> +							"partition
> name too long for partition %d, skipping\n",
> +							j + 1);
> +						r++;
> +						continue;
> +					}
> +					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);
> +					if (!dm_find_part(mapname,
> delim, j + 1, uuid,
> +								part
> name,
> +								size
> of(partname),
> +								&par
> t_uuid, verbose)) {
> +						if (partname[0] ==
> '\0') {
> +							fprintf(stde
> rr,
> +								"par
> tition name too long for partition %d, skipping\n",
> +								j +
> 1);
> +							r++;
> +							continue;
> +						}
> +						op =
> DM_DEVICE_CREATE;
> +					} else {
> +						op =
> DM_DEVICE_RELOAD;
> +					}
>  
>  					if (part_uuid && uuid) {
>  						if (check_uuid(uuid,
> part_uuid, &reason) != 0) {
> @@ -570,7 +590,7 @@ main(int argc, char **argv){
>  			}
>  
>  			for (j = MAXSLICES-1; j >= 0; j--) {
> -				char *part_uuid, *reason;
> +				char *part_uuid = NULL, *reason;
>  				if (slices[j].size ||
>  				    !dm_find_part(mapname, delim, j
> + 1, uuid,
>  						  partname,
> sizeof(partname),

-- 
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)