Dasharo develop #2
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Change-Id: Ifcb3a63a2386b9622e1e4bf3f77f0334136686fe Signed-off-by: Michał Kopeć <michal.kopec@3mdeb.com>
Add initial ADL-S internal programmer support
SergiiDmytruk
pushed a commit
that referenced
this pull request
Jul 26, 2022
This change addresses the following ASAN error detected in the chromium tree: * ASAN error detected: * ================================================================= * ==12==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55a8a046c916 at pc 0x55a8a038a21d bp 0x7ffd5dbc9ed0 sp 0x7ffd5dbc9ec8 * READ of size 2 at 0x55a8a046c916 thread T0 * #0 0x55a8a038a21c in nicrealtek_init /build/amd64-generic/tmp/por tage/sys-apps/flashrom-9999/work/flashrom-9999-build/../flashrom-9999/ni crealtek.c:119:15 * #1 0x55a8a032f172 in __sanitizer::BufferedStackTrace::UnwindImpl( unsigned long, unsigned long, void*, bool, unsigned int) ??:0:0 * #2 0x55a8a02b65b8 in __asan::ErrorGeneric::Print() ??:0:0 * #3 0x55a8a03294d5 in __asan::ScopedInErrorReport::~ScopedInErrorR eport() ??:0:0 * #4 0x55a8a032c5ae in __asan::ReportGenericError(unsigned long, un signed long, unsigned long, unsigned long, bool, unsigned long, unsigned int, bool) ??:0:0 * #5 0x55a8a032d0f7 in __asan_report_load2 ??:0:0 * * 0x55a8a046c916 is located 18 bytes to the right of global variable 'm ock_pci_dev' defined in '../flashrom-9999/tests/tests.c:50:16' (0x55a8a0 46c900) of size 4 * SUMMARY: AddressSanitizer: global-buffer-overflow (/tmp/portage/sys-a pps/flashrom-9999/work/flashrom-9999-build/tests/flashrom_unit_tests+0x1 9a21c) * Shadow bytes around the buggy address: * 0x0ab5940858d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 * 0x0ab5940858e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 * 0x0ab5940858f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 * 0x0ab594085900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 * 0x0ab594085910: 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 f9 00 00 * =>0x0ab594085920: 04 f9[f9]f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 * 0x0ab594085930: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 * 0x0ab594085940: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 * 0x0ab594085950: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 * 0x0ab594085960: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 * 0x0ab594085970: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 * Shadow byte legend (one shadow byte represents 8 application bytes): * Addressable: 00 * Partially addressable: 01 02 03 04 05 06 07 * Heap left redzone: fa * Freed heap region: fd * Stack left redzone: f1 * Stack mid redzone: f2 * Stack right redzone: f3 * Stack after return: f5 * Stack use after scope: f8 * Global redzone: f9 * Global init order: f6 * Poisoned by user: f7 * Container overflow: fc * Array cookie: ac * Intra object redzone: bb * ASan internal: fe * Left alloca redzone: ca * Right alloca redzone: cb * ==12==ABORTING BUG=b:224828279 TEST=./test_build.sh; FEATURES=test emerge-amd64-generic flashrom BRANCH=none Signed-off-by: Daniel Campello <campello@chromium.org> Change-Id: I47943bf70181a9041f287df3ece0f7067a112de8 Reviewed-on: https://review.coreboot.org/c/flashrom/+/62845 Reviewed-by: Anastasia Klimchuk <aklm@chromium.org> Reviewed-by: Edward O'Callaghan <quasisec@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
macpijan
pushed a commit
that referenced
this pull request
Aug 9, 2024
Waiting a full second is a very long time, especially when default_delay() chooses to busy-loop. This code has been around for a decade, with vague references to user reports: commit 8ab49e7 Date: Wed Aug 19 13:55:34 2009 +0000 Disallow erase/write for known bad chips so people won't try without a clear understanding Still, this logic does not belong in the high-level library logic, used by all programmers and all chips. If there is a timing issue, it should either be encoded in the appropriate programmer or flashchip timing. However, we don't really know what chips were in use, as the above commit doesn't have any links to reports. So in a feeble attempt at avoiding breaking users here, we also surmise that... * SPI chips weren't all that common in 2009; * I'm mostly motivated by flashrom performance on Chromebooks, were SPI chips (and linux_mtd / BUS_PROG) are the rule; and * SPI chips have precise timing requirements and an appropriate BUSY status. So we guess that this "calm down" magic delay wouldn't be necessary there. Thus, we allow this magic delay only on non-SPI (and non-BUG_PROG, used by linux_mtd for one) buses as a compromise. Now, this change has some (hopefully [1] tiny) chance of regression, so we have the following considerations: 1. emergency_help_message() already provides documentation on how to contact support, in case we need to handle any user-reported regressions. 2. If there is any regression here, it's only in the --verify code; so we can always provide workarounds for testing this, to determine whether this change may have been at fault. For example, something like: flashrom --write /my/new/image.bin --noverify sleep 1 flashrom --read /tmp/bar.bin cmp /my/new/image.bin /tmp/bar.bin If such problems occur, we can collect system/programmer/chip info to try to encode a more targeted delay into the appropriate chip/programmer implementation, and avoid penalizing the entire project like this. 3. We already have (embedded in erase_write()) erase verification that performs no such delay. So depending on the type of timing error that this delay was attempting to cover, we may have some proof that this delay is no longer necessary (or at least, that whatever systems were needing this delay in the first place are no longer caring about flashrom). 4. We've retained the delay for buses that were likely common in 2009 (per the above "feeble attempt"). NB: I avoid using the BUS_NONSPI macro, because I want to exclude any future buses from this workaround, even in the event that the BUS_NONSPI category grows in the future. [1] Famous last words. BUG=b:325539928 TEST=`flashrom_tester --flashrom_binary=$(which flashrom) \ internal Erase_and_Write Fail_to_verify`, TEST=`vpd -i RW_VPD -s foo=bar; vpd -i RW_VPD -l; \ vpd -i RW_VPD -d foo; vpd -i RW_VPD -l` TEST=`elogtool list; elogtool add 0xa7; elogtool list` on (at least) 2 systems: #1: Kukui/Kakadu rev2 - MTD programmer / kernel 5.10.215-24542-g0515a679eb42 / CrOS ~ 15857.0.0 #2: Zork/Dirinboz rev2 - chip name: vendor="Winbond" name="W25Q128.JW.DTR" / BIOS: Google_Dirinboz.13434.688.0 / kernel 5.4.267-21940-g67f70e251a74 / CrOS ~ 15753.43.0 Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc Signed-off-by: Brian Norris <briannorris@chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/80807 Reviewed-by: Peter Marheine <pmarheine@chromium.org> Reviewed-by: Anastasia Klimchuk <aklm@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.