diff --git a/cli_classic.c b/cli_classic.c index 870f8da0c..a9f9dc107 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -1250,14 +1250,14 @@ int main(int argc, char *argv[]) msg_cinfo("More than one compatible controller found for the requested flash " "chip, using the first one.\n"); - int startchip = -1; + int force_probe_ret = ERROR_FLASHROM_PROBE_NO_CHIPS_FOUND; for (j = 0; j < registered_master_count; j++) { mst = ®istered_masters[j]; - startchip = probe_flash(mst, 0, &context, 1, options.chip_to_probe); - if (startchip != -1) + force_probe_ret = probe_flash(mst, 0, &context, 1, options.chip_to_probe); + if (force_probe_ret >= 0) break; } - if (startchip == -1) { + if (force_probe_ret < 0) { // FIXME: This should never happen! Ask for a bug report? msg_cinfo("Probing for flash chip '%s' failed.\n", options.chip_to_probe); ret = 1; diff --git a/flashrom.c b/flashrom.c index af1555283..457df728b 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1104,6 +1104,15 @@ static probe_func_t *lookup_probe_func_ptr(const struct flashchip *chip) return NULL; } +/* + * Probes the entries in flashchips array one by one, starting from `startchip` index. + * Probing keeps going until first match found or end of array reached. + * + * Returns: + * the position of the matched chip, i.e. index of the entry in flashchips array + * ERROR_FLASHROM_PROBE_NO_CHIPS_FOUND if no matches found + * ERROR_FLASHROM_PROBE_INTERNAL_ERROR if some unexpected error happened during this operation +*/ int probe_flash(struct registered_master *mst, int startchip, struct flashctx *flash, int force, const char *const chip_to_probe) { const struct flashchip *chip; @@ -1130,7 +1139,7 @@ int probe_flash(struct registered_master *mst, int startchip, struct flashctx *f flash->chip = calloc(1, sizeof(*flash->chip)); if (!flash->chip) { msg_gerr("Out of memory!\n"); - return -1; + return ERROR_FLASHROM_PROBE_INTERNAL_ERROR; } *flash->chip = *chip; flash->mst = mst; @@ -1188,10 +1197,10 @@ notfound: } if (!flash->chip) - return -1; + return ERROR_FLASHROM_PROBE_NO_CHIPS_FOUND; if (init_default_layout(flash) < 0) - return -1; + return ERROR_FLASHROM_PROBE_INTERNAL_ERROR; tmp = flashbuses_to_text(flash->chip->bustype); msg_cinfo("%s %s flash chip \"%s\" (%d kB, %s) ", force ? "Assuming" : "Found", diff --git a/include/flash.h b/include/flash.h index b5ea2b5e7..59f9a8ae5 100644 --- a/include/flash.h +++ b/include/flash.h @@ -720,6 +720,9 @@ int write_flash(struct flashctx *flash, const uint8_t *buf, unsigned int start, */ #define ERROR_FLASHROM_LIMIT -201 +#define ERROR_FLASHROM_PROBE_NO_CHIPS_FOUND -1 +#define ERROR_FLASHROM_PROBE_INTERNAL_ERROR -2 + struct cli_progress { unsigned int stage_pc[FLASHROM_PROGRESS_NR]; unsigned int visible_stages; /* Bitmask of stages with non-zero progress. */ diff --git a/include/libflashrom.h b/include/libflashrom.h index 0b3a5b5fc..73261dd24 100644 --- a/include/libflashrom.h +++ b/include/libflashrom.h @@ -4,6 +4,8 @@ * Copyright (C) 2010 Google Inc. * Copyright (C) 2012 secunet Security Networks AG * (Written by Nico Huber for secunet) + * Copyright (C) 2025 Dmitry Zhadinets + * Copyright (C) 2025 Google LLC * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/libflashrom.c b/libflashrom.c index a45e0d170..a98d4ba53 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -4,6 +4,7 @@ * Copyright (C) 2012, 2016 secunet Security Networks AG * (Written by Nico Huber for secunet) * Copyright (C) 2025 Dmitry Zhadinets + * Copyright 2025 Google LLC * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -342,11 +343,22 @@ int flashrom_flash_probe(struct flashrom_flashctx **const flashctx, memset(*flashctx, 0, sizeof(**flashctx)); for (i = 0; i < registered_master_count; ++i) { - int flash_idx = -1; - if (!ret || (flash_idx = probe_flash(®istered_masters[i], 0, *flashctx, 0, chip_name)) != -1) { + int flash_idx = ERROR_FLASHROM_PROBE_NO_CHIPS_FOUND; + if (!ret || (flash_idx = probe_flash(®istered_masters[i], 0, *flashctx, 0, chip_name)) + != ERROR_FLASHROM_PROBE_NO_CHIPS_FOUND) { + if (flash_idx == ERROR_FLASHROM_PROBE_INTERNAL_ERROR) { + ret = 1; + break; + } ret = 0; /* We found one chip, now check that there is no second match. */ - if (probe_flash(®istered_masters[i], flash_idx + 1, &second_flashctx, 0, chip_name) != -1) { + int second_flash_idx = + probe_flash(®istered_masters[i], flash_idx + 1, &second_flashctx, 0, chip_name); + if (second_flash_idx == ERROR_FLASHROM_PROBE_INTERNAL_ERROR) { + ret = 1; + break; + } + if (second_flash_idx != ERROR_FLASHROM_PROBE_NO_CHIPS_FOUND) { flashrom_layout_release(second_flashctx.default_layout); free(second_flashctx.chip); ret = 3; @@ -366,7 +378,7 @@ int flashrom_flash_probe_v2(struct flashrom_flashctx *flashctx, const struct flashrom_programmer *flashprog, const char *chip_name) { - int startchip; + int startchip = 0; unsigned int all_matched_count = 0; // start with no match found const char **matched_names = calloc(flashchips_size + 1, sizeof(char*)); @@ -377,7 +389,7 @@ int flashrom_flash_probe_v2(struct flashrom_flashctx *flashctx, struct flashctx *context_for_probing = (all_matched_count > 0) ? &second_flashctx : flashctx; startchip = probe_flash(®istered_masters[i], startchip, context_for_probing, 0, chip_name); - if (startchip == -1) + if (startchip < 0) break; matched_names[all_matched_count] = context_for_probing->chip->name; @@ -396,16 +408,7 @@ int flashrom_flash_probe_v2(struct flashrom_flashctx *flashctx, matched_names = realloc(matched_names, (all_matched_count + 1) * sizeof(char*)); *all_matched_names = matched_names; - /* The return value should be the number of matched chips, or -1 on error. - * However, currently the error is never returned, because probe_flash return code - * is -1 for both cases of error and no match found, so there is no way - * to distinguish between probing error (e.g. error talking to hw, or out of memory) - * and no match. - * (no match can happen if chip is not in our database, even if hw work perfectly). - * - * TODO improve probe_flash return code to distinguish between - * probing error and no match found. */ - int ret = (int) all_matched_count; + int ret = (startchip == ERROR_FLASHROM_PROBE_INTERNAL_ERROR) ? -1 : (int) all_matched_count; return ret; } diff --git a/tests/libflashrom.c b/tests/libflashrom.c index 59ffeafd3..764a839ca 100644 --- a/tests/libflashrom.c +++ b/tests/libflashrom.c @@ -2,6 +2,7 @@ * This file is part of the flashrom project. * * Copyright 2025 Dmitry Zhadinets + * Copyright 2025 Google LLC * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -19,6 +20,7 @@ #include "tests.h" #include "libflashrom.h" #include "flash.h" +#include "programmer.h" static int test_log_callback(enum flashrom_log_level level, const char *format, va_list vargs) @@ -114,3 +116,58 @@ void flashrom_supported_programmers_test_success(void **state) assert_int_not_equal(ptr - array, 0); } + +#if CONFIG_DUMMY == 1 +typedef void* (map_flash_fn)(const char *descr, uintptr_t phys_addr, size_t len); +typedef int (spi_send_fn)(const struct flashctx *flash, unsigned int writecnt, + unsigned int readcnt, + const unsigned char *writearr, + unsigned char *readarr); + +static void *always_fail_map(const char *descr, uintptr_t phys_addr, size_t len) +{ + return ERROR_PTR; +} + +static int always_fail_spi_send_command(const struct flashctx *flash, unsigned int writecnt, + unsigned int readcnt, + const unsigned char *writearr, + unsigned char *readarr) +{ + return -1; +} + +void probe_v2_error_code_propagation(void **state) +{ + (void) state; /* unused */ + + struct flashrom_flashctx flashctx = { 0 }; + struct flashrom_programmer *flashprog; + const char **all_matched_names = NULL; + + assert_int_equal(0, flashrom_programmer_init(&flashprog, + programmer_dummy.name, + "bus=spi,emulate=W25Q128FV")); + + map_flash_fn *original_map_flash = registered_masters[0].spi.map_flash_region; + spi_send_fn *original_spi_send = registered_masters[0].spi.command; + /* This makes sure probe_flash fails. */ + registered_masters[0].spi.map_flash_region = &always_fail_map; + registered_masters[0].spi.command = &always_fail_spi_send_command; + + assert_int_equal(0 /* no chips found */, + flashrom_flash_probe_v2(&flashctx, &all_matched_names, + flashprog, + NULL)); + + /* restore programmer functions */ + registered_masters[0].spi.map_flash_region = original_map_flash; + registered_masters[0].spi.command = original_spi_send; + + assert_int_equal(0, flashrom_programmer_shutdown(flashprog)); + + flashrom_data_free(all_matched_names); +} +#else + SKIP_TEST(probe_v2_error_code_propagation) +#endif /* CONFIG_DUMMY */ diff --git a/tests/tests.c b/tests/tests.c index c43af3715..0b4b7adb5 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -483,6 +483,7 @@ int main(int argc, char *argv[]) cmocka_unit_test(flashrom_set_log_callback_v2_test_success), cmocka_unit_test(flashrom_set_log_level_test_success), cmocka_unit_test(flashrom_supported_programmers_test_success), + cmocka_unit_test(probe_v2_error_code_propagation), }; ret |= cmocka_run_group_tests_name("libflashrom.c tests", libflashrom_tests, NULL, NULL); diff --git a/tests/tests.h b/tests/tests.h index 6ec40568e..d9e54b774 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -36,6 +36,7 @@ void flashrom_set_log_callback_test_success(void **state); void flashrom_set_log_callback_v2_test_success(void **state); void flashrom_set_log_level_test_success(void **state); void flashrom_supported_programmers_test_success(void **state); +void probe_v2_error_code_propagation(void **state); /* spi25.c */ void spi_write_enable_test_success(void **state);