From 731f7adf4f4541a36b32d59087148f78b39d7a27 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Tue, 18 Feb 2025 16:11:13 +1100 Subject: [PATCH] stm32/sdcard: Fix unchecked uint32_t overflow in SD card driver. Manifests as `readblocks(-1, buf)` failing. The ST HAL does a bounds check, but it checks `(block_num + num_blocks)` is within bounds, so if these values overflow then it allows the read which seems to hang some SD Cards (but not all). Fix by explicitly testing for overflow in our layer of the driver. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton --- ports/stm32/sdcard.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/ports/stm32/sdcard.c b/ports/stm32/sdcard.c index a7d062939..eef6548dd 100644 --- a/ports/stm32/sdcard.c +++ b/ports/stm32/sdcard.c @@ -498,13 +498,27 @@ static HAL_StatusTypeDef sdcard_wait_finished(uint32_t timeout) { return HAL_OK; } -mp_uint_t sdcard_read_blocks(uint8_t *dest, uint32_t block_num, uint32_t num_blocks) { +static HAL_StatusTypeDef sdcard_common_checks(uint32_t block_num, uint32_t num_blocks) { // check that SD card is initialised if (!(pyb_sdmmc_flags & PYB_SDMMC_FLAG_ACTIVE)) { return HAL_ERROR; } - HAL_StatusTypeDef err = HAL_OK; + // check that adding block_num & num_blocks don't overflow + // (the ST HAL does a bounds check, but only after adding them...) + uint32_t end_block; + if (__builtin_add_overflow(block_num, num_blocks, &end_block)) { + return HAL_ERROR; + } + + return HAL_OK; +} + +mp_uint_t sdcard_read_blocks(uint8_t *dest, uint32_t block_num, uint32_t num_blocks) { + HAL_StatusTypeDef err = sdcard_common_checks(block_num, num_blocks); + if (err != HAL_OK) { + return err; + } // check that dest pointer is aligned on a 4-byte boundary uint8_t *orig_dest = NULL; @@ -595,13 +609,11 @@ mp_uint_t sdcard_read_blocks(uint8_t *dest, uint32_t block_num, uint32_t num_blo } mp_uint_t sdcard_write_blocks(const uint8_t *src, uint32_t block_num, uint32_t num_blocks) { - // check that SD card is initialised - if (!(pyb_sdmmc_flags & PYB_SDMMC_FLAG_ACTIVE)) { - return HAL_ERROR; + HAL_StatusTypeDef err = sdcard_common_checks(block_num, num_blocks); + if (err != HAL_OK) { + return err; } - HAL_StatusTypeDef err = HAL_OK; - // check that src pointer is aligned on a 4-byte boundary if (((uint32_t)src & 3) != 0) { // pointer is not aligned, so allocate a temporary block to do the write