July 2017 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[staging]: rpci: Use pci_dev struct pointer to avoid API breaks
by build bot (Jenkins) (Code Review) 27 Jul '17

27 Jul '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/20784 ) Change subject: rpci: Use pci_dev struct pointer to avoid API breaks ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/448/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/478/ : SUCCESS -- To view, visit https://review.coreboot.org/20784 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Gerrit-Change-Number: 20784 Gerrit-PatchSet: 2 Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 27 Jul 2017 18:42:17 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: rpci: Use pci_dev struct pointer to avoid API breaks
by Youness Alaoui (Code Review) 27 Jul '17

27 Jul '17

Hello David Hendricks, Paul Menzel, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/20784 to look at the new patch set (#2). Change subject: rpci: Use pci_dev struct pointer to avoid API breaks ...................................................................... rpci: Use pci_dev struct pointer to avoid API breaks The pci_dev structure is never meant to be used as is, but always as a pointer. By using the struct itself in undo_pci_write_data, we are risking data corruption, or buffer overflows if the structure size changes. This is especially apparent on my system where flashrom segfaults because I compile it with pciutils 3.3.0 and I run it on a system with pciutils 3.5.2. The struture size is different and causes a struct with the wrong size to be sent to the library, with invalid internal field values. This has been discovered and discussed in Change ID 18925 [1] [1] https://review.coreboot.org/#/c/18925/ Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Signed-off-by: Youness Alaoui <kakaroto(a)kakaroto.homelinux.net> --- M pcidev.c M programmer.h 2 files changed, 16 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/84/20784/2 -- To view, visit https://review.coreboot.org/20784 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Gerrit-Change-Number: 20784 Gerrit-PatchSet: 2 Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: chipset_enable: Add support for Intel Skylake / Kabylake
by Nico Huber (Code Review) 27 Jul '17

27 Jul '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/18925 ) Change subject: chipset_enable: Add support for Intel Skylake / Kabylake ...................................................................... Patch Set 8: > The RPCI fix is pushed here : https://review.coreboot.org/#/c/20784/ > Now all we need is for this patch to set the 'pacc' variable and > restore it at the end of the function. Um, I don't see why I have to wait for a seven years old bug to get fixed before I add another chipset... also, if we fix it here and not in your patch, we'll lose the explicit information that the changes are related (e.g. if anyone will ever fix the rpci interface he'll more likely miss to remove the workaround here). -- To view, visit https://review.coreboot.org/18925 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I000819aff25fbe9764f33df85f040093b82cd948 Gerrit-Change-Number: 18925 Gerrit-PatchSet: 8 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 27 Jul 2017 11:32:26 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: rpci: Use pci_dev struct pointer to avoid API breaks
by Nico Huber (Code Review) 27 Jul '17

27 Jul '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/20784 ) Change subject: rpci: Use pci_dev struct pointer to avoid API breaks ...................................................................... Patch Set 1: (3 comments) Thanks. https://review.coreboot.org/#/c/20784/1/pcidev.c File pcidev.c: https://review.coreboot.org/#/c/20784/1/pcidev.c@262 PS1, Line 262: struct pci_dev * dev; No whitespace after the asterisk, please. https://review.coreboot.org/#/c/20784/1/pcidev.c@297 PS1, Line 297: Add a comment that `dev` passed to rpci_* has to be compatible to `pacc`. Here and or in the header file? https://review.coreboot.org/#/c/20784/1/pcidev.c@307 PS1, Line 307: a->domain, a->bus, a->dev, a->func); \ Bail out if `undo_pci_write_data->dev` is NULL or check it in undo_pci_write(). -- To view, visit https://review.coreboot.org/20784 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Gerrit-Change-Number: 20784 Gerrit-PatchSet: 1 Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 27 Jul 2017 09:06:41 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: rpci: Use pci_dev struct pointer to avoid API breaks
by David Hendricks (Code Review) 27 Jul '17

27 Jul '17

David Hendricks has posted comments on this change. ( https://review.coreboot.org/20784 ) Change subject: rpci: Use pci_dev struct pointer to avoid API breaks ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/20784 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Gerrit-Change-Number: 20784 Gerrit-PatchSet: 1 Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 26 Jul 2017 23:08:05 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: chipset_enable: Add support for Intel Skylake / Kabylake
by Youness Alaoui (Code Review) 27 Jul '17

27 Jul '17

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/18925 ) Change subject: chipset_enable: Add support for Intel Skylake / Kabylake ...................................................................... Patch Set 8: The RPCI fix is pushed here : https://review.coreboot.org/#/c/20784/ Now all we need is for this patch to set the 'pacc' variable and restore it at the end of the function. -- To view, visit https://review.coreboot.org/18925 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I000819aff25fbe9764f33df85f040093b82cd948 Gerrit-Change-Number: 18925 Gerrit-PatchSet: 8 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 26 Jul 2017 22:57:56 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: rpci: Use pci_dev struct pointer to avoid API breaks
by build bot (Jenkins) (Code Review) 27 Jul '17

27 Jul '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/20784 ) Change subject: rpci: Use pci_dev struct pointer to avoid API breaks ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/447/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/477/ : SUCCESS -- To view, visit https://review.coreboot.org/20784 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Gerrit-Change-Number: 20784 Gerrit-PatchSet: 1 Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 26 Jul 2017 22:40:16 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: rpci: Use pci_dev struct pointer to avoid API breaks
by Youness Alaoui (Code Review) 27 Jul '17

27 Jul '17

Youness Alaoui has uploaded this change for review. ( https://review.coreboot.org/20784 Change subject: rpci: Use pci_dev struct pointer to avoid API breaks ...................................................................... rpci: Use pci_dev struct pointer to avoid API breaks The pci_dev structure is never meant to be used as is, but always as a pointer. By using the struct itself in undo_pci_write_data, we are risking data corruption, or buffer overflows if the structure size changes. This is especially apparent on my system where flashrom segfaults because I compile it with pciutils 3.3.0 and I run it on a system with pciutils 3.5.2. The struture size is different and causes a struct with the wrong size to be sent to the library, with invalid internal field values. This has been discovered and discussed in Change ID 18925 [1] [1] https://review.coreboot.org/#/c/18925/ Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Signed-off-by: Youness Alaoui <kakaroto(a)kakaroto.homelinux.net> --- M pcidev.c 1 file changed, 7 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/84/20784/1 diff --git a/pcidev.c b/pcidev.c index 2c78063..7a211e8 100644 --- a/pcidev.c +++ b/pcidev.c @@ -259,7 +259,7 @@ }; struct undo_pci_write_data { - struct pci_dev dev; + struct pci_dev * dev; int reg; enum pci_write_type type; union { @@ -278,16 +278,16 @@ return 1; } msg_pdbg("Restoring PCI config space for %02x:%02x:%01x reg 0x%02x\n", - data->dev.bus, data->dev.dev, data->dev.func, data->reg); + data->dev->bus, data->dev->dev, data->dev->func, data->reg); switch (data->type) { case pci_write_type_byte: - pci_write_byte(&data->dev, data->reg, data->bytedata); + pci_write_byte(data->dev, data->reg, data->bytedata); break; case pci_write_type_word: - pci_write_word(&data->dev, data->reg, data->worddata); + pci_write_word(data->dev, data->reg, data->worddata); break; case pci_write_type_long: - pci_write_long(&data->dev, data->reg, data->longdata); + pci_write_long(data->dev, data->reg, data->longdata); break; } /* p was allocated in register_undo_pci_write. */ @@ -303,7 +303,8 @@ msg_gerr("Out of memory!\n"); \ exit(1); \ } \ - undo_pci_write_data->dev = *a; \ + undo_pci_write_data->dev = pci_get_dev(pacc, \ + a->domain, a->bus, a->dev, a->func); \ undo_pci_write_data->reg = b; \ undo_pci_write_data->type = pci_write_type_##c; \ undo_pci_write_data->c##data = pci_read_##c(dev, reg); \ -- To view, visit https://review.coreboot.org/20784 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newchange Gerrit-Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Gerrit-Change-Number: 20784 Gerrit-PatchSet: 1 Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com>

1 0

Change in flashrom[staging]: ichspi: Add support for Intel Skylake
by David Hendricks (Code Review) 26 Jul '17

26 Jul '17

David Hendricks has posted comments on this change. ( https://review.coreboot.org/18962 ) Change subject: ichspi: Add support for Intel Skylake ...................................................................... Patch Set 10: Code-Review+2 -- To view, visit https://review.coreboot.org/18962 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I0f4565a3c39f5fe3aec4fc8863605cebed1ad4ee Gerrit-Change-Number: 18962 Gerrit-PatchSet: 10 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 26 Jul 2017 21:54:47 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: [NFM] Convert flashrom to git
by Patrick Georgi (Code Review) 14 Jul '17

14 Jul '17

Patrick Georgi has abandoned this change. ( https://review.coreboot.org/18812 ) Change subject: [NFM] Convert flashrom to git ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/18812 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: abandon Gerrit-Change-Id: I49e4cca56938b9f945de2ccd2d0a4ec6acdb0a30 Gerrit-Change-Number: 18812 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit July 2017