diff --git a/Makefile b/Makefile index 464544f6c..94dfc3e16 100644 --- a/Makefile +++ b/Makefile @@ -1223,6 +1223,7 @@ $(eval $(call assert_booleans,\ CONDITIONAL_CMO \ RAS_FFH_SUPPORT \ PSA_CRYPTO \ + ENABLE_CONSOLE_GETC \ ))) # Numeric_Flags @@ -1414,6 +1415,7 @@ $(eval $(call add_defines,\ SVE_VECTOR_LEN \ ENABLE_SPMD_LP \ PSA_CRYPTO \ + ENABLE_CONSOLE_GETC \ ))) ifeq (${SANITIZE_UB},trap) diff --git a/docs/getting_started/build-options.rst b/docs/getting_started/build-options.rst index 34d83f255..c045a6aa2 100644 --- a/docs/getting_started/build-options.rst +++ b/docs/getting_started/build-options.rst @@ -1191,6 +1191,12 @@ Common build options per the `PSA Crypto API specification`_. This feature is only supported if using MbedTLS 3.x version. By default it is disabled (``0``). +- ``ENABLE_CONSOLE_GETC``: Boolean option to enable `getc()` feature in console + driver(s). By default it is disabled (``0``) because it constitutes an attack + vector into TF-A by potentially allowing an attacker to inject arbitrary data. + This option should only be enabled on a need basis if there is a use case for + reading characters from the console. + GICv3 driver options -------------------- diff --git a/docs/process/security-hardening.rst b/docs/process/security-hardening.rst index f9618db08..eace467d4 100644 --- a/docs/process/security-hardening.rst +++ b/docs/process/security-hardening.rst @@ -135,6 +135,16 @@ Several build options can be used to check for security issues. Refer to the it is recommended to develop against ``W=2`` (which will eventually become the default). +Additional guidelines are provided below for some security-related build +options: + +- The ``ENABLE_CONSOLE_GETC`` build flag should be set to 0 to disable the + `getc()` feature, which allows the firmware to read characters from the + console. Keeping this feature enabled is considered dangerous from a security + point of view because it potentially allows an attacker to inject arbitrary + data into the firmware. It should only be enabled on a need basis if there is + a use case for it, for example in a testing or factory environment. + .. rubric:: References - `Arm ARM`_ diff --git a/drivers/amlogic/console/aarch64/meson_console.S b/drivers/amlogic/console/aarch64/meson_console.S index 6d0a2d62e..d955d83c5 100644 --- a/drivers/amlogic/console/aarch64/meson_console.S +++ b/drivers/amlogic/console/aarch64/meson_console.S @@ -69,7 +69,7 @@ func console_meson_register mov x0, x6 mov x30, x7 - finish_console_register meson putc=1, getc=1, flush=1 + finish_console_register meson putc=1, getc=ENABLE_CONSOLE_GETC, flush=1 register_fail: ret x7 diff --git a/drivers/arm/dcc/dcc_console.c b/drivers/arm/dcc/dcc_console.c index 8d9f7936d..d8f9462ae 100644 --- a/drivers/arm/dcc/dcc_console.c +++ b/drivers/arm/dcc/dcc_console.c @@ -53,6 +53,7 @@ static inline uint32_t __dcc_getstatus(void) return read_mdccsr_el0(); } +#if ENABLE_CONSOLE_GETC static inline char __dcc_getchar(void) { char c; @@ -61,6 +62,7 @@ static inline char __dcc_getchar(void) return c; } +#endif static inline void __dcc_putchar(char c) { @@ -102,6 +104,7 @@ static int32_t dcc_console_putc(int32_t ch, struct console *console) return ch; } +#if ENABLE_CONSOLE_GETC static int32_t dcc_console_getc(struct console *console) { unsigned int status; @@ -113,6 +116,7 @@ static int32_t dcc_console_getc(struct console *console) return __dcc_getchar(); } +#endif /** * dcc_console_flush() - Function to force a write of all buffered data @@ -135,7 +139,9 @@ static struct dcc_console dcc_console = { .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 9caeb0c69..b7d1747f5 100644 --- a/drivers/arm/pl011/aarch32/pl011_console.S +++ b/drivers/arm/pl011/aarch32/pl011_console.S @@ -116,7 +116,7 @@ func console_pl011_register 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 861d2ed22..8cb0122be 100644 --- a/drivers/arm/pl011/aarch64/pl011_console.S +++ b/drivers/arm/pl011/aarch64/pl011_console.S @@ -103,7 +103,7 @@ func console_pl011_register 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 diff --git a/drivers/cadence/uart/aarch64/cdns_console.S b/drivers/cadence/uart/aarch64/cdns_console.S index 1bdaa4872..d2dd0a8e2 100644 --- a/drivers/cadence/uart/aarch64/cdns_console.S +++ b/drivers/cadence/uart/aarch64/cdns_console.S @@ -79,7 +79,7 @@ func console_cdns_register mov x0, x6 mov x30, x7 - finish_console_register cdns putc=1, getc=1, flush=1 + finish_console_register cdns putc=1, getc=ENABLE_CONSOLE_GETC, flush=1 register_fail: ret x7 diff --git a/drivers/console/aarch32/skeleton_console.S b/drivers/console/aarch32/skeleton_console.S index a9e13ec44..05a598508 100644 --- a/drivers/console/aarch32/skeleton_console.S +++ b/drivers/console/aarch32/skeleton_console.S @@ -63,7 +63,7 @@ func console_xxx_register * If any of the argument is unspecified, then the corresponding * entry in console_t is set to 0. */ - finish_console_register xxx putc=1, getc=1, flush=1 + finish_console_register xxx putc=1, getc=ENABLE_CONSOLE_GETC, flush=1 /* Jump here if hardware init fails or parameters are invalid. */ register_fail: diff --git a/drivers/console/aarch64/skeleton_console.S b/drivers/console/aarch64/skeleton_console.S index 7ea2eec9f..3310d2892 100644 --- a/drivers/console/aarch64/skeleton_console.S +++ b/drivers/console/aarch64/skeleton_console.S @@ -63,7 +63,7 @@ func console_xxx_register * If any of the argument is unspecified, then the corresponding * entry in console_t is set to 0. */ - finish_console_register xxx putc=1, getc=1, flush=1 + finish_console_register xxx putc=1, getc=ENABLE_CONSOLE_GETC, flush=1 /* Jump here if hardware init fails or parameters are invalid. */ register_fail: diff --git a/drivers/console/multi_console.c b/drivers/console/multi_console.c index 93c38d849..e962fff37 100644 --- a/drivers/console/multi_console.c +++ b/drivers/console/multi_console.c @@ -108,6 +108,7 @@ int putchar(int c) return EOF; } +#if ENABLE_CONSOLE_GETC int console_getc(void) { int err = ERROR_NO_VALID_CONSOLE; @@ -127,6 +128,7 @@ int console_getc(void) return err; } +#endif void console_flush(void) { diff --git a/drivers/marvell/uart/a3700_console.S b/drivers/marvell/uart/a3700_console.S index c7eb165e6..a1eacbc43 100644 --- a/drivers/marvell/uart/a3700_console.S +++ b/drivers/marvell/uart/a3700_console.S @@ -140,7 +140,7 @@ func console_a3700_register mov x0, x6 mov x30, x7 - finish_console_register a3700, putc=1, getc=1, flush=1 + finish_console_register a3700, putc=1, getc=ENABLE_CONSOLE_GETC, flush=1 register_fail: ret x7 diff --git a/drivers/nxp/console/16550_console.S b/drivers/nxp/console/16550_console.S index 044d3d074..b5617a3e8 100644 --- a/drivers/nxp/console/16550_console.S +++ b/drivers/nxp/console/16550_console.S @@ -167,7 +167,7 @@ func nxp_console_16550_register register_16550: mov x0, x6 mov x30, x7 - finish_console_register 16550 putc=1, getc=1, flush=1 + finish_console_register 16550 putc=1, getc=ENABLE_CONSOLE_GETC, flush=1 register_fail: ret x7 diff --git a/drivers/ti/uart/aarch32/16550_console.S b/drivers/ti/uart/aarch32/16550_console.S index 0429f8702..898a68d8c 100644 --- a/drivers/ti/uart/aarch32/16550_console.S +++ b/drivers/ti/uart/aarch32/16550_console.S @@ -124,7 +124,7 @@ func console_16550_register register_16550: mov r0, r4 pop {r4, lr} - finish_console_register 16550 putc=1, getc=1, flush=1 + finish_console_register 16550 putc=1, getc=ENABLE_CONSOLE_GETC, flush=1 register_fail: pop {r4, pc} diff --git a/drivers/ti/uart/aarch64/16550_console.S b/drivers/ti/uart/aarch64/16550_console.S index cb2151253..2b1b5a9d7 100644 --- a/drivers/ti/uart/aarch64/16550_console.S +++ b/drivers/ti/uart/aarch64/16550_console.S @@ -118,7 +118,7 @@ func console_16550_register register_16550: mov x0, x6 mov x30, x7 - finish_console_register 16550 putc=1, getc=1, flush=1 + finish_console_register 16550 putc=1, getc=ENABLE_CONSOLE_GETC, flush=1 register_fail: ret x7 diff --git a/include/arch/aarch32/console_macros.S b/include/arch/aarch32/console_macros.S index 996cb327f..726b28186 100644 --- a/include/arch/aarch32/console_macros.S +++ b/include/arch/aarch32/console_macros.S @@ -29,12 +29,20 @@ .endif str r1, [r0, #CONSOLE_T_PUTC] + /* + * If ENABLE_CONSOLE_GETC support is disabled, but a getc callback is + * specified nonetheless, the assembler will abort on encountering the + * CONSOLE_T_GETC macro, which is undefined. + */ .ifne \getc ldr r1, =console_\_driver\()_getc + str r1, [r0, #CONSOLE_T_GETC] .else +#if ENABLE_CONSOLE_GETC mov r1, #0 + str r1, [r0, #CONSOLE_T_GETC] +#endif .endif - str r1, [r0, #CONSOLE_T_GETC] .ifne \flush ldr r1, =console_\_driver\()_flush diff --git a/include/arch/aarch64/console_macros.S b/include/arch/aarch64/console_macros.S index 3285d855a..8adb9cdb1 100644 --- a/include/arch/aarch64/console_macros.S +++ b/include/arch/aarch64/console_macros.S @@ -30,12 +30,19 @@ str xzr, [x0, #CONSOLE_T_PUTC] .endif + /* + * If ENABLE_CONSOLE_GETC support is disabled, but a getc callback is + * specified nonetheless, the assembler will abort on encountering the + * CONSOLE_T_GETC macro, which is undefined. + */ .ifne \getc adrp x1, console_\_driver\()_getc add x1, x1, :lo12:console_\_driver\()_getc str x1, [x0, #CONSOLE_T_GETC] .else +#if ENABLE_CONSOLE_GETC str xzr, [x0, #CONSOLE_T_GETC] +#endif .endif .ifne \flush diff --git a/include/drivers/console.h b/include/drivers/console.h index f499571f7..fa4eb9462 100644 --- a/include/drivers/console.h +++ b/include/drivers/console.h @@ -12,10 +12,16 @@ #define CONSOLE_T_NEXT (U(0) * REGSZ) #define CONSOLE_T_FLAGS (U(1) * REGSZ) #define CONSOLE_T_PUTC (U(2) * REGSZ) +#if ENABLE_CONSOLE_GETC #define CONSOLE_T_GETC (U(3) * REGSZ) #define CONSOLE_T_FLUSH (U(4) * REGSZ) #define CONSOLE_T_BASE (U(5) * REGSZ) #define CONSOLE_T_DRVDATA (U(6) * REGSZ) +#else +#define CONSOLE_T_FLUSH (U(3) * REGSZ) +#define CONSOLE_T_BASE (U(4) * REGSZ) +#define CONSOLE_T_DRVDATA (U(5) * REGSZ) +#endif #define CONSOLE_FLAG_BOOT (U(1) << 0) #define CONSOLE_FLAG_RUNTIME (U(1) << 1) @@ -42,7 +48,9 @@ typedef struct console { */ u_register_t flags; int (*const putc)(int character, struct console *console); +#if ENABLE_CONSOLE_GETC int (*const getc)(struct console *console); +#endif void (*const flush)(struct console *console); uintptr_t base; /* Additional private driver data may follow here. */ @@ -75,8 +83,10 @@ void console_set_scope(console_t *console, unsigned int scope); void console_switch_state(unsigned int new_state); /* Output a character on all consoles registered for the current state. */ int console_putc(int c); +#if ENABLE_CONSOLE_GETC /* Read a character (blocking) from any console registered for current state. */ int console_getc(void); +#endif /* Flush all consoles registered for the current state. */ void console_flush(void); diff --git a/include/drivers/console_assertions.h b/include/drivers/console_assertions.h index 00caa3141..9f0657326 100644 --- a/include/drivers/console_assertions.h +++ b/include/drivers/console_assertions.h @@ -19,8 +19,10 @@ CASSERT(CONSOLE_T_FLAGS == __builtin_offsetof(console_t, flags), assert_console_t_flags_offset_mismatch); CASSERT(CONSOLE_T_PUTC == __builtin_offsetof(console_t, putc), assert_console_t_putc_offset_mismatch); +#if ENABLE_CONSOLE_GETC CASSERT(CONSOLE_T_GETC == __builtin_offsetof(console_t, getc), assert_console_t_getc_offset_mismatch); +#endif CASSERT(CONSOLE_T_FLUSH == __builtin_offsetof(console_t, flush), assert_console_t_flush_offset_mismatch); CASSERT(CONSOLE_T_DRVDATA == sizeof(console_t), diff --git a/make_helpers/defaults.mk b/make_helpers/defaults.mk index 321ae9fd1..f31365a1f 100644 --- a/make_helpers/defaults.mk +++ b/make_helpers/defaults.mk @@ -362,3 +362,8 @@ ENABLE_SPMD_LP := 0 # By default, disable PSA crypto (use MbedTLS legacy crypto API). PSA_CRYPTO := 0 + +# getc() support from the console(s). +# Disabled by default because it constitutes an attack vector into TF-A. It +# should only be enabled if there is a use case for it. +ENABLE_CONSOLE_GETC := 0 diff --git a/plat/imx/common/aarch32/imx_uart_console.S b/plat/imx/common/aarch32/imx_uart_console.S index 1a1229aab..2a35b5edf 100644 --- a/plat/imx/common/aarch32/imx_uart_console.S +++ b/plat/imx/common/aarch32/imx_uart_console.S @@ -28,7 +28,7 @@ func console_imx_uart_register mov r0, r4 pop {r4, lr} - finish_console_register imx_uart putc=1, getc=1, flush=1 + finish_console_register imx_uart putc=1, getc=ENABLE_CONSOLE_GETC, flush=1 register_fail: pop {r4, pc} diff --git a/plat/imx/common/imx_uart_console.S b/plat/imx/common/imx_uart_console.S index 4d17288a1..560db15b5 100644 --- a/plat/imx/common/imx_uart_console.S +++ b/plat/imx/common/imx_uart_console.S @@ -33,7 +33,7 @@ func console_imx_uart_register mov x0, x6 mov x30, x7 - finish_console_register imx_uart putc=1, getc=1, flush=1 + finish_console_register imx_uart putc=1, getc=ENABLE_CONSOLE_GETC, flush=1 register_fail: ret x7 diff --git a/plat/imx/common/lpuart_console.S b/plat/imx/common/lpuart_console.S index ff01e3551..7acf77384 100644 --- a/plat/imx/common/lpuart_console.S +++ b/plat/imx/common/lpuart_console.S @@ -27,7 +27,7 @@ func console_lpuart_register mov x0, x6 mov x30, x7 - finish_console_register lpuart putc=1, getc=1, flush=1 + finish_console_register lpuart putc=1, getc=ENABLE_CONSOLE_GETC, flush=1 register_fail: ret x7 diff --git a/plat/nvidia/tegra/drivers/spe/shared_console.S b/plat/nvidia/tegra/drivers/spe/shared_console.S index d1b18dd44..5ad4eb8ab 100644 --- a/plat/nvidia/tegra/drivers/spe/shared_console.S +++ b/plat/nvidia/tegra/drivers/spe/shared_console.S @@ -71,7 +71,7 @@ func console_spe_register cbz x3, register_fail str x0, [x3, #CONSOLE_T_BASE] mov x0, x3 - finish_console_register spe putc=1, getc=1, flush=1 + finish_console_register spe putc=1, getc=ENABLE_CONSOLE_GETC, flush=1 register_fail: mov w0, wzr diff --git a/plat/socionext/uniphier/uniphier_console_setup.c b/plat/socionext/uniphier/uniphier_console_setup.c index 9fda26e93..9268f5d5a 100644 --- a/plat/socionext/uniphier/uniphier_console_setup.c +++ b/plat/socionext/uniphier/uniphier_console_setup.c @@ -30,7 +30,9 @@ static console_t uniphier_console = { CONSOLE_FLAG_CRASH | CONSOLE_FLAG_TRANSLATE_CRLF, .putc = uniphier_console_putc, +#if ENABLE_CONSOLE_GETC .getc = uniphier_console_getc, +#endif .flush = uniphier_console_flush, };