Project

General

Profile

Actions

Bug #555

closed

create_erase_layout Segmentation Fault

Added by Grant Pannell 3 months ago. Updated 2 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
Start date:
09/04/2024
Due date:
% Done:

0%

Estimated time:
Affected versions:
Needs backport to:
Affected hardware:
Protectli FW4B
Affected OS:
OpenBSD 7.5

Description

Hi,

Using flashrom 1.4 on OpenBSD. This is a Protectli FW4B which has a Macronix MX25U6435E/F. Previous versions of flashrom worked OK. A bit of digging looks like this has been refactored.

Still narrowing down the issue, but flashrom finishes "Reading old flash chip contents.... done." then crashes.

GDB Backtrace is essentially:

create_erase_layout
flashrom_image_write
do_write
main


Files

patch-erasure_layout_c (511 Bytes) patch-erasure_layout_c Grant Pannell, 09/05/2024 06:03 PM
Actions #1

Updated by Grant Pannell 3 months ago

  • Subject changed from create_erase_layout SegFault to create_erase_layout Segmentation Fault
Actions #2

Updated by Grant Pannell 3 months ago ยท Edited

Grant Pannell wrote:

Hi,

Using flashrom 1.4 on OpenBSD. This is a Protectli FW4B which has a Macronix MX25U6435E/F. Previous versions of flashrom worked OK. A bit of digging looks like this has been refactored.

Still narrowing down the issue, but flashrom finishes "Reading old flash chip contents.... done." then crashes.

GDB Backtrace is essentially:

create_erase_layout
flashrom_image_write
do_write
main

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?

Actions #4

Updated by Anastasia Klimchuk 3 months ago

  • Target version changed from none to 1.5
Actions #5

Updated by Anastasia Klimchuk 2 months ago

  • Status changed from New to Resolved
  • Assignee set to Grant Pannell

Grant fixed the bug with the patch: https://review.coreboot.org/c/flashrom/+/84234
I am closing this ticket.

Also relevant thread on the mailing list:
https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/BAQWGZQ5ASH2TCXQYFNFDWPYBUTSNNZV/

Actions

Also available in: Atom PDF