[S] Change in flashrom[master]: cli_classic.c: Early init of layout obscures invalid memory access
Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Angel Pons.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67723 )
Change subject: cli_classic.c: Early init of layout obscures invalid memory access ......................................................................
Patch Set 2:
(3 comments)
Patchset:
PS2: n
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/67723/comment/91978e8d_106a8121 PS2, Line 541: struct flashrom_layout *layout = NULL;
The issue is not that `NULL` means 'no layout' but rather it is the attempted use of a layout before […]
Ultimately `layout` can still end up as NULL if no layout provided/found, so anything that uses it (e.g. get_region_range call below) should check it is non-null.
But we can't really get the compiler to enforce that since it only checks the variable has been initialized, and it doesn't check for initialized-to-non-null.
I'm ok with leaving layout set to NULL here instead of setting it to NULL in the new `else` block below.
An alternative would be to delete `default_layout` in `struct flashctx` and assign `flash->layout` to a default layout in `probe_flash()`. Then users can override the default `flash->layout` value if they want, and flash->layout will always be a valid layout.
Anyway I'm going to move this patch to the end since the other two are sufficient to fix the segfault.
https://review.coreboot.org/c/flashrom/+/67723/comment/0fe60a19_efc92a2a PS2, Line 1110: msg_gdbg("Valid layout could not be found without image.\n");
Nik, can you come up with a better string here for your bug. […]
Maybe we should change it to "No layout provided or found in image", it's really just for debugging.
-
Nikolai Artemiev (Code Review)