[flashrom] [PATCH] layout: Verify layout entries before building a new image using them.
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Mon Sep 23 09:51:03 CEST 2013
Am 23.09.2013 05:42 schrieb Stefan Tauner:
> This fixes a SEGFAULT if a layout entry is included that addresses memory
> outside the current chip's address range.
>
> Also, abort for non-write operations if a layout file is given.
>
> Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
Thanks for your patch!
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
with one small comment (no need to change the code, just think about it)
> ---
> cli_classic.c | 6 ++++++
> flash.h | 11 ++++++++++-
> flashrom.c | 11 ++++++++---
> layout.c | 33 +++++++++++++++++++++++++++++----
> 4 files changed, 53 insertions(+), 8 deletions(-)
>
> diff --git a/layout.c b/layout.c
> index 86351b8..5254115 100644
> --- a/layout.c
> +++ b/layout.c
> @@ -232,7 +232,32 @@ romentry_t *get_next_included_romentry(unsigned int start)
> return best_entry;
> }
>
> -int handle_romentries(const struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents)
> +/* Validate and - if needed - normalize layout entries. */
> +int normalize_romentries(const struct flashctx *flash)
> +{
> + chipsize_t total_size = flash->chip->total_size * 1024;
> + int ret = 0;
> +
> + int i;
> + for (i = 0; i < num_rom_entries; i++) {
> + if (rom_entries[i].start >= total_size || rom_entries[i].end >= total_size) {
> + msg_gwarn("Warning: Address range of region \"%s\" exceeds the current chip's "
> + "address space.\n", rom_entries[i].name);
> + if (rom_entries[i].included)
Shouldn't we warn about invalid rom entries in all cases, not just when
they are active/included? I'm a bit torn on that question.
> + ret = 1;
> + }
> + if (rom_entries[i].start > rom_entries[i].end) {
> + msg_gwarn("Warning: Size of the address range of region \"%s\" is not positive.\n",
> + rom_entries[i].name);
> + if (rom_entries[i].included)
Same here.
> + ret = 1;
> + }
> + }
> +
> + return ret;
> +}
> +
> +int build_new_image(const struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents)
> {
> unsigned int start = 0;
> romentry_t *entry;
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the flashrom
mailing list