Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/31069
Change subject: Fix erasing of unaligned regions ......................................................................
Fix erasing of unaligned regions
The erase (-E) feature is somehow a brute force method, but still, if we are given a region to erase, we should make sure to restore surrounding data if the erase block expands beyond the region.
Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Signed-off-by: Nico Huber nico.huber@secunet.com --- M flashrom.c 1 file changed, 66 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/31069/1
diff --git a/flashrom.c b/flashrom.c index 1a7dfb1..cc0a631 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1690,17 +1690,80 @@ const struct walk_info *const info, const erasefn_t erasefn) { const unsigned int erase_len = info->erase_end + 1 - info->erase_start; + const bool region_unaligned = info->region_start > info->erase_start || + info->erase_end > info->region_end; + uint8_t *backup_contents = NULL, *erased_contents = NULL; + int ret = 2;
+ /* + * If the region is not erase-block aligned, merge current flash con- + * tents into a new buffer `backup_contents`. + */ + if (region_unaligned) { + backup_contents = malloc(erase_len); + erased_contents = malloc(erase_len); + if (!backup_contents || !erased_contents) { + msg_cerr("Out of memory!\n"); + ret = 1; + goto _free_ret; + } + memset(backup_contents, ERASED_VALUE(flashctx), erase_len); + memset(erased_contents, ERASED_VALUE(flashctx), erase_len); + + msg_cdbg("R"); + /* Merge data preceding the current region. */ + if (info->region_start > info->erase_start) { + const chipoff_t start = info->erase_start; + const chipsize_t len = info->region_start - info->erase_start; + if (flashctx->chip->read(flashctx, backup_contents, start, len)) { + msg_cerr("Can't read! Aborting.\n"); + goto _free_ret; + } + } + /* Merge data following the current region. */ + if (info->erase_end > info->region_end) { + const chipoff_t start = info->region_end + 1; + const chipoff_t rel_start = start - info->erase_start; /* within this erase block */ + const chipsize_t len = info->erase_end - info->region_end; + if (flashctx->chip->read(flashctx, backup_contents + rel_start, start, len)) { + msg_cerr("Can't read! Aborting.\n"); + goto _free_ret; + } + } + } + + ret = 1; all_skipped = false;
msg_cdbg("E"); if (erasefn(flashctx, info->erase_start, erase_len)) - return 1; + goto _free_ret; if (check_erased_range(flashctx, info->erase_start, erase_len)) { msg_cerr("ERASE FAILED!\n"); - return 1; + goto _free_ret; } - return 0; + + if (region_unaligned) { + unsigned int starthere = 0, lenhere = 0, writecount = 0; + /* get_next_write() sets starthere to a new value after the call. */ + while ((lenhere = get_next_write(erased_contents + starthere, backup_contents + starthere, + erase_len - starthere, &starthere, flashctx->chip->gran))) { + if (!writecount++) + msg_cdbg("W"); + /* Needs the partial write function signature. */ + if (flashctx->chip->write(flashctx, backup_contents + starthere, + info->erase_start + starthere, lenhere)) + goto _free_ret; + starthere += lenhere; + } + } + + ret = 0; + +_free_ret: + free(erased_contents); + free(backup_contents); + return ret; }
/**
Hello Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/31069
to look at the new patch set (#2).
Change subject: Fix erasing of unaligned regions ......................................................................
Fix erasing of unaligned regions
The erase (-E) feature is somehow a brute force method, but still, if we are given a region to erase, we should make sure to restore surrounding data if the erase block expands beyond the region.
Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Signed-off-by: Nico Huber nico.huber@secunet.com --- M flashrom.c 1 file changed, 66 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/31069/2
Hello Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/31069
to look at the new patch set (#3).
Change subject: Fix erasing of unaligned regions ......................................................................
Fix erasing of unaligned regions
The erase (-E) feature is somehow a brute force method, but still, if we are given a region to erase, we should make sure to restore surrounding data if the erase block expands beyond the region.
Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Signed-off-by: Nico Huber nico.huber@secunet.com --- M flashrom.c 1 file changed, 68 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/31069/3
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/31069 )
Change subject: Fix erasing of unaligned regions ......................................................................
Patch Set 3: Code-Review+2
Already reviewed for master branch with a slight alteration: Guarding free() from null-pointers to be regression safe.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/31069 )
Change subject: Fix erasing of unaligned regions ......................................................................
Patch Set 3: -Code-Review
Ok, I'll use vim :-P
Hello Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/31069
to look at the new patch set (#4).
Change subject: Fix erasing of unaligned regions ......................................................................
Fix erasing of unaligned regions
The erase (-E) feature is somehow a brute force method, but still, if we are given a region to erase, we should make sure to restore surrounding data if the erase block expands beyond the region.
Slight alteration from `master` commit: Guard free() from NULL pointers to be regression safe even in case of broken libc.
Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Signed-off-by: Nico Huber nico.huber@secunet.com --- M flashrom.c 1 file changed, 68 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/31069/4
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/31069 )
Change subject: Fix erasing of unaligned regions ......................................................................
Patch Set 4: Code-Review+2
Already reviewed for master branch with a slight alteration: Guarding free() from null-pointers to be regression safe.
Another shot.
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/31069 )
Change subject: Fix erasing of unaligned regions ......................................................................
Fix erasing of unaligned regions
The erase (-E) feature is somehow a brute force method, but still, if we are given a region to erase, we should make sure to restore surrounding data if the erase block expands beyond the region.
Slight alteration from `master` commit: Guard free() from NULL pointers to be regression safe even in case of broken libc.
Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Signed-off-by: Nico Huber nico.huber@secunet.com Reviewed-on: https://review.coreboot.org/c/31069 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M flashrom.c 1 file changed, 68 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/flashrom.c b/flashrom.c index 1a7dfb1..51f9721 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1690,17 +1690,82 @@ const struct walk_info *const info, const erasefn_t erasefn) { const unsigned int erase_len = info->erase_end + 1 - info->erase_start; + const bool region_unaligned = info->region_start > info->erase_start || + info->erase_end > info->region_end; + uint8_t *backup_contents = NULL, *erased_contents = NULL; + int ret = 2;
+ /* + * If the region is not erase-block aligned, merge current flash con- + * tents into a new buffer `backup_contents`. + */ + if (region_unaligned) { + backup_contents = malloc(erase_len); + erased_contents = malloc(erase_len); + if (!backup_contents || !erased_contents) { + msg_cerr("Out of memory!\n"); + ret = 1; + goto _free_ret; + } + memset(backup_contents, 0xff, erase_len); + memset(erased_contents, 0xff, erase_len); + + msg_cdbg("R"); + /* Merge data preceding the current region. */ + if (info->region_start > info->erase_start) { + const chipoff_t start = info->erase_start; + const chipsize_t len = info->region_start - info->erase_start; + if (flashctx->chip->read(flashctx, backup_contents, start, len)) { + msg_cerr("Can't read! Aborting.\n"); + goto _free_ret; + } + } + /* Merge data following the current region. */ + if (info->erase_end > info->region_end) { + const chipoff_t start = info->region_end + 1; + const chipoff_t rel_start = start - info->erase_start; /* within this erase block */ + const chipsize_t len = info->erase_end - info->region_end; + if (flashctx->chip->read(flashctx, backup_contents + rel_start, start, len)) { + msg_cerr("Can't read! Aborting.\n"); + goto _free_ret; + } + } + } + + ret = 1; all_skipped = false;
msg_cdbg("E"); if (erasefn(flashctx, info->erase_start, erase_len)) - return 1; + goto _free_ret; if (check_erased_range(flashctx, info->erase_start, erase_len)) { msg_cerr("ERASE FAILED!\n"); - return 1; + goto _free_ret; } - return 0; + + if (region_unaligned) { + unsigned int starthere = 0, lenhere = 0, writecount = 0; + /* get_next_write() sets starthere to a new value after the call. */ + while ((lenhere = get_next_write(erased_contents + starthere, backup_contents + starthere, + erase_len - starthere, &starthere, flashctx->chip->gran))) { + if (!writecount++) + msg_cdbg("W"); + /* Needs the partial write function signature. */ + if (flashctx->chip->write(flashctx, backup_contents + starthere, + info->erase_start + starthere, lenhere)) + goto _free_ret; + starthere += lenhere; + } + } + + ret = 0; + +_free_ret: + if (erased_contents != NULL) + free(erased_contents); + if (backup_contents != NULL) + free(backup_contents); + return ret; }
/**
-
Nico Huber (Code Review)