From f3055c89d0d916b2d2dc589706c0d94e7f0e448b Mon Sep 17 00:00:00 2001 From: cepetr Date: Wed, 16 Oct 2024 11:50:10 +0200 Subject: [PATCH] feat(core): added access control for framebuffer [no changelog] --- core/embed/trezorhal/mpu.h | 12 ++++++++++ core/embed/trezorhal/stm32f4/mpu.c | 4 ++++ .../stm32f4/xdisplay/st-7789/display_driver.c | 6 +++-- .../stm32f4/xdisplay/st-7789/display_fb.c | 5 ++++ .../stm32f429i-disc1/display_driver.c | 8 +++++++ .../stm32f4/xdisplay/ug-2828/display_driver.c | 16 ++++++++++++- .../stm32f4/xdisplay/vg-2864/display_driver.c | 16 ++++++++++--- core/embed/trezorhal/stm32u5/mpu.c | 23 +++++++++++++++++++ .../xdisplay/stm32u5a9j-dk/display_driver.c | 3 +++ .../xdisplay/stm32u5a9j-dk/display_fb.c | 7 ++++++ .../xdisplay/stm32u5a9j-dk/display_internal.h | 4 ++++ 11 files changed, 98 insertions(+), 6 deletions(-) diff --git a/core/embed/trezorhal/mpu.h b/core/embed/trezorhal/mpu.h index 8729c62e8c7..657d311356e 100644 --- a/core/embed/trezorhal/mpu.h +++ b/core/embed/trezorhal/mpu.h @@ -20,6 +20,8 @@ #ifndef TREZORHAL_MPU_H #define TREZORHAL_MPU_H +#include + #ifdef KERNEL_MODE // The MPU driver can be set to on of the following modes. @@ -66,6 +68,16 @@ mpu_mode_t mpu_reconfig(mpu_mode_t mode); // Same as `mpu_reconfig()`, but with a more descriptive name. void mpu_restore(mpu_mode_t mode); +// Sets the MPU to allow unprivileged access to the +// framebuffer at the given address and size. +// +// The changes are made effective after the next MPU reconfiguration +// to the `MPU_MODE_APP` mode. +// +// Addr and size must be aligned to the 32-byte boundary. +// If addr == 0, the framebuffer is not accessible in the unprivileged mode. +void mpu_set_unpriv_fb(void* addr, size_t size); + #endif // KERNEL_MODE #endif // TREZORHAL_MPU_H diff --git a/core/embed/trezorhal/stm32f4/mpu.c b/core/embed/trezorhal/stm32f4/mpu.c index 281bb26b0f6..230c5d625c8 100644 --- a/core/embed/trezorhal/stm32f4/mpu.c +++ b/core/embed/trezorhal/stm32f4/mpu.c @@ -204,6 +204,10 @@ mpu_mode_t mpu_get_mode(void) { return drv->mode; } +void mpu_set_unpriv_fb(void* addr, size_t size) { + // Not implemented on STM32F4 +} + // STM32F4xx memory map // // 0x08000000 2MB FLASH diff --git a/core/embed/trezorhal/stm32f4/xdisplay/st-7789/display_driver.c b/core/embed/trezorhal/stm32f4/xdisplay/st-7789/display_driver.c index d08911ae8c0..ff59d1b60dd 100644 --- a/core/embed/trezorhal/stm32f4/xdisplay/st-7789/display_driver.c +++ b/core/embed/trezorhal/stm32f4/xdisplay/st-7789/display_driver.c @@ -21,12 +21,12 @@ #include +#include "backlight_pwm.h" #include "display_fb.h" #include "display_internal.h" #include "display_io.h" #include "display_panel.h" - -#include "backlight_pwm.h" +#include "mpu.h" #ifndef BOARDLOADER #include "bg_copy.h" @@ -98,6 +98,8 @@ void display_deinit(display_content_mode_t mode) { #endif #endif + mpu_set_unpriv_fb(NULL, 0); + backlight_pwm_deinit(mode == DISPLAY_RESET_CONTENT ? BACKLIGHT_RESET : BACKLIGHT_RETAIN); diff --git a/core/embed/trezorhal/stm32f4/xdisplay/st-7789/display_fb.c b/core/embed/trezorhal/stm32f4/xdisplay/st-7789/display_fb.c index b9580fd6338..c599090b751 100644 --- a/core/embed/trezorhal/stm32f4/xdisplay/st-7789/display_fb.c +++ b/core/embed/trezorhal/stm32f4/xdisplay/st-7789/display_fb.c @@ -177,6 +177,8 @@ bool display_get_frame_buffer(display_fb_info_t *fb) { fb->ptr = get_fb_ptr(drv->queue.wix); fb->stride = DISPLAY_RESX * sizeof(uint16_t); + // Enable access to the frame buffer from the unprivileged code + mpu_set_unpriv_fb(fb->ptr, PHYSICAL_FRAME_BUFFER_SIZE); return true; } @@ -215,6 +217,9 @@ void display_refresh(void) { return; } + // Disable access to the frame buffer from the unprivileged code + mpu_set_unpriv_fb(NULL, 0); + #ifndef BOARDLOADER if (is_mode_exception()) { // Disable scheduling of any new background copying diff --git a/core/embed/trezorhal/stm32f4/xdisplay/stm32f429i-disc1/display_driver.c b/core/embed/trezorhal/stm32f4/xdisplay/stm32f429i-disc1/display_driver.c index 8030eb2b308..78647ad1285 100644 --- a/core/embed/trezorhal/stm32f4/xdisplay/stm32f429i-disc1/display_driver.c +++ b/core/embed/trezorhal/stm32f4/xdisplay/stm32f429i-disc1/display_driver.c @@ -27,6 +27,7 @@ #include "display_internal.h" #include "ili9341_spi.h" +#include "mpu.h" #include "xdisplay.h" #if (DISPLAY_RESX != 240) || (DISPLAY_RESY != 320) @@ -73,6 +74,8 @@ void display_init(display_content_mode_t mode) { void display_deinit(display_content_mode_t mode) { display_driver_t *drv = &g_display_driver; + mpu_set_unpriv_fb(NULL, 0); + drv->initialized = false; } @@ -133,12 +136,17 @@ bool display_get_frame_buffer(display_fb_info_t *fb) { } else { fb->ptr = (void *)drv->framebuf; fb->stride = DISPLAY_RESX * sizeof(uint16_t); + // Enable access to the frame buffer from the unprivileged code + mpu_set_unpriv_fb(fb->ptr, FRAME_BUFFER_SIZE); return true; } } void display_refresh(void) { // Do nothing as using just a single frame buffer + + // Disable access to the frame buffer from the unprivileged code + mpu_set_unpriv_fb(NULL, 0); } void display_fill(const gfx_bitblt_t *bb) { diff --git a/core/embed/trezorhal/stm32f4/xdisplay/ug-2828/display_driver.c b/core/embed/trezorhal/stm32f4/xdisplay/ug-2828/display_driver.c index bf924ad098e..0aed268fd38 100644 --- a/core/embed/trezorhal/stm32f4/xdisplay/ug-2828/display_driver.c +++ b/core/embed/trezorhal/stm32f4/xdisplay/ug-2828/display_driver.c @@ -23,6 +23,7 @@ #include TREZOR_BOARD #include STM32_HAL_H +#include "mpu.h" #include "xdisplay.h" #if (DISPLAY_RESX != 128) || (DISPLAY_RESY != 128) @@ -35,12 +36,16 @@ // This type of displayed was used on some preliminary dev kits for T3T1 (Trezor // TS3) +#define FRAME_BUFFER_SIZE (DISPLAY_RESX * DISPLAY_RESY) + +__attribute__((section(".fb1"))) uint8_t g_framebuf[FRAME_BUFFER_SIZE]; + // Display driver context. typedef struct { // Set if the driver is initialized bool initialized; // Frame buffer (8-bit Mono) - uint8_t framebuf[DISPLAY_RESX * DISPLAY_RESY]; + uint8_t *framebuf; // Current display orientation (0 or 180) int orientation_angle; // Current backlight level ranging from 0 to 255 @@ -307,6 +312,7 @@ void display_init(display_content_mode_t mode) { } memset(drv, 0, sizeof(display_driver_t)); + drv->framebuf = g_framebuf; if (mode == DISPLAY_RESET_CONTENT) { // Initialize GPIO & FSMC controller @@ -321,6 +327,8 @@ void display_init(display_content_mode_t mode) { void display_deinit(display_content_mode_t mode) { display_driver_t *drv = &g_display_driver; + mpu_set_unpriv_fb(NULL, 0); + drv->initialized = false; } @@ -400,6 +408,8 @@ bool display_get_frame_buffer(display_fb_info_t *fb) { } else { fb->ptr = &drv->framebuf[0]; fb->stride = DISPLAY_RESX; + // Enable access to the frame buffer from the unprivileged code + mpu_set_unpriv_fb(fb->ptr, FRAME_BUFFER_SIZE); return true; } } @@ -411,6 +421,10 @@ void display_refresh(void) { return NULL; } + // Disable access to the frame buffer from the unprivileged code + mpu_set_unpriv_fb(NULL, 0); + + // Copy the frame buffer to the display display_sync_with_fb(); } diff --git a/core/embed/trezorhal/stm32f4/xdisplay/vg-2864/display_driver.c b/core/embed/trezorhal/stm32f4/xdisplay/vg-2864/display_driver.c index 5ed52592b50..9f9bb964d82 100644 --- a/core/embed/trezorhal/stm32f4/xdisplay/vg-2864/display_driver.c +++ b/core/embed/trezorhal/stm32f4/xdisplay/vg-2864/display_driver.c @@ -25,6 +25,7 @@ #include TREZOR_BOARD #include STM32_HAL_H +#include "mpu.h" #include "xdisplay.h" #ifdef USE_CONSUMPTION_MASK @@ -41,7 +42,9 @@ // // This type of display is used with T3B1 model (Trezor TS3) -__attribute__((section(".fb1"))) uint8_t framebuf[DISPLAY_RESX * DISPLAY_RESY]; +#define FRAME_BUFFER_SIZE (DISPLAY_RESX * DISPLAY_RESY) + +__attribute__((section(".fb1"))) uint8_t g_framebuf[FRAME_BUFFER_SIZE]; // Display driver context. typedef struct { @@ -236,7 +239,7 @@ void display_init(display_content_mode_t mode) { memset(drv, 0, sizeof(display_driver_t)); drv->backlight_level = 255; - drv->framebuf = framebuf; + drv->framebuf = g_framebuf; if (mode == DISPLAY_RESET_CONTENT) { OLED_DC_CLK_ENA(); @@ -308,6 +311,8 @@ void display_init(display_content_mode_t mode) { void display_deinit(display_content_mode_t mode) { display_driver_t *drv = &g_display_driver; + mpu_set_unpriv_fb(NULL, 0); + drv->initialized = false; } @@ -369,6 +374,8 @@ bool display_get_frame_buffer(display_fb_info_t *fb) { } else { fb->ptr = &drv->framebuf[0]; fb->stride = DISPLAY_RESX; + // Enable access to the frame buffer from the unprivileged code + mpu_set_unpriv_fb(fb->ptr, FRAME_BUFFER_SIZE); return true; } } @@ -386,7 +393,10 @@ void display_refresh(void) { consumption_mask_randomize(); #endif - // Sends the current frame buffer to the display + // Disable access to the frame buffer from the unprivileged code + mpu_set_unpriv_fb(NULL, 0); + + // Copy the frame buffer to the display display_sync_with_fb(drv); } diff --git a/core/embed/trezorhal/stm32u5/mpu.c b/core/embed/trezorhal/stm32u5/mpu.c index 1744061e802..598cee1c265 100644 --- a/core/embed/trezorhal/stm32u5/mpu.c +++ b/core/embed/trezorhal/stm32u5/mpu.c @@ -192,6 +192,11 @@ typedef struct { bool initialized; // Current mode mpu_mode_t mode; + // Address of the framebuffer visible to unprivileged code + // (if set to 0, the framebuffer is not accessible) + uint32_t unpriv_fb_addr; + // Size of the framebuffer in bytes + size_t unpriv_fb_size; } mpu_driver_t; @@ -287,6 +292,17 @@ mpu_mode_t mpu_get_mode(void) { return drv->mode; } +void mpu_set_unpriv_fb(void* addr, size_t size) { + mpu_driver_t* drv = &g_mpu_driver; + + if (!drv->initialized) { + return; + } + + drv->unpriv_fb_addr = (uint32_t)addr; + drv->unpriv_fb_size = size; +} + mpu_mode_t mpu_reconfig(mpu_mode_t mode) { mpu_driver_t* drv = &g_mpu_driver; @@ -307,6 +323,13 @@ mpu_mode_t mpu_reconfig(mpu_mode_t mode) { case MPU_MODE_SAES: SET_REGION( 5, PERIPH_BASE_NS, PERIPH_SIZE, PERIPHERAL, YES, YES ); // Peripherals - SAES, TAMP break; + case MPU_MODE_APP: + if (drv->unpriv_fb_addr != 0) { + SET_REGION( 5, drv->unpriv_fb_addr, drv->unpriv_fb_size, SRAM, YES, YES ); // Frame buffer + } else { + DIS_REGION( 5 ); + } + break; default: SET_REGION( 5, GRAPHICS_START, GRAPHICS_SIZE, SRAM, YES, YES ); // Frame buffer or display interface break; diff --git a/core/embed/trezorhal/stm32u5/xdisplay/stm32u5a9j-dk/display_driver.c b/core/embed/trezorhal/stm32u5/xdisplay/stm32u5a9j-dk/display_driver.c index 6bb01934a27..ea011796b64 100644 --- a/core/embed/trezorhal/stm32u5/xdisplay/stm32u5a9j-dk/display_driver.c +++ b/core/embed/trezorhal/stm32u5/xdisplay/stm32u5a9j-dk/display_driver.c @@ -24,6 +24,7 @@ #include STM32_HAL_H #include "display_internal.h" +#include "mpu.h" #include "xdisplay.h" #if (DISPLAY_RESX != 240) || (DISPLAY_RESY != 240) @@ -110,6 +111,8 @@ void display_deinit(display_content_mode_t mode) { BSP_LCD_DeInit(0); } + mpu_set_unpriv_fb(NULL, 0); + drv->initialized = false; } diff --git a/core/embed/trezorhal/stm32u5/xdisplay/stm32u5a9j-dk/display_fb.c b/core/embed/trezorhal/stm32u5/xdisplay/stm32u5a9j-dk/display_fb.c index 1ca991fbe86..41fd36e433c 100644 --- a/core/embed/trezorhal/stm32u5/xdisplay/stm32u5a9j-dk/display_fb.c +++ b/core/embed/trezorhal/stm32u5/xdisplay/stm32u5a9j-dk/display_fb.c @@ -25,6 +25,7 @@ #include #include "display_internal.h" +#include "mpu.h" #ifdef KERNEL_MODE @@ -67,6 +68,9 @@ bool display_get_frame_buffer(display_fb_info_t *fb) { fb->ptr = (void *)addr; fb->stride = fb_stride; + // Enable access to the frame buffer from the unprivileged code + mpu_set_unpriv_fb(fb->ptr, VIRTUAL_FRAME_BUFFER_SIZE); + return true; } @@ -77,6 +81,9 @@ void display_refresh(void) { return; } + // Disable access to the frame buffer from the unprivileged code + mpu_set_unpriv_fb(NULL, 0); + if (current_frame_buffer == 0) { current_frame_buffer = 1; BSP_LCD_SetFrameBuffer(0, GFXMMU_VIRTUAL_BUFFER1_BASE_S); diff --git a/core/embed/trezorhal/stm32u5/xdisplay/stm32u5a9j-dk/display_internal.h b/core/embed/trezorhal/stm32u5/xdisplay/stm32u5a9j-dk/display_internal.h index bb7d5582a23..af6e543c89b 100644 --- a/core/embed/trezorhal/stm32u5/xdisplay/stm32u5a9j-dk/display_internal.h +++ b/core/embed/trezorhal/stm32u5/xdisplay/stm32u5a9j-dk/display_internal.h @@ -45,6 +45,10 @@ extern display_driver_t g_display_driver; // Pitch (in pixels) of the virtual frame buffer #define FRAME_BUFFER_PIXELS_PER_LINE 768 +// Size of the virtual frame buffer in bytes +#define VIRTUAL_FRAME_BUFFER_SIZE \ + (FRAME_BUFFER_PIXELS_PER_LINE * DISPLAY_RESY * 4) + // Physical frame buffers in internal SRAM memory // // Both frame buffers layes in the fixed addresses that