From 6be9938c36bf9ee65cacc42550e8541cdc0e0ad4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Iwanicki?= Date: Wed, 4 Dec 2024 12:38:00 +0100 Subject: [PATCH 1/5] flashrom.c: Fail immediately when trying to write/erase wp regions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: Ic39b4acf1da73d093ce3a7df71c1c549d92240d5 Signed-off-by: Michał Iwanicki --- flashrom.c | 39 +++++++++++++++++++++++++++++++++++++++ ichspi.c | 44 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/flashrom.c b/flashrom.c index fc52046c0..de0d8d865 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1738,6 +1738,38 @@ static int unlock_flash_wp(struct flashctx *const flash, return ret; } +static int write_protect_check(struct flashctx *flash) { + bool check_wp = false; + size_t wp_start, wp_len; + enum flashrom_wp_mode mode; + struct flashrom_wp_cfg *cfg = NULL; + const struct romentry *entry = NULL; + const struct flashrom_layout *const layout = get_layout(flash); + + if (flashrom_wp_cfg_new(&cfg) == FLASHROM_WP_OK && + flashrom_wp_read_cfg(cfg, flash) == FLASHROM_WP_OK ) + { + flashrom_wp_get_range(&wp_start, &wp_len, cfg); + mode = flashrom_wp_get_mode(cfg); + if (mode != FLASHROM_WP_MODE_DISABLED && wp_len != 0) + check_wp = true; + } + flashrom_wp_cfg_release(cfg); + + while ((entry = layout_next_included(layout, entry))) { + if ((!flash->flags.skip_unwritable_regions && + check_for_unwritable_regions(flash, entry->region.start, entry->region.end - entry->region.start) + ) + || + (check_wp && entry->region.start < wp_start + wp_len && wp_start <= entry->region.end)) + { + return -1; + } + } + + return 0; +} + int prepare_flash_access(struct flashctx *const flash, const bool read_it, const bool write_it, const bool erase_it, const bool verify_it) @@ -1752,6 +1784,13 @@ int prepare_flash_access(struct flashctx *const flash, return 1; } + if ((write_it || erase_it) && !flash->flags.force) { + if(write_protect_check(flash)) { + msg_cerr("Requested regions are write protected. Aborting.\n"); + return 1; + } + } + if (map_flash(flash) != 0) return 1; diff --git a/ichspi.c b/ichspi.c index 591884e0d..602f2f5f3 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1289,6 +1289,19 @@ struct hwseq_data { struct fd_region fd_regions[MAX_FD_REGIONS]; }; +#define MAX_PR_REGISTERS 6 +struct pr_register { + enum ich_access_protection level; + uint32_t base; + uint32_t limit; +}; + +struct _pr_access { + uint8_t count; + struct pr_register registers[MAX_PR_REGISTERS]; +}; +struct _pr_access pr_access; + static struct hwseq_data *get_hwseq_data_from_context(const struct flashctx *flash) { return flash->mst->opaque.data; @@ -1464,6 +1477,17 @@ static void ich_get_region(const struct flashctx *flash, unsigned int addr, stru region->end = limit; region->read_prot = (level == LOCKED) || (level == READ_PROT); region->write_prot = (level == LOCKED) || (level == WRITE_PROT); + if (region->write_prot != true) { + for (ssize_t pr = 0; pr < pr_access.count; pr++) { + struct pr_register pr_reg = pr_access.registers[pr]; + if (pr_reg.level == WRITE_PROT && + region->start < pr_reg.base + pr_reg.limit && pr_reg.base <= region->end) + { + region->write_prot = true; + break; + } + } + } break; } } @@ -1950,28 +1974,32 @@ static enum ich_access_protection ich9_handle_region_access(struct fd_region *fd #define ICH_PR_PERMS(pr) (((~((pr) >> PR_RP_OFF) & 1) << 0) | \ ((~((pr) >> PR_WP_OFF) & 1) << 1)) -static enum ich_access_protection ich9_handle_pr(const size_t reg_pr0, unsigned int i) +static struct pr_register ich9_handle_pr(const size_t reg_pr0, unsigned int i) { uint8_t off = reg_pr0 + (i * 4); uint32_t pr = mmio_readl(ich_spibar + off); unsigned int rwperms_idx = ICH_PR_PERMS(pr); - enum ich_access_protection rwperms = access_perms_to_protection[rwperms_idx]; + struct pr_register pr_reg = { + .level = access_perms_to_protection[rwperms_idx], + .base = ICH_FREG_BASE(pr), + .limit = ICH_FREG_LIMIT(pr), + }; /* From 5 on we have GPR registers and start from 0 again. */ const char *const prefix = i >= 5 ? "G" : ""; if (i >= 5) i -= 5; - if (rwperms == NO_PROT) { + if (pr_reg.level == NO_PROT) { msg_pdbg2("0x%02"PRIX8": 0x%08"PRIx32" (%sPR%u is unused)\n", off, pr, prefix, i); - return NO_PROT; + return pr_reg; } msg_pdbg("0x%02"PRIX8": 0x%08"PRIx32" ", off, pr); msg_pwarn("%sPR%u: Warning: 0x%08"PRIx32"-0x%08"PRIx32" is %s.\n", prefix, i, ICH_FREG_BASE(pr), - ICH_FREG_LIMIT(pr), access_names[rwperms]); + ICH_FREG_LIMIT(pr), access_names[pr_reg.level]); - return rwperms; + return pr_reg; } /* Set/Clear the read and write protection enable bits of PR register @i @@ -2170,6 +2198,7 @@ static int init_ich_default(const struct programmer_cfg *cfg, void *spibar, enum size_t num_freg, num_pr, reg_pr0; struct hwseq_data hwseq_data = { 0 }; init_chipset_properties(&swseq_data, &hwseq_data, &num_freg, &num_pr, ®_pr0, ich_gen); + pr_access.count = num_pr; int ret = get_ich_spi_mode_param(cfg, &ich_spi_mode); if (ret) @@ -2243,7 +2272,8 @@ static int init_ich_default(const struct programmer_cfg *cfg, void *spibar, enum /* if not locked down try to disable PR locks first */ if (!ichspi_lock) ich9_set_pr(reg_pr0, i, 0, 0); - ich_spi_rw_restricted |= ich9_handle_pr(reg_pr0, i); + pr_access.registers[i] = ich9_handle_pr(reg_pr0, i); + ich_spi_rw_restricted |= pr_access.registers[i].level; } switch (ich_spi_rw_restricted) { From 8484210d1308775ebb4cd1da75b8599973552943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Iwanicki?= Date: Fri, 6 Dec 2024 19:28:53 +0100 Subject: [PATCH 2/5] Check PR ranges when deciding if range is wp protected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I989006eae97f5e015df20026b6f5361ad41223c1 Signed-off-by: Michał Iwanicki --- flashrom.c | 22 +++++++++++++---- ichspi.c | 56 +++++++++++++++++--------------------------- include/layout.h | 13 ++++++++++ include/programmer.h | 1 + 4 files changed, 53 insertions(+), 39 deletions(-) diff --git a/flashrom.c b/flashrom.c index de0d8d865..a5b3909fb 100644 --- a/flashrom.c +++ b/flashrom.c @@ -383,6 +383,13 @@ void get_flash_region(const struct flashctx *flash, int addr, struct flash_regio } } +struct protected_ranges get_protected_ranges(const struct flashctx *flash) { + struct protected_ranges ranges = { 0 }; + if ((flash->mst->buses_supported & BUS_PROG) && flash->mst->opaque.get_protected_ranges) + ranges = flash->mst->opaque.get_protected_ranges(); + return ranges; +} + int check_for_unwritable_regions(const struct flashctx *flash, unsigned int start, unsigned int len) { struct flash_region region; @@ -1745,6 +1752,7 @@ static int write_protect_check(struct flashctx *flash) { struct flashrom_wp_cfg *cfg = NULL; const struct romentry *entry = NULL; const struct flashrom_layout *const layout = get_layout(flash); + struct protected_ranges ranges = get_protected_ranges(flash); if (flashrom_wp_cfg_new(&cfg) == FLASHROM_WP_OK && flashrom_wp_read_cfg(cfg, flash) == FLASHROM_WP_OK ) @@ -1757,14 +1765,18 @@ static int write_protect_check(struct flashctx *flash) { flashrom_wp_cfg_release(cfg); while ((entry = layout_next_included(layout, entry))) { - if ((!flash->flags.skip_unwritable_regions && - check_for_unwritable_regions(flash, entry->region.start, entry->region.end - entry->region.start) - ) - || - (check_wp && entry->region.start < wp_start + wp_len && wp_start <= entry->region.end)) + if (!flash->flags.skip_unwritable_regions && + check_for_unwritable_regions(flash, entry->region.start, entry->region.end - entry->region.start)) { return -1; } + if (check_wp && entry->region.start < wp_start + wp_len && wp_start <= entry->region.end) + return -1; + for (int i = 0; i < ranges.count; ++i) { + struct protected_range prot = ranges.ranges[i]; + if (prot.write_prot && entry->region.start < prot.base + prot.limit && prot.base <= entry->region.end) + return -1; + } } return 0; diff --git a/ichspi.c b/ichspi.c index 602f2f5f3..1fdfa4bfa 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1290,18 +1290,12 @@ struct hwseq_data { }; #define MAX_PR_REGISTERS 6 -struct pr_register { - enum ich_access_protection level; - uint32_t base; - uint32_t limit; +struct protected_range _ranges[MAX_PR_REGISTERS]; +struct protected_ranges ranges = { + .count = 0, + .ranges = _ranges, }; -struct _pr_access { - uint8_t count; - struct pr_register registers[MAX_PR_REGISTERS]; -}; -struct _pr_access pr_access; - static struct hwseq_data *get_hwseq_data_from_context(const struct flashctx *flash) { return flash->mst->opaque.data; @@ -1477,17 +1471,6 @@ static void ich_get_region(const struct flashctx *flash, unsigned int addr, stru region->end = limit; region->read_prot = (level == LOCKED) || (level == READ_PROT); region->write_prot = (level == LOCKED) || (level == WRITE_PROT); - if (region->write_prot != true) { - for (ssize_t pr = 0; pr < pr_access.count; pr++) { - struct pr_register pr_reg = pr_access.registers[pr]; - if (pr_reg.level == WRITE_PROT && - region->start < pr_reg.base + pr_reg.limit && pr_reg.base <= region->end) - { - region->write_prot = true; - break; - } - } - } break; } } @@ -1495,6 +1478,10 @@ static void ich_get_region(const struct flashctx *flash, unsigned int addr, stru region->name = strdup(name); } +static struct protected_ranges ich_get_protected_ranges() { + return ranges; +} + /* Given RDID info, return pointer to entry in flashchips[] */ static const struct flashchip *flash_id_to_entry(uint32_t mfg_id, uint32_t model_id) { @@ -1974,32 +1961,33 @@ static enum ich_access_protection ich9_handle_region_access(struct fd_region *fd #define ICH_PR_PERMS(pr) (((~((pr) >> PR_RP_OFF) & 1) << 0) | \ ((~((pr) >> PR_WP_OFF) & 1) << 1)) -static struct pr_register ich9_handle_pr(const size_t reg_pr0, unsigned int i) +static enum ich_access_protection ich9_handle_pr(const size_t reg_pr0, unsigned int i, struct protected_range* prot) { uint8_t off = reg_pr0 + (i * 4); uint32_t pr = mmio_readl(ich_spibar + off); unsigned int rwperms_idx = ICH_PR_PERMS(pr); - struct pr_register pr_reg = { - .level = access_perms_to_protection[rwperms_idx], - .base = ICH_FREG_BASE(pr), - .limit = ICH_FREG_LIMIT(pr), - }; + enum ich_access_protection rwperms = access_perms_to_protection[rwperms_idx]; + + prot->base = ICH_FREG_BASE(pr); + prot->limit = ICH_FREG_LIMIT(pr); + prot->write_prot = rwperms == WRITE_PROT; + prot->read_prot = rwperms == READ_PROT || rwperms == LOCKED; /* From 5 on we have GPR registers and start from 0 again. */ const char *const prefix = i >= 5 ? "G" : ""; if (i >= 5) i -= 5; - if (pr_reg.level == NO_PROT) { + if (rwperms == NO_PROT) { msg_pdbg2("0x%02"PRIX8": 0x%08"PRIx32" (%sPR%u is unused)\n", off, pr, prefix, i); - return pr_reg; + return NO_PROT; } msg_pdbg("0x%02"PRIX8": 0x%08"PRIx32" ", off, pr); msg_pwarn("%sPR%u: Warning: 0x%08"PRIx32"-0x%08"PRIx32" is %s.\n", prefix, i, ICH_FREG_BASE(pr), - ICH_FREG_LIMIT(pr), access_names[pr_reg.level]); + ICH_FREG_LIMIT(pr), access_names[rwperms]); - return pr_reg; + return rwperms; } /* Set/Clear the read and write protection enable bits of PR register @i @@ -2047,6 +2035,7 @@ static const struct opaque_master opaque_master_ich_hwseq = { .read_register = ich_hwseq_read_status, .write_register = ich_hwseq_write_status, .get_region = ich_get_region, + .get_protected_ranges = ich_get_protected_ranges, .shutdown = ich_hwseq_shutdown, }; @@ -2198,7 +2187,6 @@ static int init_ich_default(const struct programmer_cfg *cfg, void *spibar, enum size_t num_freg, num_pr, reg_pr0; struct hwseq_data hwseq_data = { 0 }; init_chipset_properties(&swseq_data, &hwseq_data, &num_freg, &num_pr, ®_pr0, ich_gen); - pr_access.count = num_pr; int ret = get_ich_spi_mode_param(cfg, &ich_spi_mode); if (ret) @@ -2268,12 +2256,12 @@ static int init_ich_default(const struct programmer_cfg *cfg, void *spibar, enum } /* Handle PR registers */ + ranges.count = num_pr; for (i = 0; i < num_pr; i++) { /* if not locked down try to disable PR locks first */ if (!ichspi_lock) ich9_set_pr(reg_pr0, i, 0, 0); - pr_access.registers[i] = ich9_handle_pr(reg_pr0, i); - ich_spi_rw_restricted |= pr_access.registers[i].level; + ich_spi_rw_restricted |= ich9_handle_pr(reg_pr0, i, &_ranges[i]); } switch (ich_spi_rw_restricted) { diff --git a/include/layout.h b/include/layout.h index d03a15b20..6af048a6f 100644 --- a/include/layout.h +++ b/include/layout.h @@ -57,6 +57,18 @@ struct romentry { struct flash_region region; }; +struct protected_range { + uint32_t base; + uint32_t limit; + bool read_prot; + bool write_prot; +}; + +struct protected_ranges { + int count; + const struct protected_range *ranges; +}; + struct flashrom_layout; struct layout_include_args; @@ -79,5 +91,6 @@ void prepare_layout_for_extraction(struct flashrom_flashctx *); int layout_sanity_checks(const struct flashrom_flashctx *); int check_for_unwritable_regions(const struct flashrom_flashctx *flash, unsigned int start, unsigned int len); void get_flash_region(const struct flashrom_flashctx *flash, int addr, struct flash_region *region); +struct protected_ranges get_protected_ranges(const struct flashrom_flashctx *flash); #endif /* !__LAYOUT_H__ */ diff --git a/include/programmer.h b/include/programmer.h index 10feaf1c0..5d64a9c9a 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -450,6 +450,7 @@ struct opaque_master { enum flashrom_wp_result (*wp_read_cfg)(struct flashrom_wp_cfg *, struct flashctx *); enum flashrom_wp_result (*wp_get_ranges)(struct flashrom_wp_ranges **, struct flashctx *); void (*get_region)(const struct flashctx *flash, unsigned int addr, struct flash_region *region); + struct protected_ranges (*get_protected_ranges)(); int (*shutdown)(void *data); void (*delay) (const struct flashctx *flash, unsigned int usecs); void *data; From 11e7895e5b7c2e24d9e05ccd6eb1a57ba9fc855a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Iwanicki?= Date: Wed, 18 Dec 2024 09:40:05 +0100 Subject: [PATCH 3/5] .gitignore: Add folders generated by 'meson test' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: Iee172044959df8efd7148adb3a918a7dad963f6f Signed-off-by: Michał Iwanicki --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index d1cf656d6..fc483e87f 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,6 @@ /util/ich_descriptors_tool/.obj target/ + +subprojects/cmocka-*/ +subprojects/packagecache/ From c80f0327d3a26e14236019c926902d01267135ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Iwanicki?= Date: Wed, 18 Dec 2024 09:42:22 +0100 Subject: [PATCH 4/5] flashrom.c & ichspi.c: code style fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I1cdcfacaf61285bf4d7789610461a3f02ea4c64b Signed-off-by: Michał Iwanicki --- flashrom.c | 2 +- ichspi.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/flashrom.c b/flashrom.c index a5b3909fb..06a79b156 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1797,7 +1797,7 @@ int prepare_flash_access(struct flashctx *const flash, } if ((write_it || erase_it) && !flash->flags.force) { - if(write_protect_check(flash)) { + if (write_protect_check(flash)) { msg_cerr("Requested regions are write protected. Aborting.\n"); return 1; } diff --git a/ichspi.c b/ichspi.c index 1fdfa4bfa..9e7848277 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1971,7 +1971,7 @@ static enum ich_access_protection ich9_handle_pr(const size_t reg_pr0, unsigned prot->base = ICH_FREG_BASE(pr); prot->limit = ICH_FREG_LIMIT(pr); prot->write_prot = rwperms == WRITE_PROT; - prot->read_prot = rwperms == READ_PROT || rwperms == LOCKED; + prot->read_prot = (rwperms == READ_PROT) || (rwperms == LOCKED); /* From 5 on we have GPR registers and start from 0 again. */ const char *const prefix = i >= 5 ? "G" : ""; From 4de6d012667f1eee7f4c9bf7d7b02af190c5e230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Iwanicki?= Date: Thu, 13 Feb 2025 11:23:56 +0100 Subject: [PATCH 5/5] ichspi.c: set write_prot if rwperms is LOCKED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I0d8034aac411049e0223ad50f56b584602aa6899 Signed-off-by: Michał Iwanicki --- ichspi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ichspi.c b/ichspi.c index 9e7848277..0947eeabb 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1970,7 +1970,7 @@ static enum ich_access_protection ich9_handle_pr(const size_t reg_pr0, unsigned prot->base = ICH_FREG_BASE(pr); prot->limit = ICH_FREG_LIMIT(pr); - prot->write_prot = rwperms == WRITE_PROT; + prot->write_prot = (rwperms == WRITE_PROT) || (rwperms == LOCKED); prot->read_prot = (rwperms == READ_PROT) || (rwperms == LOCKED); /* From 5 on we have GPR registers and start from 0 again. */