From ee18d277225fb7456d81a1d3668afa7a2eec3fe9 Mon Sep 17 00:00:00 2001 From: Movitz Aaron Date: Wed, 28 Oct 2026 14:37:51 +0000 Subject: [PATCH] Fix SetStatusDelimiter: stack over-read and pointer-sign warnings Two remaining issues in YEEP144_BluetoothUART.c, neither addressed by 1.2. SetStatusDelimiter() transmits sizeof(rxBuf) = 10 bytes from an 8-byte stack buffer (rxBuf is a different buffer declared earlier in the same function), so two bytes of stack garbage follow every "SO" command the module receives. Send strlen(txBuf) instead. The same function passes char* to HAL_UART_Transmit(), which takes const uint8_t*, producing three -Wpointer-sign warnings. They sit on the same lines as the over-read, which is part of why it went unnoticed; with the casts the file compiles warning-free. See CODE_FIXES.md for the full write-up. --- CODE_FIXES.md | 108 +++++++++++++++++++++++++ YEEP144lib/src/YEEP144_BluetoothUART.c | 10 ++- 2 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 CODE_FIXES.md diff --git a/CODE_FIXES.md b/CODE_FIXES.md new file mode 100644 index 0000000..cc1a0c8 --- /dev/null +++ b/CODE_FIXES.md @@ -0,0 +1,108 @@ +# CODE_FIXES — YEEP-144 library + +Two fixes to `YEEP144lib/src/YEEP144_BluetoothUART.c`, against the **1.2 release**. + +Both are pre-existing issues that 1.2 does not address. + +> **Already fixed in 1.2, not part of this change:** an earlier report covered +> `YEEP144_BluetoothUART_Init()` entering the RN4678's command mode and +> rebooting the module on every boot (by calling `SetStatusDelimiter()` +> unconditionally). 1.2 has removed that call from `Init()`, so no change is +> needed here. See "Suggested follow-up" below. + +| # | Fix | Severity | +|---|---|---| +| 1 | `SetStatusDelimiter()` over-reads a stack buffer | Medium — undefined behaviour, and it sends garbage to the module | +| 2 | Three `-Wpointer-sign` warnings in the same function | Low — but they sit on the same lines as #1 | + +--- + +## Fix 1 — SetStatusDelimiter over-reads a stack buffer + +### Root cause + +```c +uint8_t rxBuf[10]; +... +char txBuf[8] = {0}; +if(prefix == '\0' && postfix == '\0'){ + sprintf(txBuf, "SO,,%c",'\r'); +} +... +HAL_UART_Transmit(&huart2, txBuf, sizeof(rxBuf), UART_POLL_TIMEOUT); + ^^^^^^^^^^^^ rxBuf is uint8_t[10] +``` + +`rxBuf` is a **different, unrelated** buffer declared earlier in the same +function. This transmits **10 bytes from an 8-byte array** — a two-byte stack +over-read, so two bytes of stack garbage follow every `SO` command the module +receives. + +1.2 no longer calls this function from `Init()`, so it is only reachable if an +application calls `YEEP144_BluetoothUART_SetStatusDelimiter()` directly. It is +still a public API function, and the over-read is undefined behaviour either way. + +### Patch + +```c +HAL_UART_Transmit(&huart2, (const uint8_t *)txBuf, (uint16_t)strlen(txBuf), + UART_POLL_TIMEOUT); +``` + +`strlen()` is the right length: it stops at the terminating NUL that +`sprintf()` wrote. `sizeof(txBuf)` would also fix the over-read, but would +still transmit the padding NULs that `sprintf()` left in the buffer. + +--- + +## Fix 2 — Three pointer-sign warnings + +### Symptom + +``` +warning: pointer targets in passing argument 2 of 'HAL_UART_Transmit' differ in + signedness [-Wpointer-sign] +``` + +at three call sites in `SetStatusDelimiter()`. + +### Root cause + +`HAL_UART_Transmit()` takes `const uint8_t *`; the function passes `char *` +(two string literals, and `txBuf`). + +### Patch + +Cast at the three call sites: + +```c +HAL_UART_Transmit(&huart2, (const uint8_t *)"$$$", 3, UART_POLL_TIMEOUT); +HAL_UART_Transmit(&huart2, (const uint8_t *)txBuf, (uint16_t)strlen(txBuf), UART_POLL_TIMEOUT); +HAL_UART_Transmit(&huart2, (const uint8_t *)"R,1", 3, UART_POLL_TIMEOUT); +``` + +With this, the file compiles warning-free. + +--- + +## Suggested follow-up (not part of this change) + +1.2 removed the module configuration from `Init()`. The capability itself is +still worth having, as an explicitly-named function rather than a side effect of +startup — e.g. `YEEP144_BluetoothUART_ConfigureModule()`, documented as taking +several seconds, entering the module's command mode, rebooting the module, and +therefore dropping any active connection. + +--- + +## Files changed + +| File | Change | +|---|---| +| `YEEP144lib/src/YEEP144_BluetoothUART.c` | Both fixes — 7 insertions, 3 deletions against `main` | + +## Repository note + +`Drivers/` (the vendored ST HAL and CMSIS) is not in this repository: it is +173 MB, of which 108 MB is ST's `.chm` user manuals. Fetch it from +STMicroelectronics/STM32CubeH5 if you need to build. diff --git a/YEEP144lib/src/YEEP144_BluetoothUART.c b/YEEP144lib/src/YEEP144_BluetoothUART.c index 4956e94..dde1329 100644 --- a/YEEP144lib/src/YEEP144_BluetoothUART.c +++ b/YEEP144lib/src/YEEP144_BluetoothUART.c @@ -192,7 +192,7 @@ void YEEP144_BluetoothUART_SetStatusDelimiter(char prefix, char postfix){ uint16_t rxSize; // Enter command mode - HAL_UART_Transmit(&huart2, "$$$", 3, UART_POLL_TIMEOUT); + HAL_UART_Transmit(&huart2, (const uint8_t *)"$$$", 3, UART_POLL_TIMEOUT); // Wait to receive data back from BT module HAL_UARTEx_ReceiveToIdle(&huart2, rxBuf, 10, &rxSize, UART_POLL_TIMEOUT); @@ -211,13 +211,17 @@ void YEEP144_BluetoothUART_SetStatusDelimiter(char prefix, char postfix){ else{ sprintf(txBuf, "SO,%c,%c%c", prefix, postfix, '\r'); } - HAL_UART_Transmit(&huart2, txBuf, sizeof(rxBuf), UART_POLL_TIMEOUT); + // rxBuf is a different, unrelated buffer (uint8_t rxBuf[10]); using its + // size here transmits 10 bytes from this 8-byte array, so two bytes of + // stack garbage follow every command. Send exactly the string. + HAL_UART_Transmit(&huart2, (const uint8_t *)txBuf, (uint16_t)strlen(txBuf), + UART_POLL_TIMEOUT); // Wait to receive data back from BT module HAL_UARTEx_ReceiveToIdle(&huart2, rxBuf, 10, &rxSize, UART_POLL_TIMEOUT); // Reboot device to make changes - HAL_UART_Transmit(&huart2, "R,1", 3, UART_POLL_TIMEOUT); + HAL_UART_Transmit(&huart2, (const uint8_t *)"R,1", 3, UART_POLL_TIMEOUT); // Wait to receive data back from BT module HAL_UARTEx_ReceiveToIdle(&huart2, rxBuf, 10, &rxSize, UART_POLL_TIMEOUT);