[PATCH v3] 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:
- Having dm_find_part() return -1 when format_partname() fails, with
  an error message printed by dm_find_part() itself, 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 count),
  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 |  5 ++---
 kpartx/kpartx.c    | 46 ++++++++++++++++++++++++++++++----------------
 2 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
index 45dac585..e03bcf44 100644
--- a/kpartx/devmapper.c
+++ b/kpartx/devmapper.c
@@ -644,9 +644,8 @@ int dm_find_part(const char *parent, const char *delim, int part,
 	char dev_t[32];
 
 	if (!format_partname(name, namesiz, parent, delim, part)) {
-		if (verbose)
-			fprintf(stderr, "partname too small\n");
-		return 0;
+		fprintf(stderr, "partition name too long for partition %d\n", part);
+		return -1;
 	}
 
 	r = dm_map_present(name, part_uuid);
diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index cfd82128..020f92e3 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,17 @@ 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) {
+					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 +507,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 +533,18 @@ 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) {
+						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 +584,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) {
-- 
2.43.0