Grant,
Thank you so much for posting! And for debugging and suggesting a
potential solution. I really like that you unblocked yourself, and I
also want to get to the root cause.
From looking at the code, it seemed like a real issue, and after some
time of debugging, I think this is a real issue. But the big question
for me was: why no one noticed before?
For context, this code path has been enabled by default since May 2023
, and since that time we have had lots of contributions and new
development, and I was thinking how could it be that no one hit the
issue before?
I can't see your values in your debugger on erasure_layout.c line 55,
but I did my best to reproduce. For debugging I used Test case #8 from
tests/erase_func_algo.c (set args 'Erase test case #8' in gdb in case
someone wants to run) and I think I reproed for eraseblock of size 8.
subedata went out of bounds! it was pointing to a memory it shouldn't
be , like this
{start_addr = 4025479151, end_addr = 4025479151, selected = 239,
block_num = 0, first_sub_block_index = 5249, last_sub_block_index = 0}
but it wasn't segfaulting , and those random large numbers in
start_addr and end_addr made the loop condition false and the program
kept running. without segfaults!
I think your patch may be valid, and please don't throw it away, maybe
you will send it as a patch.
Can it be the difference between Linux and OpenBSD? It's true that
most developers work on Linux, also from what I remember OpenBSD has
more strict memory protections. Is it possible that OpenBSD segfaults
in the same code where Linux keeps running?
On Fri, Sep 6, 2024 at 3:58 AM Grant Pannell via flashrom
<[email protected]> wrote:
>
> Hi,
>
> Trying to report a bug in flashrom 1.4. I'm not subject matter expert, but
> I've done my best to try and debug and fix the issue. Seeking guidance from
> the experts on whether this is the solution :-)
>
> I found that in 1.4, it looks like the erase and write logic was refactored.
> Since then, I get segfaults when trying to flash my coreboot image.
>
> The chip is a Macronix MX25U6435E in a Protectli FW4B. I'm running on OpenBSD
> 7.5.
>
> The segfault occurs in erasure_layout.c: init_eraseblock line 55. I am no
> expert on the internals of flashrom, or what's going on here...but I've
> narrowed it down to segfaulting at the last iteration of this while loop.
> Code in question looks like:
>
> edata->first_sub_block_index = *sub_block_index;
> struct eraseblock_data *subedata = &layout[idx -
> 1].layout_list[*sub_block_index];
> while (subedata->start_addr >= start_addr && subedata->end_addr <= end_addr &&
> *sub_block_index < layout[idx-1].block_count) {
> (*sub_block_index)++;
> subedata++;
> }
>
> In my case, it seems that the last iteration looks like:
>
> layout[idx-1].block_count == 2048
> *sub_block_index == 2047
> subedata->end_addr == end_addr
>
> What then happens is, the variable "subedata" is incremented and the while
> condition is checked, but subedata is now out of bounds and the application
> segfaults. I'm pretty sure the while loop shouldn't iterate again because the
> next iteration would fail the *sub_block_index < layout[idx-1].block_count
> check (2048 < 2028). I solved this by short circuiting the while condition
> and checking that condition first, so that subedata is not accessed and
> flashrom successfully flashes my coreboot image. Patch included below. Is
> this an appropriate fix?
>
> Thank you,
>
> Grant
>
> --- erasure_layout.c.orig
> +++ erasure_layout.c
> @@ -52,8 +52,8 @@
>
> edata->first_sub_block_index = *sub_block_index;
> struct eraseblock_data *subedata = &layout[idx -
> 1].layout_list[*sub_block_index];
> - while (subedata->start_addr >= start_addr && subedata->end_addr <=
> end_addr &&
> - *sub_block_index < layout[idx-1].block_count) {
> + while (*sub_block_index < layout[idx-1].block_count &&
> + subedata->start_addr >= start_addr && subedata->end_addr <=
> end_addr) {
> (*sub_block_index)++;
> subedata++;
> }
> _______________________________________________
> flashrom mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
--
Anastasia.
_______________________________________________
flashrom mailing list -- [email protected]
To unsubscribe send an email to [email protected]