Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/54147 )
Change subject: libflashrom: clear layout on flashrom_layout_release ......................................................................
libflashrom: clear layout on flashrom_layout_release
This changes the behavior of flashrom_layout_release to always clear the provided layout, even if it is the global layout (as is currently always the case).
Not clearing it causes confusing behavior where a user may release a layout and create a new one, but because it's always the global layout which wasn't cleared there will be stale information in the returned layout. This can easily lead to writing unexpected flash regions because the API provides no way to stop including regions, and makes it effectively impossible to stop including a region without completely tearing down the library.
Change-Id: I8ba4b356f2a40ccbfd4e91f81af01d5f6a6de515 Signed-off-by: Peter Marheine pmarheine@chromium.org --- M cli_classic.c M flash.h M layout.c 3 files changed, 7 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/54147/1
diff --git a/cli_classic.c b/cli_classic.c index 5fd8092..9dcfe71 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -852,7 +852,7 @@ for (i = 0; i < chipcount; i++) free(flashes[i].chip);
- layout_cleanup(&include_args); + layout_cleanup_args(&include_args); free(filename); free(fmapfile); free(referencefile); diff --git a/flash.h b/flash.h index dd8e156..fbc3539 100644 --- a/flash.h +++ b/flash.h @@ -421,7 +421,7 @@ int register_include_arg(struct layout_include_args **args, const char *arg); int read_romlayout(const char *name); int normalize_romentries(const struct flashctx *flash); -void layout_cleanup(struct layout_include_args **args); +void layout_cleanup_args(struct layout_include_args **args);
/* spi.c */ struct spi_command { diff --git a/layout.c b/layout.c index 4d1dd56..f81e01d 100644 --- a/layout.c +++ b/layout.c @@ -276,10 +276,8 @@ return overlap_detected; }
-void layout_cleanup(struct layout_include_args **args) +void layout_cleanup_args(struct layout_include_args **args) { - struct flashrom_layout *const layout = get_global_layout(); - unsigned int i; struct layout_include_args *tmp;
while (*args) { @@ -289,13 +287,6 @@ free(*args); *args = tmp; } - - for (i = 0; i < layout->num_entries; i++) { - free(layout->entries[i].name); - free(layout->entries[i].file); - layout->entries[i].included = false; - } - layout->num_entries = 0; }
/* Validate and - if needed - normalize layout entries. */ @@ -405,14 +396,16 @@ { unsigned int i;
- if (!layout || layout == get_global_layout()) + if (!layout) return;
for (i = 0; i < layout->num_entries; ++i) { free(layout->entries[i].name); free(layout->entries[i].file); } - free(layout); + layout->num_entries = 0; + if (layout != get_global_layout()) + free(layout); }
/** @} */ /* end flashrom-layout */