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

From: Kou Wenqi <[email protected]>

On Thu, 2026-07-16 at 15:49 +0200, [email protected] wrote:
> Thanks again. This looks quite good. However, what about this?
>
> enum {
>       DFP_DEVICE_CREATE = DM_DEVICE_CREATE,
>       DFP_DEVICE_RELOAD = DM_DEVICE_RELOAD,
>       DFP_ERR = -1,
> };

Good idea. I've defined this enum in kpartx/devmapper.h (which now
also includes <libdevmapper.h> to make the header self-contained),
and dm_find_part() uses these enum values explicitly in all return
statements.

> Then use values of this enum as return values of dm_find_part().
> If it returns DFP_ERR, bail out; otherwise use the return value as
> "op", like this:
>
>         if ((op = dm_find_part(...)) == DFP_ERR)
>                 continue;

Done. In the ADD/UPDATE loops, the return value is assigned directly
to 'op', and DFP_ERR is handled with r++ and continue. In the DELETE
loop, I check against DFP_DEVICE_RELOAD to proceed with removal.

> If you want to be extra careful, you could add something like
> this at the beginning of dm_find_part():
>
> BUILD_BUG_ON(DM_DEVICE_CREATE < 0 || DM_DEVICE_RELOAD < 0);
>
> But I think we can trust that these values will always be positive.

I've omitted this check, as I agree that DM_DEVICE_CREATE and
DM_DEVICE_RELOAD are defined by libdevmapper as positive constants
and are unlikely to change.

Changes since v3:
  - Defined enum { DFP_DEVICE_CREATE, DFP_DEVICE_RELOAD, DFP_ERR }
    in kpartx/devmapper.h
  - dm_find_part() returns enum values explicitly in all return paths
  - Callers assign return value directly to 'op' and check for DFP_ERR
  - kpartx/devmapper.h now includes <libdevmapper.h> directly

Here is the new patch.

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 DFP_ERR when format_partname() fails,
  with an error message printed by dm_find_part() itself
- Defining an enum to represent the return values of dm_find_part():

  enum {
      DFP_DEVICE_CREATE = DM_DEVICE_CREATE,
      DFP_DEVICE_RELOAD = DM_DEVICE_RELOAD,
      DFP_ERR = -1
  };

  This enum is placed in kpartx/devmapper.h, which now also includes
  <libdevmapper.h> directly to make the header self-contained.
  dm_find_part() uses these enum values explicitly in all return
  statements.
- In the ADD/UPDATE loops (both main and container), assigning the
  return value directly to 'op' and checking for DFP_ERR to handle
  errors
- In the DELETE loop, checking the return value against
  DFP_DEVICE_RELOAD to proceed with removal, skipping on error or
  not-found
- 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 | 17 +++++++++--------
 kpartx/devmapper.h |  9 +++++++++
 kpartx/kpartx.c    | 38 ++++++++++++++++++++++----------------
 3 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
index 45dac585..091d2a49 100644
--- a/kpartx/devmapper.c
+++ b/kpartx/devmapper.c
@@ -644,18 +644,19 @@ 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 DFP_ERR;
 	}
 
 	r = dm_map_present(name, part_uuid);
-	if (r == 1 || parent_uuid == NULL || *parent_uuid == '\0')
-		return r;
+	if (r == 1)
+		return DFP_DEVICE_RELOAD;
+	if (parent_uuid == NULL || *parent_uuid == '\0')
+		return DFP_DEVICE_CREATE;
 
 	uuid = make_prefixed_uuid(part, parent_uuid);
 	if (!uuid)
-		return 0;
+		return DFP_DEVICE_CREATE;
 
 	tmp = dm_find_uuid(uuid);
 	if (tmp == NULL)
@@ -688,14 +689,14 @@ int dm_find_part(const char *parent, const char *delim, int part,
 	if (r == 1) {
 		free(tmp);
 		*part_uuid = uuid;
-		return 1;
+		return DFP_DEVICE_RELOAD;
 	}
 	if (verbose)
 		fprintf(stderr, "renaming %s->%s failed\n", tmp, name);
 out:
 	free(uuid);
 	free(tmp);
-	return r;
+	return DFP_DEVICE_CREATE;
 }
 
 char *nondm_create_uuid(dev_t devt)
diff --git a/kpartx/devmapper.h b/kpartx/devmapper.h
index e4db8621..99bab221 100644
--- a/kpartx/devmapper.h
+++ b/kpartx/devmapper.h
@@ -1,6 +1,8 @@
 #ifndef KPARTX_DEVMAPPER_H_INCLUDED
 #define KPARTX_DEVMAPPER_H_INCLUDED
 
+#include <libdevmapper.h>
+
 #ifdef DM_SUBSYSTEM_UDEV_FLAG0
 #define MPATH_UDEV_RELOAD_FLAG DM_SUBSYSTEM_UDEV_FLAG0
 #else
@@ -18,6 +20,13 @@ dev_t dm_get_first_dep(char *devname);
 char * dm_mapuuid(const char *mapname);
 int dm_devn (const char * mapname, unsigned int *major, unsigned int *minor);
 int dm_remove_partmaps (char * mapname, char *uuid, dev_t devt, int verbose);
+
+enum {
+	DFP_DEVICE_CREATE = DM_DEVICE_CREATE,
+	DFP_DEVICE_RELOAD = DM_DEVICE_RELOAD,
+	DFP_ERR = -1
+};
+
 int dm_find_part(const char *parent, const char *delim, int part,
 		 const char *parent_uuid,
 		 char *name, size_t namesiz, char **part_uuid, int verbose);
diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index cfd82128..1a12f55d 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,13 @@ 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);
+				op = dm_find_part(mapname, delim, j + 1, uuid,
+						  partname, sizeof(partname),
+						  &part_uuid, verbose);
+				if (op == DFP_ERR) {
+					r++;
+					continue;
+				}
 
 				if (part_uuid && uuid) {
 					if (check_uuid(uuid, part_uuid, &reason) != 0) {
@@ -500,7 +503,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 +529,14 @@ 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);
+					op = dm_find_part(mapname, delim, j + 1, uuid,
+							  partname,
+							  sizeof(partname),
+							  &part_uuid, verbose);
+					if (op == DFP_ERR) {
+						r++;
+						continue;
+					}
 
 					if (part_uuid && uuid) {
 						if (check_uuid(uuid, part_uuid, &reason) != 0) {
@@ -570,11 +576,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 != DFP_DEVICE_RELOAD)
 					continue;
 
 				if (part_uuid && uuid) {
-- 
2.43.0