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
