Change in flashrom[master]: it85spi.c: Kill off dead code system() hack
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/46547 ) Change subject: it85spi.c: Kill off dead code system() hack ...................................................................... it85spi.c: Kill off dead code system() hack This is dead code sitting under a 'if 0' and is extremely hacky. One could argue this patch perhaps should include a update to documentation to stop powerd instead of keeping this rot around? BUG=none BRANCH=none TEST=builds Change-Id: I5761dea48d553b1334eef3c9cf0ad79cedbd7806 Signed-off-by: Edward O'Callaghan <quasisec@google.com> --- M it85spi.c 1 file changed, 0 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/46547/1 diff --git a/it85spi.c b/it85spi.c index b998790..1d93a5b 100644 --- a/it85spi.c +++ b/it85spi.c @@ -114,15 +114,6 @@ if (it85xx_scratch_rom_reenter > 0) return; -#if 0 - /* FIXME: this a workaround for the bug that SMBus signal would - * interfere the EC firmware update. Should be removed if - * we find out the root cause. */ - ret = system("stop powerd >&2"); - if (ret) - msg_perr("Cannot stop powerd.\n"); -#endif - for (tries = 0; tries < MAX_TRY; ++tries) { /* Wait until IBF (input buffer) is not full. */ if (wait_for(KB_IBF, 0, MAX_TIMEOUT, @@ -165,9 +156,6 @@ static void it85xx_exit_scratch_rom(void) { -#if 0 - int ret; -#endif int tries; msg_pdbg("%s():%d was called ...\n", __func__, __LINE__); @@ -204,15 +192,6 @@ } else { msg_perr("%s():%d * Max try reached.\n", __func__, __LINE__); } - -#if 0 - /* FIXME: this a workaround for the bug that SMBus signal would - * interfere the EC firmware update. Should be removed if - * we find out the root cause. */ - ret = system("start powerd >&2"); - if (ret) - msg_perr("Cannot start powerd again.\n"); -#endif } static int it85xx_shutdown(void *data) -- To view, visit https://review.coreboot.org/c/flashrom/+/46547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5761dea48d553b1334eef3c9cf0ad79cedbd7806 Gerrit-Change-Number: 46547 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: newchange
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46547 ) Change subject: it85spi.c: Kill off dead code system() hack ...................................................................... Patch Set 1: Code-Review-1 -- To view, visit https://review.coreboot.org/c/flashrom/+/46547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5761dea48d553b1334eef3c9cf0ad79cedbd7806 Gerrit-Change-Number: 46547 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Mon, 19 Oct 2020 09:17:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46547 ) Change subject: it85spi.c: Kill off dead code system() hack ...................................................................... Patch Set 1: I once tried it85spi on an ITE IT8502E EC, just probing made the laptop power off after a few seconds. -- To view, visit https://review.coreboot.org/c/flashrom/+/46547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5761dea48d553b1334eef3c9cf0ad79cedbd7806 Gerrit-Change-Number: 46547 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Mon, 19 Oct 2020 09:23:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46547 ) Change subject: it85spi.c: Kill off dead code system() hack ...................................................................... Patch Set 1:
Patch Set 1:
I once tried it85spi on an ITE IT8502E EC, just probing made the laptop power off after a few seconds.
eek yea ok. Thanks for the extra info we should perhaps try to fix it once I can recover time from Chromium deforking. Sounds like we should put a warning in the man page? -- To view, visit https://review.coreboot.org/c/flashrom/+/46547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5761dea48d553b1334eef3c9cf0ad79cedbd7806 Gerrit-Change-Number: 46547 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Mon, 19 Oct 2020 09:47:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46547 ) Change subject: it85spi.c: Kill off dead code system() hack ...................................................................... Patch Set 1:
Patch Set 1:
Patch Set 1:
I once tried it85spi on an ITE IT8502E EC, just probing made the laptop power off after a few seconds.
eek yea ok. Thanks for the extra info we should perhaps try to fix it once I can recover time from Chromium deforking. Sounds like we should put a warning in the man page?
The code doesn't run unless one uncomments a function call somewhere. -- To view, visit https://review.coreboot.org/c/flashrom/+/46547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5761dea48d553b1334eef3c9cf0ad79cedbd7806 Gerrit-Change-Number: 46547 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Mon, 19 Oct 2020 12:13:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46547 ) Change subject: it85spi.c: Kill off dead code system() hack ...................................................................... Patch Set 1: Code-Review+2 While the driver is called IT8502E, it is talking to a very specific version of the EC firmware that was running on a very specific set of machines that used to have this chip. Chances are that none of these machines are in operation anymore, and if they are, they are most likely no longer relevant. But that goes for a lot of the code in coreboot / flashrom et all. Angel, chances are also, that your behavior is caused by one of two things happening: 1. the CPU is catching IO writes to the IO ports in question and, because flashrom does not pass the consistency checks for these accesses, an SMI reboots the machine. 2. the IT8502 firmware on your machine gets a command that it does not understand, because it's not running the firmware from the same ODM as the IT8502 that this driver was developed against, and the EC decides that because someone is sending potentially evil commands, the EC pulls the AP's reset line to stop the behavior. In a lot of these older systems there is no actual security strategy in the software design, and so the only way to keep the device safe is by obscurity. To prevent people from reverse engineering the project, rate limiting the access through resets is a somewhat reasonable method (hu hu). I bet that we could remove the it85spi driver from flashrom all together and no person or entity would actually ever care. Of course we might want to decide against it for educational reasons. -- To view, visit https://review.coreboot.org/c/flashrom/+/46547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5761dea48d553b1334eef3c9cf0ad79cedbd7806 Gerrit-Change-Number: 46547 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 03 Nov 2020 02:26:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46547 ) Change subject: it85spi.c: Kill off dead code system() hack ...................................................................... Patch Set 1: -Code-Review -- To view, visit https://review.coreboot.org/c/flashrom/+/46547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5761dea48d553b1334eef3c9cf0ad79cedbd7806 Gerrit-Change-Number: 46547 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 03 Nov 2020 02:26:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46547 ) Change subject: it85spi.c: Kill off dead code system() hack ...................................................................... Patch Set 1:
Patch Set 1: Code-Review+2
While the driver is called IT8502E, it is talking to a very specific version of the EC firmware that was running on a very specific set of machines that used to have this chip.
Chances are that none of these machines are in operation anymore, and if they are, they are most likely no longer relevant. But that goes for a lot of the code in coreboot / flashrom et all.
Angel, chances are also, that your behavior is caused by one of two things happening:
1. the CPU is catching IO writes to the IO ports in question and, because flashrom does not pass the consistency checks for these accesses, an SMI reboots the machine.
It doesn't reboot the machine, it just powers off.
2. the IT8502 firmware on your machine gets a command that it does not understand, because it's not running the firmware from the same ODM as the IT8502 that this driver was developed against, and the EC decides that because someone is sending potentially evil commands, the EC pulls the AP's reset line to stop the behavior. In a lot of these older systems there is no actual security strategy in the software design, and so the only way to keep the device safe is by obscurity. To prevent people from reverse engineering the project, rate limiting the access through resets is a somewhat reasonable method (hu hu).
I doubt the firmware on that device has any consideration w.r.t. security. I still need to look at it more closely, but I'm getting buried under more and more computers.
I bet that we could remove the it85spi driver from flashrom all together and no person or entity would actually ever care. Of course we might want to decide against it for educational reasons.
If anything, we would need to ensure we're only trying to use the code on whitelisted machines. Currently, there's no whitelist and the call to the it85spi code is commented-out, sooo... it doesn't make a difference. It's still being build-tested, though, so it's not as rotten as it could be. -- To view, visit https://review.coreboot.org/c/flashrom/+/46547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5761dea48d553b1334eef3c9cf0ad79cedbd7806 Gerrit-Change-Number: 46547 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 03 Nov 2020 11:23:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/46547?usp=email ) Change subject: it85spi.c: Kill off dead code system() hack ...................................................................... Abandoned it85spi has been deleted -- To view, visit https://review.coreboot.org/c/flashrom/+/46547?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I5761dea48d553b1334eef3c9cf0ad79cedbd7806 Gerrit-Change-Number: 46547 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-MessageType: abandon
participants (4)
-
Anastasia Klimchuk (Code Review) -
Angel Pons (Code Review)
-
Edward O'Callaghan (Code Review)
-
Stefan Reinauer (Code Review)