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

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

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,
+								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) {
@@ -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),
-- 
2.43.0