Hi Phillip,
Sorry it's taken so long.
I began looking at your p%d vs %d change and started by
factoring things. In doing that I spotted two real bugs.
So these 3 c-sets fix those and do the factorization, for now.
>From a4d412f8f2a625978a7276c2df0794ebc8fe50a9 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyer...@redhat.com>
Date: Sun, 17 Apr 2011 14:15:54 +0200
Subject: [PATCH 1/3] linux: don't reference before start of buffer for short
device name
* libparted/arch/linux.c (_device_get_part_path): Avoid invalid
reference to memory before dev->path when its length is 4 or less.
---
libparted/arch/linux.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index e77210e..c65df3d 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2209,7 +2209,7 @@ _device_get_part_path (PedDevice* dev, int num)
/* Check for devfs-style /disc => /partN transformation
unconditionally; the system might be using udev with devfs rules,
and if not the test is harmless. */
- if (!strcmp (dev->path + path_len - 5, "/disc")) {
+ if (5 < path_len && !strcmp (dev->path + path_len - 5, "/disc")) {
/* replace /disc with /path%d */
strcpy (result, dev->path);
snprintf (result + path_len - 5, 16, "/part%d", num);
--
1.7.5.rc2.295.g19c42
>From aa4e9dacae79a3508b9efc2703e3c548df5ad863 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyer...@redhat.com>
Date: Thu, 14 Apr 2011 13:22:28 +0200
Subject: [PATCH 2/3] linux: clean up device naming code (no semantic change)
* libparted/arch/linux.c (zasprintf): New function.
(_device_get_part_path): Clean up:
Use size_t, not "int" for strlen-returned value.
Combine mostly duplicate snprintf uses.
Use zasprintf instead of malloc+snprintf.
---
libparted/arch/linux.c | 43 +++++++++++++++++++++++++------------------
1 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index c65df3d..5c73a55 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2195,32 +2195,39 @@ linux_probe_all ()
_probe_proc_partitions ();
}
-static char*
-_device_get_part_path (PedDevice* dev, int num)
+static char * _GL_ATTRIBUTE_FORMAT ((__printf__, 1, 2))
+zasprintf (const char *format, ...)
{
- int path_len = strlen (dev->path);
- int result_len = path_len + 16;
- char* result;
+ va_list args;
+ char *resultp;
+ va_start (args, format);
+ int r = vasprintf (&resultp, format, args);
+ va_end (args);
+ return r < 0 ? NULL : resultp;
+}
- result = (char*) ped_malloc (result_len);
- if (!result)
- return NULL;
+static char*
+_device_get_part_path (PedDevice *dev, int num)
+{
+ size_t path_len = strlen (dev->path);
+ char *result;
/* Check for devfs-style /disc => /partN transformation
unconditionally; the system might be using udev with devfs rules,
and if not the test is harmless. */
if (5 < path_len && !strcmp (dev->path + path_len - 5, "/disc")) {
/* replace /disc with /path%d */
- strcpy (result, dev->path);
- snprintf (result + path_len - 5, 16, "/part%d", num);
- } else if (dev->type == PED_DEVICE_DAC960
- || dev->type == PED_DEVICE_CPQARRAY
- || dev->type == PED_DEVICE_ATARAID
- || dev->type == PED_DEVICE_DM
- || isdigit (dev->path[path_len - 1]))
- snprintf (result, result_len, "%sp%d", dev->path, num);
- else
- snprintf (result, result_len, "%s%d", dev->path, num);
+ result = zasprintf ("%.*s/part%d",
+ (int) (path_len - 5), dev->path, num);
+ } else {
+ char const *p = (dev->type == PED_DEVICE_DAC960
+ || dev->type == PED_DEVICE_CPQARRAY
+ || dev->type == PED_DEVICE_ATARAID
+ || dev->type == PED_DEVICE_DM
+ || isdigit (dev->path[path_len - 1])
+ ? "p" : "");
+ result = zasprintf ("%s%s%d", dev->path, p, num);
+ }
return result;
}
--
1.7.5.rc2.295.g19c42
>From b4a1a6d2168ede2983da532e517483c9f90a98af Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyer...@redhat.com>
Date: Sun, 17 Apr 2011 14:25:41 +0200
Subject: [PATCH 3/3] linux: don't free invalid pointer upon asprintf failure
* libparted/arch/linux.c (_device_get_part_path): When asprintf
fails, it leaves its first argument in an undefined state, and
hence that pointer must not be freed. However, here, in two
places we could potentially free an invalid pointer. Use
zasprintf; then the pointer is either NULL or allocated,
and hence always freeable.
---
libparted/arch/linux.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 5c73a55..fa51b73 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2740,15 +2740,15 @@ _dm_add_partition (PedDisk* disk, PedPartition* part)
dev_name = dm_task_get_name (task);
- if (asprintf (&vol_name, "%sp%d", dev_name, part->num) == -1)
+ if ( ! (vol_name = zasprintf ("%sp%d", dev_name, part->num)))
goto err;
/* Caution: dm_task_destroy frees dev_name. */
dm_task_destroy (task);
task = NULL;
- if (asprintf (¶ms, "%d:%d %lld", arch_specific->major,
- arch_specific->minor, part->geom.start) == -1)
+ if ( ! (params = zasprintf ("%d:%d %lld", arch_specific->major,
+ arch_specific->minor, part->geom.start)))
goto err;
task = dm_task_create (DM_DEVICE_CREATE);
--
1.7.5.rc2.295.g19c42
_______________________________________________
bug-parted mailing list
bug-parted@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-parted
