Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flashrom.c: Fail immediately when trying to write/erase wp regions #17

Open
wants to merge 4 commits into
base: sync
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,6 @@
/util/ich_descriptors_tool/.obj

target/

subprojects/cmocka-*/
subprojects/packagecache/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either remove or split into a separate commit, please

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

11e7895: those are folders generated by meson test.

51 changes: 51 additions & 0 deletions flashrom.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1738,6 +1745,43 @@ 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);
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 )
{
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))
{
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;
}

int prepare_flash_access(struct flashctx *const flash,
const bool read_it, const bool write_it,
const bool erase_it, const bool verify_it)
Expand All @@ -1752,6 +1796,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;

Expand Down
22 changes: 20 additions & 2 deletions ichspi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,13 @@ struct hwseq_data {
struct fd_region fd_regions[MAX_FD_REGIONS];
};

#define MAX_PR_REGISTERS 6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this come from a specification?

Copy link
Author

@m-iwanicki m-iwanicki Dec 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I used largest value that could be returned in

flashrom/ichspi.c

Line 2108 in ababbe1

static void init_chipset_properties(struct swseq_data *swseq, struct hwseq_data *hwseq,

struct protected_range _ranges[MAX_PR_REGISTERS];
struct protected_ranges ranges = {
.count = 0,
.ranges = _ranges,
};

static struct hwseq_data *get_hwseq_data_from_context(const struct flashctx *flash)
{
return flash->mst->opaque.data;
Expand Down Expand Up @@ -1471,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)
{
Expand Down Expand Up @@ -1950,13 +1961,18 @@ 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 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);
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
prot->write_prot = rwperms == WRITE_PROT;
prot->write_prot = (rwperms == WRITE_PROT) || (rwperms == LOCKED);

I think, at least

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed based on

flashrom/ichspi.c

Line 2267 in ababbe1

switch (ich_spi_rw_restricted) {
that LOCKED != write-protected
If I'm reading this correctly in case ich_spi_rw_restricted == LOCKED then warning will only mention that's it's read-protected

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)
Expand Down Expand Up @@ -2019,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,
};

Expand Down Expand Up @@ -2239,11 +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);
ich_spi_rw_restricted |= ich9_handle_pr(reg_pr0, i);
ich_spi_rw_restricted |= ich9_handle_pr(reg_pr0, i, &_ranges[i]);
}

switch (ich_spi_rw_restricted) {
Expand Down
13 changes: 13 additions & 0 deletions include/layout.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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__ */
1 change: 1 addition & 0 deletions include/programmer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down