[PATCH 0/3] Some interface improvements

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

This small patch
* fixes wrong version number in AC_INIT
* adds color to the console output to make it more readable
* enables the user to override kernel args by pressing <a>

Tested with qemu-system-x86_64 + OVMF.

Signed-of-by: Andreas Reichel <andreas.r...@siemens.com>

Andreas Reichel (3):
Add color to make console more readable
Add hot-key to alter bootargs during boot
Fix wrong version in configure.ac

Makefile.am | 1 +
color.c | 21 +++++++++++++++++++++
configure.ac | 2 +-
include/color.h | 24 ++++++++++++++++++++++++
main.c | 28 +++++++++++++++++++++++++++-
5 files changed, 74 insertions(+), 2 deletions(-)
create mode 100644 color.c
create mode 100644 include/color.h

--
2.19.1

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

Makefile.am | 1 +
color.c | 21 +++++++++++++++++++++

include/color.h | 24 ++++++++++++++++++++++++
main.c | 3 +++
4 files changed, 49 insertions(+)

create mode 100644 color.c
create mode 100644 include/color.h

diff --git a/Makefile.am b/Makefile.am
index 74fa7f0..f8945a9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -131,6 +131,7 @@ efi_sources = \
env/fatvars.c \
utils.c \
bootguard.c \
+ color.c \
main.c

efi_cppflags = \
diff --git a/color.c b/color.c
new file mode 100644
index 0000000..737bfd6
--- /dev/null
+++ b/color.c
@@ -0,0 +1,21 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include "color.h"
+
+void Color(EFI_SYSTEM_TABLE *system_table, char fgcolor, char bgcolor)
+{
+ EFI_SIMPLE_TEXT_OUT_PROTOCOL *con = system_table->ConOut;
+ (void)uefi_call_wrapper(con->SetAttribute, 3, con, (bgcolor << 8) | fgcolor);
+}
diff --git a/include/color.h b/include/color.h
new file mode 100644
index 0000000..3187156
--- /dev/null
+++ b/include/color.h
@@ -0,0 +1,24 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#ifndef __COLOR_H__
+#define __COLOR_H__
+
+#include <efi.h>
+#include <efilib.h>
+#include <efiprot.h>
+
+void Color(EFI_SYSTEM_TABLE *system_table, char fgcolor, char bgcolor);
+
+#endif
diff --git a/main.c b/main.c
index 8c08df9..0d5f14b 100644
--- a/main.c
+++ b/main.c
@@ -21,6 +21,7 @@
#include <bootguard.h>
#include <configuration.h>
#include "version.h"
+#include "color.h"

extern const unsigned long init_array_start[];
extern const unsigned long init_array_end[];
@@ -113,7 +114,9 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *system_table)
this_image = image_handle;
InitializeLib(this_image, system_table);

+ Color(system_table, 3, 0);
Print(L"\nEFI Boot Guard %s\n", L""EFIBOOTGUARD_VERSION);
+ Color(system_table, 7, 0);

status =
uefi_call_wrapper(BS->OpenProtocol, 6, this_image,
--
2.19.1

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

This patch

* fixes wrong version number in AC_INIT
* adds color to the console output to make it more readable

* enables the user to override kernel args by pressing <a> if
enabled with ./configure --enable-boot-keys

Tested with qemu-system-x86_64 + OVMF.

Signed-of-by: Andreas Reichel <andreas.r...@siemens.com>

Andreas Reichel (3):
Add color to make console more readable
Add hot-key to alter bootargs during boot
Fix wrong version in configure.ac

configure.ac | 14 +++++++++++++-
include/utils.h | 1 +
main.c | 32 +++++++++++++++++++++++++++++++-
utils.c | 6 ++++++
4 files changed, 51 insertions(+), 2 deletions(-)

--
2.19.1

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Give the user a chance to override kernel arguments.
EBG waits for 3 seconds and checks if user pressed the <a> key.
This can help in unbricking a system.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

configure.ac | 12 ++++++++++++
main.c | 29 ++++++++++++++++++++++++++++-
2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 6c7f381..784defa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -166,6 +166,16 @@ AC_ARG_WITH([mem-uservars],

AC_DEFINE_UNQUOTED([ENV_MEM_USERVARS], [${ENV_MEM_USERVARS}], [Reserved memory for user variables])

+AC_ARG_ENABLE([boot-keys],
+ AS_HELP_STRING([--enable-boot-keys], [enable on-boot hot keys for debugging]),
+ [
+ EBG_BOOT_KEYS=[enabled]
+ AC_DEFINE([EBG_BOOT_KEYS],[1],[Enable on-boot hot keys])
+ ],
+ [
+ EBG_BOOT_KEYS=[disabled]
+ ])
+
dnl pkg-config
AC_PATH_PROG(PKG_CONFIG, pkg-config, no)
if test "x$PKG_CONFIG" = "xno"; then
@@ -196,4 +206,6 @@ AC_MSG_RESULT([
environment backend: ${ENV_API_FILE}.c
number of config parts: ${ENV_NUM_CONFIG_PARTS}
reserved for uservars: ${ENV_MEM_USERVARS} bytes
+
+ ebg boot keys: ${EBG_BOOT_KEYS}
])
diff --git a/main.c b/main.c
index 9fe8bbe..410da4e 100644
--- a/main.c
+++ b/main.c
@@ -22,6 +22,8 @@
#include <configuration.h>
#include "version.h"
#include "utils.h"
+#include "config.h"
+#include "envdata.h"

extern const unsigned long init_array_start[];
extern const unsigned long init_array_end[];

@@ -198,11 +200,36 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *system_table)
status);
}

+#ifdef EBG_BOOT_KEYS
+ EFI_INPUT_KEY key;
+ CHAR16 input_buffer[ENV_STRING_LENGTH];
+
+ /* Give user a chance to change the boot parameters on the fly */
+ Color(system_table, 15, 0);
+ Print(L"Press <a> to change kernel arguments\n");
+ Color(system_table, 7, 0);
+
+ /* Wait for key for 3 seconds */
+ WaitForSingleEvent(system_table->ConIn->WaitForKey, 30000000);
+
+ input_buffer[0] = L'\0';
+ if (uefi_call_wrapper(system_table->ConIn->ReadKeyStroke,
+ 2, system_table->ConIn, &key) == EFI_SUCCESS) {
+ if (key.UnicodeChar == L'a') {
+ Input(L"\nInput kernel args (empty str to cancel): ",
+ input_buffer, ENV_STRING_LENGTH - 1);
+ }
+ }
+ if (StrLen(input_buffer) > 0) {
+ StrCpy(bg_loader_params.payload_options, input_buffer);
+ }
+#endif //EBG_BOOT_KEYS
+
loaded_image->LoadOptions = bg_loader_params.payload_options;
loaded_image->LoadOptionsSize =
(StrLen(bg_loader_params.payload_options) + 1) * sizeof(CHAR16);

- Print(L"Starting %s with watchdog set to %d seconds\n",
+ Print(L"\nStarting %s with watchdog set to %d seconds\n",
bg_loader_params.payload_path, bg_loader_params.timeout);

return uefi_call_wrapper(BS->StartImage, 3, payload_handle, 0, 0);
--
2.19.1

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

include/utils.h | 1 +
main.c | 3 +++
utils.c | 6 ++++++
3 files changed, 10 insertions(+)

diff --git a/include/utils.h b/include/utils.h
index 8f7b333..d8a7db5 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -43,5 +43,6 @@ EFI_DEVICE_PATH *FileDevicePathFromConfig(EFI_HANDLE device,
CHAR16 *payloadpath);
CHAR16 *GetBootMediumPath(CHAR16 *input);
BOOLEAN IsOnBootMedium(EFI_DEVICE_PATH *dp);
+VOID Color(EFI_SYSTEM_TABLE *system_table, char fgcolor, char bgcolor);

#endif // __H_UTILS__
diff --git a/main.c b/main.c
index 8c08df9..9fe8bbe 100644
--- a/main.c
+++ b/main.c

@@ -21,6 +21,7 @@
#include <bootguard.h>
#include <configuration.h>
#include "version.h"

+#include "utils.h"

extern const unsigned long init_array_start[];
extern const unsigned long init_array_end[];

@@ -113,7 +114,9 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *system_table)
this_image = image_handle;
InitializeLib(this_image, system_table);

+ Color(system_table, 3, 0);
Print(L"\nEFI Boot Guard %s\n", L""EFIBOOTGUARD_VERSION);
+ Color(system_table, 7, 0);

status =
uefi_call_wrapper(BS->OpenProtocol, 6, this_image,

diff --git a/utils.c b/utils.c
index 3b0044c..d74cc7c 100644
--- a/utils.c
+++ b/utils.c
@@ -319,3 +319,9 @@ CHAR16 *GetBootMediumPath(CHAR16 *input)

return dst;
}
+
+VOID Color(EFI_SYSTEM_TABLE *system_table, char fgcolor, char bgcolor)

+{
+ EFI_SIMPLE_TEXT_OUT_PROTOCOL *con = system_table->ConOut;

+ (VOID)uefi_call_wrapper(con->SetAttribute, 3, con, (bgcolor << 8) | fgcolor);
+}
--
2.19.1

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

AC_INIT contains inconsistent version. Fix it to use content of
VERSION.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

configure.ac | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 784defa..a938c4f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -17,7 +17,7 @@
#

AC_INIT([efibootguard],
- [0.1.0],
+ [m4_esyscmd_s([cat VERSION])],
[],
[efibootguard],
[])
--
2.19.1

[Jan Kiszka]

Applied patch 1 and 3 to next, some remaining comments on patch 2.

Jan

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

[Jan Kiszka]

On 25.10.18 17:38, [ext] Andreas J. Reichel wrote:

> From: Andreas Reichel <andreas.r...@siemens.com>
>
> Give the user a chance to override kernel arguments.
> EBG waits for 3 seconds and checks if user pressed the <a> key.
> This can help in unbricking a system.

We have no chance to make this runtime configurable?

This value might be worth to be configurable as well. "0" could mean "do not wait".

> +
> + input_buffer[0] = L'\0';
> + if (uefi_call_wrapper(system_table->ConIn->ReadKeyStroke,
> + 2, system_table->ConIn, &key) == EFI_SUCCESS) {
> + if (key.UnicodeChar == L'a') {
> + Input(L"\nInput kernel args (empty str to cancel): ",
> + input_buffer, ENV_STRING_LENGTH - 1);
> + }
> + }
> + if (StrLen(input_buffer) > 0) {
> + StrCpy(bg_loader_params.payload_options, input_buffer);
> + }
> +#endif //EBG_BOOT_KEYS
> +
> loaded_image->LoadOptions = bg_loader_params.payload_options;
> loaded_image->LoadOptionsSize =
> (StrLen(bg_loader_params.payload_options) + 1) * sizeof(CHAR16);
>
> - Print(L"Starting %s with watchdog set to %d seconds\n",
> + Print(L"\nStarting %s with watchdog set to %d seconds\n",

The line break should go into the conditional path.

> bg_loader_params.payload_path, bg_loader_params.timeout);
>
> return uefi_call_wrapper(BS->StartImage, 3, payload_handle, 0, 0);
>