refactor(console): disable getc() by default
The ability to read a character from the console constitutes an attack
vector into TF-A, as it gives attackers a means to inject arbitrary
data into TF-A. It is dangerous to keep that feature enabled if not
strictly necessary, especially in production firmware builds.
Thus, we need a way to disable this feature. Moreover, when it is
disabled, all related code should be eliminated from the firmware
binaries, such that no remnant/dead getc() code remains in memory,
which could otherwise be used as a gadget as part of a bigger security
attack.
This patch disables getc() feature by default. For legitimate getc()
use cases [1], it can be explicitly enabled by building TF-A with
ENABLE_CONSOLE_GETC=1.
The following changes are introduced when getc() is disabled:
- The multi-console framework no longer provides the console_getc()
function.
- If the console driver selected by the platform attempts to register
a getc() callback into the multi-console framework then TF-A will
now fail to build.
If registered through the assembly function finish_console_register():
- On AArch64, you'll get:
Error: undefined symbol CONSOLE_T_GETC used as an immediate value.
- On AArch32, you'll get:
Error: internal_relocation (type: OFFSET_IMM) not fixed up
If registered through the C function console_register(), this requires
populating a struct console with a getc field, which will trigger:
error: 'console_t' {aka 'struct console'} has no member named 'getc'
- All console drivers which previously registered a getc() callback
have been modified to do so only when ENABLE_CONSOLE_GETC=1.
[1] Example of such use cases would be:
- Firmware recovery: retrieving a golden BL2 image over the console in
order to repair a broken firmware on a bricked board.
- Factory CLI tool: Drive some soak tests through the console.
Discussed on TF-A mailing list here:
https://lists.trustedfirmware.org/archives/list/tf-a@lists.trustedfirmware.org/thread/YS7F6RCNTWBTEOBLAXIRTXWIOYINVRW7/
Change-Id: Icb412304cd23dbdd7662df7cf8992267b7975cc5
Signed-off-by: Sandrine Bailleux <sandrine.bailleux@arm.com>
Acked-by: Baruch Siach <baruch@tkos.co.il>
diff --git a/drivers/arm/dcc/dcc_console.c b/drivers/arm/dcc/dcc_console.c
index 8d9f793..d8f9462 100644
--- a/drivers/arm/dcc/dcc_console.c
+++ b/drivers/arm/dcc/dcc_console.c
@@ -53,6 +53,7 @@
return read_mdccsr_el0();
}
+#if ENABLE_CONSOLE_GETC
static inline char __dcc_getchar(void)
{
char c;
@@ -61,6 +62,7 @@
return c;
}
+#endif
static inline void __dcc_putchar(char c)
{
@@ -102,6 +104,7 @@
return ch;
}
+#if ENABLE_CONSOLE_GETC
static int32_t dcc_console_getc(struct console *console)
{
unsigned int status;
@@ -113,6 +116,7 @@
return __dcc_getchar();
}
+#endif
/**
* dcc_console_flush() - Function to force a write of all buffered data
@@ -135,7 +139,9 @@
.flags = CONSOLE_FLAG_BOOT |
CONSOLE_FLAG_RUNTIME,
.putc = dcc_console_putc,
+#if ENABLE_CONSOLE_GETC
.getc = dcc_console_getc,
+#endif
.flush = dcc_console_flush,
},
};
diff --git a/drivers/arm/pl011/aarch32/pl011_console.S b/drivers/arm/pl011/aarch32/pl011_console.S
index 9caeb0c..b7d1747 100644
--- a/drivers/arm/pl011/aarch32/pl011_console.S
+++ b/drivers/arm/pl011/aarch32/pl011_console.S
@@ -116,7 +116,7 @@
mov r0, r4
pop {r4, lr}
- finish_console_register pl011 putc=1, getc=1, flush=1
+ finish_console_register pl011 putc=1, getc=ENABLE_CONSOLE_GETC, flush=1
register_fail:
pop {r4, pc}
diff --git a/drivers/arm/pl011/aarch64/pl011_console.S b/drivers/arm/pl011/aarch64/pl011_console.S
index 861d2ed..8cb0122 100644
--- a/drivers/arm/pl011/aarch64/pl011_console.S
+++ b/drivers/arm/pl011/aarch64/pl011_console.S
@@ -103,7 +103,7 @@
mov x0, x6
mov x30, x7
- finish_console_register pl011 putc=1, getc=1, flush=1
+ finish_console_register pl011 putc=1, getc=ENABLE_CONSOLE_GETC, flush=1
register_fail:
ret x7