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)
