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, 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 message),
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 | 2 +-
kpartx/kpartx.c | 52 ++++++++++++++++++++++++++++++++--------------
2 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
index 45dac585..c50d6368 100644
--- a/kpartx/devmapper.c
+++ b/kpartx/devmapper.c
@@ -646,7 +646,7 @@ int dm_find_part(const char *parent, const char *delim, int
part,
if (!format_partname(name, namesiz, parent, delim, part)) {
if (verbose)
fprintf(stderr, "partname too small\n");
- return 0;
+ return -1;
}
r = dm_map_present(name, part_uuid);
diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index cfd82128..72f0bb70 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);
+ int res = dm_find_part(mapname, delim, j + 1,
uuid,
+ partname,
sizeof(partname),
+ &part_uuid, verbose);
+ if (res < 0) {
+ fprintf(stderr,
+ "partition name too long for
partition %d, skipping\n",
+ j + 1);
+ 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 +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);
+ int res = dm_find_part(mapname, delim,
j + 1, uuid,
+ partname,
+
sizeof(partname),
+ &part_uuid,
verbose);
+ if (res < 0) {
+ fprintf(stderr,
+ "partition name too
long for partition %d, skipping\n",
+ j + 1);
+ 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 +590,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
