Attention is currently required from: Angel Pons.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62747
to look at the new patch set (#4).
Change subject: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
......................................................................
flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
In flashrom_image_write variables curcontents …
[View More]and oldcontents are
dynamically allocated using malloc. These could remain uninitialized and
when later used in need_erase could result in undefined behaviour.
Similiar reasoning for variables hbuf, hdrs, tbuf in function
prob_spi_sfdp. So allocate them using calloc to initialize them to
zeroes or if allocating using malloc separately initialize them using a loop.
Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M flashrom.c
M sfdp.c
2 files changed, 5 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/62747/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/62747
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab
Gerrit-Change-Number: 62747
Gerrit-PatchSet: 4
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
[View Less]
Attention is currently required from: Paul Menzel, Angel Pons.
Hello build bot (Jenkins), Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62725
to look at the new patch set (#10).
Change subject: libflashrom.c: Fix unintialized value passed to function
......................................................................
libflashrom.c: Fix unintialized value passed to function
In function flash_layout_read_from_ifd …
[View More]variable chip_layout remains
uninitialized if prepare_flash_access returns false. This uninitialized
value is passed to flashrom_layout_release later.
Change-Id: Iacbd7bf9cdf897cc2a732c1dc6568845a4ab804d
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
Signed-off-by: aarya <aarya.chaumal(a)gmail.com>
---
M libflashrom.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/62725/10
--
To view, visit https://review.coreboot.org/c/flashrom/+/62725
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iacbd7bf9cdf897cc2a732c1dc6568845a4ab804d
Gerrit-Change-Number: 62725
Gerrit-PatchSet: 10
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
[View Less]
Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62761 )
Change subject: board_enable.c: Fix dead assignment
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I don't this change should affect the logic in any way but still wondering why were there 2 assignments in the first place
--
To view, visit https://review.coreboot.org/c/flashrom/+/62761
To unsubscribe, or for help writing mail …
[View More]filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc
Gerrit-Change-Number: 62761
Gerrit-PatchSet: 1
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sat, 12 Mar 2022 18:47:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
[View Less]
Light has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62761 )
Change subject: board_enable.c: Fix dead assignment
......................................................................
board_enable.c: Fix dead assignment
In function board_asus_p3b_f there were two consecutive lines which
modified the value of variable b -- b=INB(0x80);b=INB(smbba);. Since the
value of b is not used after first assignment I have removed that
assignment.
Change-Id: …
[View More]I7458b416a69fd5e2aa300ca49d1352b6074ad0bc
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M board_enable.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/62761/1
diff --git a/board_enable.c b/board_enable.c
index 0ac522e..c00caf3 100644
--- a/board_enable.c
+++ b/board_enable.c
@@ -910,7 +910,7 @@
/* Wait until SMBus transaction is complete. */
b = 0x1;
while (b & 0x01) {
- b = INB(0x80);
+ INB(0x80);
b = INB(smbba);
}
--
To view, visit https://review.coreboot.org/c/flashrom/+/62761
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc
Gerrit-Change-Number: 62761
Gerrit-PatchSet: 1
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newchange
[View Less]
Attention is currently required from: Nico Huber.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61276 )
Change subject: hwaccess_x86_io: clean header concept
......................................................................
Patch Set 16:
(2 comments)
Patchset:
PS15:
> Oh, and please update the commit message wrt. the test status.
It restores old behavior; don't know the current build status for Android.
File hwaccess_x86_io.c:
…
[View More]https://review.coreboot.org/c/flashrom/+/61276/comment/e20d8a9c_48892f7a
PS15, Line 31: * - USE_LIBC_LAST
: * - USE_LIBC_FIRST
> Highlighted the identifiers in my editor, and taadaa the most errors are […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/61276
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3
Gerrit-Change-Number: 61276
Gerrit-PatchSet: 16
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Sat, 12 Mar 2022 17:42:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
[View Less]
Attention is currently required from: Thomas Heijligen.
Hello build bot (Jenkins), Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/61276
to look at the new patch set (#16).
Change subject: hwaccess_x86_io: clean header concept
......................................................................
hwaccess_x86_io: clean header concept
Move all function implementations into the .c file
TEST: `[g]make [WARNERROR=no]` on …
[View More]Linux, FreeBSD, NetBSD, OpenBSD,
DragonflyBSD, OpenIndiana, Debian-GNU/Hurd
Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
---
M Makefile
M hwaccess_x86_io.c
M hwaccess_x86_io.h
M meson.build
4 files changed, 315 insertions(+), 188 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/61276/16
--
To view, visit https://review.coreboot.org/c/flashrom/+/61276
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3
Gerrit-Change-Number: 61276
Gerrit-PatchSet: 16
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newpatchset
[View Less]
Attention is currently required from: Felix Singer.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62196 )
Change subject: Makefile: use HAS_ USE_ pattern for serial support
......................................................................
Patch Set 4:
(1 comment)
File Makefile:
https://review.coreboot.org/c/flashrom/+/62196/comment/266b18d8_82745fd7
PS4, Line 253: HAS_SERIAL := $(strip $(if $(filter $(TARGET_OS), DOS …
[View More]libpayload), no, yes))
> Only if you want to :) and probably something for another commit, as this wasn't […]
Then I'll leave it as it is for now.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62196
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ica951e76d6362b01f09d23a729a2a6049e7f0b66
Gerrit-Change-Number: 62196
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Comment-Date: Sat, 12 Mar 2022 16:51:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
[View Less]
Attention is currently required from: Felix Singer, Thomas Heijligen.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62196 )
Change subject: Makefile: use HAS_ USE_ pattern for serial support
......................................................................
Patch Set 4:
(1 comment)
File Makefile:
https://review.coreboot.org/c/flashrom/+/62196/comment/26896cc1_34a08ee3
PS4, Line 253: HAS_SERIAL := $(strip $(if $(filter $(TARGET_OS), …
[View More]DOS libpayload), no, yes))
> Yes, DEPENDS_ON_SOCKETS is a subset of DEPENDS_OF_SERIAL. […]
Only if you want to :) and probably something for another commit, as this wasn't
handled explicilty before either.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62196
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ica951e76d6362b01f09d23a729a2a6049e7f0b66
Gerrit-Change-Number: 62196
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Sat, 12 Mar 2022 16:48:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
[View Less]
Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/61275 )
Change subject: hwaccess_x86_io: refactor rget_io_perms()
......................................................................
hwaccess_x86_io: refactor rget_io_perms()
Abstract the different I/O Port permission methods in own functions.
Change-Id: If4b2f8c2532f3732086ee1d479da6ae6693f9a42
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
Reviewed-on: https://review.coreboot.org/c/…
[View More]flashrom/+/61275
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
---
M hwaccess_x86_io.c
1 file changed, 98 insertions(+), 50 deletions(-)
Approvals:
build bot (Jenkins): Verified
Nico Huber: Looks good to me, approved
Angel Pons: Looks good to me, but someone else must approve
Edward O'Callaghan: Looks good to me, approved
diff --git a/hwaccess_x86_io.c b/hwaccess_x86_io.c
index 635b27e..e891d98 100644
--- a/hwaccess_x86_io.c
+++ b/hwaccess_x86_io.c
@@ -2,6 +2,8 @@
* This file is part of the flashrom project.
*
* Copyright (C) 2009,2010 Carl-Daniel Hailfinger
+ * Copyright (C) 2022 secunet Security Networks AG
+ * (Written by Thomas Heijligen <thomas.heijligen(a)secunet.com)
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -14,70 +16,116 @@
* GNU General Public License for more details.
*/
+/*
+ * This file implements x86 I/O port permission handling.
+ *
+ * For this purpose the platform specific code is encapsuled in two functions
+ * and compiled only on the respective platform. `platform_get_io_perms()` is
+ * called to get the permissions and `platform_release_io_perms()` is called for
+ * releasing those.
+ */
+
#include <errno.h>
#include <string.h>
-#if !defined (__DJGPP__) && !defined(__LIBPAYLOAD__)
-/* No file access needed/possible to get hardware access permissions. */
-#include <unistd.h>
-#include <fcntl.h>
-#endif
-#include "hwaccess_x86_io.h"
#include "flash.h"
+#include "hwaccess_x86_io.h"
-#if USE_IOPERM
-#include <sys/io.h>
-#endif
-
-#if USE_DEV_IO
-int io_fd;
-#endif
-
-#if !(defined(__DJGPP__) || defined(__LIBPAYLOAD__))
-static int release_io_perms(void *p)
+#if defined(__DJGPP__) || defined(__LIBPAYLOAD) /* DJGPP, libpayload */
+static int platform_get_io_perms(void)
{
-#if defined (__sun)
+ return 0;
+}
+
+static int platform_release_io_perms(void *p)
+{
+ return 0;
+}
+#endif
+
+#if defined(__sun) /* SunOS */
+#include <sys/sysi86.h>
+
+static int platform_get_io_perms(void)
+{
+ return sysi86(SI86V86, V86SC_IOPL, PS_IOPL);
+}
+
+static int platform_release_io_perms(void *p)
+{
sysi86(SI86V86, V86SC_IOPL, 0);
-#elif USE_DEV_IO
+ return 0;
+}
+#endif
+
+#if USE_DEV_IO /* FreeBSD, DragonFlyBSD */
+#include <fcntl.h>
+#include <unistd.h>
+
+int io_fd;
+
+static int platform_get_io_perms(void)
+{
+ io_fd = open("/dev/io", O_RDWR);
+ return (io_fd >= 0 ? 0 : -1);
+}
+
+static int platform_release_io_perms(void *p)
+{
close(io_fd);
-#elif USE_IOPERM
+ return 0;
+}
+#endif
+
+#if USE_IOPERM /* GNU Hurd */
+#include <sys/io.h>
+
+static int platform_get_io_perms(void)
+{
+ return ioperm(0, 65536, 1);
+}
+
+static int platform_release_io_perms(void *p)
+{
ioperm(0, 65536, 0);
-#elif USE_IOPL
+ return 0;
+}
+#endif
+
+#if USE_IOPL /* Linux, Darwin, NetBSD, OpenBSD */
+static int platform_get_io_perms(void)
+{
+ return iopl(3);
+}
+
+static int platform_release_io_perms(void *p)
+{
iopl(0);
-#endif
return 0;
}
+#endif
-/* Get I/O permissions with automatic permission release on shutdown. */
+/**
+ * @brief Get access to the x86 I/O ports
+ *
+ * This function abstracts the platform dependent function to get access to the
+ * x86 I/O ports and musst be called before using IN[B/W/L] or OUT[B/W/L].
+ * It registers a shutdown function to release those privileges.
+ *
+ * @return 0 on success, platform specific number on failure
+ */
int rget_io_perms(void)
{
- #if defined (__sun)
- if (sysi86(SI86V86, V86SC_IOPL, PS_IOPL) != 0) {
-#elif USE_DEV_IO
- if ((io_fd = open("/dev/io", O_RDWR)) < 0) {
-#elif USE_IOPERM
- if (ioperm(0, 65536, 1) != 0) {
-#elif USE_IOPL
- if (iopl(3) != 0) {
-#endif
- msg_perr("ERROR: Could not get I/O privileges (%s).\n", strerror(errno));
- msg_perr("You need to be root.\n");
- msg_perr("On OpenBSD set securelevel=-1 in /etc/rc.securelevel and\n"
- "reboot, or reboot into single user mode.\n");
- msg_perr("On NetBSD reboot into single user mode or make sure\n"
- "that your kernel configuration has the option INSECURE enabled.\n");
- return 1;
- } else {
- register_shutdown(release_io_perms, NULL);
+ if (platform_get_io_perms() == 0) {
+ register_shutdown(platform_release_io_perms, NULL);
+ return 0;
}
- return 0;
+ msg_perr("ERROR: Could not get I/O privileges (%s).\n", strerror(errno));
+ msg_perr("Make sure you are root. If you are root, your kernel may still\n"
+ "prevent access based on security policies.\n");
+ msg_perr("On OpenBSD set securelevel=-1 in /etc/rc.securelevel and\n"
+ "reboot, or reboot into single user mode.\n");
+ msg_perr("On NetBSD reboot into single user mode or make sure\n"
+ "that your kernel configuration has the option INSECURE enabled.\n");
+ return 1;
}
-
-#else
-
-/* DJGPP and libpayload environments have full PCI port I/O permissions by default. */
-int rget_io_perms(void)
-{
- return 0;
-}
-#endif
--
To view, visit https://review.coreboot.org/c/flashrom/+/61275
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If4b2f8c2532f3732086ee1d479da6ae6693f9a42
Gerrit-Change-Number: 61275
Gerrit-PatchSet: 9
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
[View Less]
Attention is currently required from: Thomas Heijligen.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61276 )
Change subject: hwaccess_x86_io: clean header concept
......................................................................
Patch Set 15:
(1 comment)
Patchset:
PS15:
> Wasn't this fixing Android? That could be added to the commit message.
Oh, and please update the commit message wrt. the test status.
--
To view, visit https://…
[View More]review.coreboot.org/c/flashrom/+/61276
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3
Gerrit-Change-Number: 61276
Gerrit-PatchSet: 15
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Sat, 12 Mar 2022 16:46:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
[View Less]