[syslinux] [PATCH] chain.c: allocation fixes
Michal Soltys
soltys at ziu.info
Sat Jul 24 08:50:06 PDT 2010
1) At the end of main, there's attempt to free cur_part->record, which
rarely comes from malloc. Only valid case is if gpt handover was performed
and chaining was not successful (cur_part->record is overwritten with gpt
specifc handover record). Freeing the handover area has been adjusted.
2) If our current iterator is ebr, parent wouldn't be freed at the end of
main. Added generic function to free iterators, used by iterators themselves
and in main.
3) Malloced areas from read sector and/or file should be freed as well.
Signed-off-by: Michal Soltys <soltys at ziu.info>
---
com32/modules/chain.c | 112 ++++++++++++++++++++++++++++---------------------
1 files changed, 64 insertions(+), 48 deletions(-)
diff --git a/com32/modules/chain.c b/com32/modules/chain.c
index b5c1d8f..6c3bb64 100644
--- a/com32/modules/chain.c
+++ b/com32/modules/chain.c
@@ -464,6 +464,9 @@ struct disk_part_iter;
typedef struct disk_part_iter *(*disk_part_iter_func) (struct disk_part_iter *
part);
+/* Forward declaration */
+static struct disk_part_iter *next_ebr_part(struct disk_part_iter *part);
+
/* Contains details for a partition under examination */
struct disk_part_iter {
/* The block holding the table we are part of */
@@ -505,6 +508,21 @@ struct disk_part_iter {
} private;
};
+static void free_iter(struct disk_part_iter *part)
+{
+ struct disk_part_iter *cur;
+
+ while(part) {
+ cur = part;
+ if(part->next == next_ebr_part)
+ part = part->private.ebr.parent;
+ else
+ part = NULL;
+ free(cur->block);
+ free(cur);
+ }
+}
+
static struct disk_part_iter *next_ebr_part(struct disk_part_iter *part)
{
const struct part_entry *ebr_table;
@@ -570,10 +588,7 @@ static struct disk_part_iter *next_ebr_part(struct disk_part_iter *part)
err_ebr:
out_finished:
- free(part->private.ebr.parent->block);
- free(part->private.ebr.parent);
- free(part->block);
- free(part);
+ free_iter(part);
return NULL;
}
@@ -603,10 +618,9 @@ static struct disk_part_iter *next_mbr_part(struct disk_part_iter *part)
error("Could not allocate extended partition iterator!\n");
goto err_alloc;
}
+ memset(ebr_part, 0, sizeof(*ebr_part));
/* Setup EBR iterator parameters */
- ebr_part->block = NULL;
ebr_part->index = 4;
- ebr_part->record = NULL;
ebr_part->next = next_ebr_part;
ebr_part->private.ebr.parent = part;
/* Trigger an initial EBR load */
@@ -625,9 +639,7 @@ static struct disk_part_iter *next_mbr_part(struct disk_part_iter *part)
return part;
err_alloc:
-
- free(part->block);
- free(part);
+ free_iter(part);
return NULL;
}
@@ -870,9 +882,7 @@ static struct disk_part_iter *next_gpt_part(struct disk_part_iter *part)
return part;
err_last:
- free(part->block);
- free(part);
-
+ free_iter(part);
return NULL;
}
@@ -889,6 +899,7 @@ static struct disk_part_iter *get_first_partition(struct disk_part_iter *part)
error("Count not allocate partition iterator!\n");
goto err_alloc_iter;
}
+ memset(part, 0, sizeof(*part));
/* Read MBR */
part->block = read_sectors(0, 2);
if (!part->block) {
@@ -940,16 +951,10 @@ static struct disk_part_iter *get_first_partition(struct disk_part_iter *part)
return part->next(part);
err_gpt_table:
-
err_mbr:
-
- free(part->block);
- part->block = NULL;
err_read_mbr:
-
- free(part);
err_alloc_iter:
-
+ free_iter(part);
return NULL;
}
@@ -1292,6 +1297,9 @@ int main(int argc, char *argv[])
struct mbr *mbr = NULL;
char *p;
struct disk_part_iter *cur_part = NULL;
+ void *sect_area = NULL, *file_area = NULL;
+ struct part_entry *hand_area = NULL;
+
struct syslinux_rm_regs regs;
char *drivename, *partition;
int hd, drive, whichpart = 0; /* MBR by default */
@@ -1523,6 +1531,7 @@ int main(int argc, char *argv[])
goto bail;
}
data[ndata].base = load_base;
+ file_area = (void *)data[ndata].data;
load_base = 0x7c00; /* If we also load a boot sector */
/* Create boot info table: needed when you want to chainload
@@ -1701,7 +1710,9 @@ int main(int argc, char *argv[])
} else if (!(data[ndata].data = read_sectors(cur_part->lba_data, 1))) {
error("Cannot read boot sector\n");
goto bail;
- }
+ } else
+ sect_area = (void *)data[ndata].data;
+
data[ndata].size = SECTOR;
data[ndata].base = load_base;
@@ -1740,7 +1751,6 @@ int main(int argc, char *argv[])
if (cur_part) {
if (cur_part->next == next_gpt_part) {
/* Do GPT hand-over, if applicable (as per syslinux/doc/gpt.txt) */
- struct part_entry *record;
/* Look at the GPT partition */
const struct gpt_part *gp = (const struct gpt_part *)
(cur_part->block +
@@ -1748,73 +1758,79 @@ int main(int argc, char *argv[])
/* Note the partition length */
uint64_t lba_count = gp->lba_last - gp->lba_first + 1;
/* The length of the hand-over */
- int synth_size =
+ uint32_t synth_size =
sizeof(struct part_entry) + sizeof(uint32_t) +
cur_part->private.gpt.size;
/* Will point to the partition record length in the hand-over */
uint32_t *plen;
/* Allocate the hand-over record */
- record = malloc(synth_size);
- if (!record) {
+ hand_area = malloc(synth_size);
+ if (!hand_area) {
error("Could not build GPT hand-over record!\n");
goto bail;
}
/* Synthesize the record */
- memset(record, 0, synth_size);
- record->active_flag = 0x80;
- record->ostype = 0xED;
+ memset(hand_area, 0, synth_size);
+ hand_area->active_flag = 0x80;
+ hand_area->ostype = 0xED;
/* All bits set by default */
- record->start_lba = ~(uint32_t) 0;
- record->length = ~(uint32_t) 0;
+ hand_area->start_lba = ~(uint32_t) 0;
+ hand_area->length = ~(uint32_t) 0;
/* If these fit the precision, pass them on */
- if (cur_part->lba_data < record->start_lba)
- record->start_lba = cur_part->lba_data;
- if (lba_count < record->length)
- record->length = lba_count;
+ if (cur_part->lba_data < hand_area->start_lba)
+ hand_area->start_lba = cur_part->lba_data;
+ if (lba_count < hand_area->length)
+ hand_area->length = lba_count;
/* Next comes the GPT partition record length */
- plen = (uint32_t *) (record + 1);
+ plen = (uint32_t *) (hand_area + 1);
plen[0] = cur_part->private.gpt.size;
/* Next comes the GPT partition record copy */
memcpy(plen + 1, gp, plen[0]);
- cur_part->record = record;
regs.eax.l = 0x54504721; /* '!GPT' */
data[ndata].base = 0x7be;
data[ndata].size = synth_size;
- data[ndata].data = (void *)record;
+ data[ndata].data = (void *)hand_area;
ndata++;
regs.esi.w[0] = 0x7be;
dprintf("GPT handover:\n");
- mbr_part_dump(record);
+ mbr_part_dump(hand_area);
gpt_part_dump((struct gpt_part *)(plen + 1));
} else if (cur_part->record) {
/* MBR handover protocol */
- static struct part_entry handover_record;
- handover_record = *cur_part->record;
- handover_record.start_lba = cur_part->lba_data;
+ /* Allocate the hand-over record */
+ hand_area = malloc(sizeof(struct part_entry));
+ if (!hand_area) {
+ error("Could not build MBR hand-over record!\n");
+ goto bail;
+ }
+
+ memcpy(hand_area, cur_part->record, sizeof(struct part_entry));
+ hand_area->start_lba = cur_part->lba_data;
data[ndata].base = 0x7be;
- data[ndata].size = sizeof handover_record;
- data[ndata].data = &handover_record;
+ data[ndata].size = sizeof(struct part_entry);
+ data[ndata].data = (void *)hand_area;
ndata++;
regs.esi.w[0] = 0x7be;
dprintf("MBR handover:\n");
- mbr_part_dump(&handover_record);
+ mbr_part_dump(hand_area);
}
}
do_boot(data, ndata, ®s);
bail:
- if (cur_part) {
- free(cur_part->block);
- free((void *)cur_part->record);
- }
- free(cur_part);
+ /* Free iterator */
+ free_iter(cur_part);
free(mbr);
+ /* Free allocated areas */
+ free(file_area);
+ free(sect_area);
+ free(hand_area);
return 255;
}
--
1.6.3.1
More information about the Syslinux
mailing list