Change in ...flashrom[1.0.x]: Fix erasing of unaligned regions
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; } /** -- To view, visit https://review.coreboot.org/c/flashrom/+/31069 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Gerrit-Change-Number: 31069 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: newchange
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 -- To view, visit https://review.coreboot.org/c/flashrom/+/31069 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Gerrit-Change-Number: 31069 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
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 -- To view, visit https://review.coreboot.org/c/flashrom/+/31069 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Gerrit-Change-Number: 31069 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
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. -- To view, visit https://review.coreboot.org/c/flashrom/+/31069 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Gerrit-Change-Number: 31069 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Mon, 04 Mar 2019 14:53:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
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 -- To view, visit https://review.coreboot.org/c/flashrom/+/31069 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Gerrit-Change-Number: 31069 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Mon, 04 Mar 2019 14:55:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
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 -- To view, visit https://review.coreboot.org/c/flashrom/+/31069 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Gerrit-Change-Number: 31069 Gerrit-PatchSet: 4 Gerrit-Owner: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
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. -- To view, visit https://review.coreboot.org/c/flashrom/+/31069 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Gerrit-Change-Number: 31069 Gerrit-PatchSet: 4 Gerrit-Owner: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Mon, 04 Mar 2019 15:02:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
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; } /** -- To view, visit https://review.coreboot.org/c/flashrom/+/31069 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I5fc35310f0b090f218cd1d660e27fb39dd05c3c5 Gerrit-Change-Number: 31069 Gerrit-PatchSet: 5 Gerrit-Owner: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: merged
participants (1)
-
Nico Huber (Code Review)