[PATCH v3 6/7] riscv: Ensure gp is NULL or points to valid data

Rick Chen rickchen36 at gmail.com
Tue Sep 22 03:05:25 CEST 2020


> This ensures constructs like `if (gd & gd->...) { ... }` work when
> accessing the global data pointer. Without this change, it was possible for
> a very early trap to cause _exit_trap to directly or indirectly (through
> printf) to read arbitrary memory. This could cause a second trap,
> preventing show_regs from being printed.
>
> printf (and specifically puts) uses gd to determine what function to print
> with. These functions in turn use gd to find the serial device, etc.
> However, before accessing gd, puts first checks to see if it is non-NULL.
> This indicates an existing (perhaps undocumented) assumption that either gd
> is NULL or it is completely valid.
>
> Before this patch, gd either points to unexpected data (because it retains
> the value it did from the prior-stage) or points to uninitialized data
> (because it has not yet been initialized by board_init_f_init_reserve)
> until the hart has acquired available_harts_lock. This can cause two
> problems, depending on the value of gd->flags. If GD_FLG_SERIAL_READY is
> unset, then some garbage data will be printed to stdout, but there will not
> be a second trap. However, if GD_FLG_SERIAL_READY is set, then puts will
> try to print with serial_puts, which will likely cause a second trap.
>
> After this patch, gd is zero up until either a hart has set it in
> wait_for_gd_init, or until it is set by arch_init_gd. This prevents its
> usage before its data is initialized because both handle_trap and puts
> ensure that gd is nonzero before using it. After gd has been set, it is OK
> to access it because its data has been cleared (and so flags is valid).
>
> XIP cannot use locks because flash is not writable. This leaves it
> vulnerable to the same class of bugs regarding already-pending IPIs as
> before this series. Fixing that would require finding another method of
> synchronization, which is outside the scope of this series.
>
> Fixes: 7c6ca03eae ("riscv: additional crash information")
> Signed-off-by: Sean Anderson <seanga2 at gmail.com>
> Reviewed-by: Bin Meng <bin.meng at windriver.com>
>
> ---
>
> Changes in v3:
> - Clarify XIP comment
>
> Changes in v2:
> - Set gp early with XIP
>
>  arch/riscv/cpu/start.S      | 28 +++++++++++++++++++++++++---
>  arch/riscv/lib/interrupts.c |  3 ++-
>  2 files changed, 27 insertions(+), 4 deletions(-)
>

Reviewed-by: Rick Chen <rick at andestech.com>


More information about the U-Boot mailing list