David Cantrell <[EMAIL PROTECTED]> wrote:
> On Thu, 2007-03-08 at 22:26 +0100, Jim Meyering wrote:
>> David Cantrell <[EMAIL PROTECTED]> wrote:
>> > On Thu, 2007-03-08 at 15:43 +0100, Jim Meyering wrote:
>> >> Here's a fix for the first memory overrun bug I found:
>> >>
>> >> aix.c: Avoid memory overrun. Don't assume logical sector size <= 512B
>> >> * libparted/labels/aix.c (aix_probe): Return 0 if the
>> >> sector size is larger than our AixLabel size.
>> >> (aix_clobber): Rather than PED_ASSERT'ing that aix_probe returns 1,
>> >> simply return 0 if aix_probe returns fails.
>> ...
>> > I like the approach of allocating the required buffer size instead of
>> > using fixed-size buffers.
>> >
>> > Either solution is fine, but I would prefer we go with the ped_malloc()
>> > approach that's used in amiga_probe so we are consistent throughout
>> > libparted.
>>
>> OK. I've reworked it and tested the read-only parts.
...
> Should we not be using unsigned int where we're using int?
Yep. Good catch.
Actually, it should probably be uint32_t,
but if "unsigned int" ever ends up with size different
from 32 bits, this will be the least of our problems.
Here's an updated patch:
--------------------
aix.c: Avoid memory overrun. Don't assume logical sector size <= 512B
From: Jim Meyering <[EMAIL PROTECTED]>
* libparted/labels/aix.c (struct AixLabel): Remove definition.
(aix_label_magic_get, aix_label_magic_set): New functions.
(read_sector): New function.
(aix_probe): Rewrite not to use the above, and not a static buffer.
(aix_clobber): Likewise.
Also, rather than PED_ASSERT'ing that aix_probe returns 1,
simply return 0 if aix_probe returns fails.
(ped_disk_aix_init): Remove assertion, now that AixLabel is gone.
---
libparted/labels/aix.c | 64 ++++++++++++++++++++++++++++++++----------------
1 files changed, 43 insertions(+), 21 deletions(-)
diff --git a/libparted/labels/aix.c b/libparted/labels/aix.c
index a16ead4..9e7a3e2 100644
--- a/libparted/labels/aix.c
+++ b/libparted/labels/aix.c
@@ -33,45 +33,68 @@
# define _(String) (String)
#endif /* ENABLE_NLS */
-typedef struct {
- unsigned int magic; /* expect AIX_LABEL_MAGIC */
- unsigned int fillbytes[127];
-} AixLabel;
-
#define AIX_LABEL_MAGIC 0xc9c2d4c1
static PedDiskType aix_disk_type;
-static int
-aix_probe (const PedDevice *dev)
+static inline int
+aix_label_magic_get (const char *label)
{
- AixLabel label;
+ return *(unsigned int *)label;
+}
- PED_ASSERT (dev != NULL, return 0);
+static inline void
+aix_label_magic_set (char *label, int magic_val)
+{
+ *(unsigned int *)label = magic_val;
+}
- if (!ped_device_read (dev, &label, 0, 1))
+/* Read a single sector, of length DEV->sector_size, into malloc'd storage.
+ If the read fails, free the memory and return zero without modifying *BUF.
+ Otherwise, set *BUF to the new buffer and return 1. */
+static int
+read_sector (const PedDevice *dev, char **buf)
+{
+ char *b = ped_malloc (dev->sector_size);
+ PED_ASSERT (b != NULL, return 0);
+ if (!ped_device_read (dev, b, 0, 1)) {
+ ped_free (b);
return 0;
+ }
+ *buf = b;
+ return 1;
+}
- if (PED_BE32_TO_CPU (label.magic) != AIX_LABEL_MAGIC)
- return 0;
+static int
+aix_probe (const PedDevice *dev)
+{
+ PED_ASSERT (dev != NULL, return 0);
- return 1;
+ char *label;
+ if (!read_sector (dev, &label))
+ return 0;
+ unsigned int magic = aix_label_magic_get (label);
+ ped_free (label);
+ return magic == AIX_LABEL_MAGIC;
}
#ifndef DISCOVER_ONLY
static int
aix_clobber (PedDevice* dev)
{
- AixLabel label;
-
PED_ASSERT (dev != NULL, return 0);
- PED_ASSERT (aix_probe (dev), return 0);
- if (!ped_device_read (dev, &label, 0, 1))
+ if (!aix_probe (dev))
return 0;
-
- label.magic = 0;
- return ped_device_write (dev, &label, 0, 1);
+
+ char *label;
+ if (!read_sector (dev, &label))
+ return 0;
+
+ aix_label_magic_set (label, 0);
+ int result = ped_device_write (dev, label, 0, 1);
+ ped_free (label);
+ return result;
}
#endif /* !DISCOVER_ONLY */
@@ -263,7 +286,6 @@ static PedDiskType aix_disk_type = {
void
ped_disk_aix_init ()
{
- PED_ASSERT (sizeof (AixLabel) == 512, return);
ped_disk_type_register (&aix_disk_type);
}
_______________________________________________
parted-devel mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/parted-devel
