Generally, I think we should always assume a "sane" layout in the
internal code to keep it simple (e.g. in flashrom.c). Regions with
start above end should be filtered (warning) / denied (bail out)
when reading the layout. Regions partially out of the flash chip's
bounds should be sanitized (warning) / denied (bail out). This
should be implemented in a follow-up if necessary.

Regarding overlapping regions: I don't see a use case. But if we
want to allow them in the layout file, I'm ok with the current
solution implemented here.

View Change

2 comments:

• File flashrom.c:

  • Patch Set #5, Line 1387: if ((entry->start > size) || (entry->end > size)) {

    is entry->start always smaller than entry->end? afaik coreboot/util/ifdtool generates regions where […]

    For IFD layouts, it's taken care of. For layout files, idk.

• File layout.c:

  • Patch Set #5, Line 189: /* Quotes or whitespace are not allowed in region names. */

    I suppose this could go in the read_romlayout() function for when a layout file is provided. […]

    What is sane? Regarding IFD region names, those are hardcoded, and always
    sane.

    I don't see what we gain from this check here, if somebody wants to name
    his region "foo bar" or even "\"foo bar\"", why would it hurt? And if any-
    where, I'd filter the names when we read the layout file. Then, weird
    names in a -i argument would just not be found (but why add extra code???).
    What do I miss?

To view, visit change 23021. To unsubscribe, or for help writing mail filters, visit settings.