wireless/bluetooth: bt_hcicore and bt_uart related fixes. #16179

cederom · 2025-04-10T19:06:47Z

Summary

Bluetooth HCI and UART driver fixes.

Impact

Wireless / Bluetooth: HCI and UART.

Testing

Build verified by @cederom on ESP32-DevkitC and Nucleo-WB55RG. Runtime verification by external party.

% uname -a
FreeBSD XXX 14.2-RELEASE-p1 FreeBSD 14.2-RELEASE-p1 GENERIC amd64

% git log -5 --oneline
2209ce8002 (HEAD -> 20250325-cederom-wireless_bluetooth_fixes, cederom/20250325-cederom-wireless_bluetooth_fixes)  wireless/bt_hcicore: Fix H4 header and data buffer length verification.
0d141f6285 wireless/bt_hcicore: Fix buffer type confusion on missing response.
c63a85239b wireless/bt_uart: Fix ACL data buffer length verification.
1b9e6a8563 (origin/master, origin/HEAD, master) boards/nucleo-c092rc: add minimal NSH configuration
e474569472 boards/nucleo-c071rb: add minimal NSH configuration

% ./tools/configure.sh -B esp32-devkitc:ble
  Copy files
  Select CONFIG_HOST_BSD=y
  Refreshing...
CP: arch/dummy/Kconfig to /XXX/nuttx.git/arch/dummy/dummy_kconfig
CP: boards/dummy/Kconfig to /XXX/nuttx.git/boards/dummy/dummy_kconfig
LN: platform/board to /XXX/nuttx-apps.git/platform/dummy
LN: include/arch to arch/xtensa/include
LN: include/arch/board to /XXX/nuttx.git/boards/xtensa/esp32/esp32-devkitc/include
LN: drivers/platform to /XXX/nuttx.git/drivers/dummy
LN: include/arch/chip to /XXX/nuttx.git/arch/xtensa/include/esp32
LN: arch/xtensa/src/chip to /XXX/nuttx.git/arch/xtensa/src/esp32
LN: arch/xtensa/src/board to /XXX/nuttx.git/boards/xtensa/esp32/esp32-devkitc/../common
LN: arch/xtensa/src/board/board to /XXX/nuttx.git/boards/xtensa/esp32/esp32-devkitc/src
mkkconfig in /XXX/nuttx-apps.git/audioutils
mkkconfig in /XXX/nuttx-apps.git/benchmarks
mkkconfig in /XXX/nuttx-apps.git/boot
mkkconfig in /XXX/nuttx-apps.git/canutils
mkkconfig in /XXX/nuttx-apps.git/crypto
mkkconfig in /XXX/nuttx-apps.git/database
mkkconfig in /XXX/nuttx-apps.git/examples/mcuboot
mkkconfig in /XXX/nuttx-apps.git/examples/module
mkkconfig in /XXX/nuttx-apps.git/examples/rust
mkkconfig in /XXX/nuttx-apps.git/examples/sotest
mkkconfig in /XXX/nuttx-apps.git/examples
mkkconfig in /XXX/nuttx-apps.git/fsutils
mkkconfig in /XXX/nuttx-apps.git/games
mkkconfig in /XXX/nuttx-apps.git/graphics
mkkconfig in /XXX/nuttx-apps.git/industry
mkkconfig in /XXX/nuttx-apps.git/inertial
mkkconfig in /XXX/nuttx-apps.git/interpreters/luamodules
mkkconfig in /XXX/nuttx-apps.git/interpreters
mkkconfig in /XXX/nuttx-apps.git/logging
mkkconfig in /XXX/nuttx-apps.git/lte
mkkconfig in /XXX/nuttx-apps.git/math
mkkconfig in /XXX/nuttx-apps.git/mlearning
mkkconfig in /XXX/nuttx-apps.git/netutils
mkkconfig in /XXX/nuttx-apps.git/sdr
mkkconfig in /XXX/nuttx-apps.git/system
mkkconfig in /XXX/nuttx-apps.git/testing/arch
mkkconfig in /XXX/nuttx-apps.git/testing/cxx
mkkconfig in /XXX/nuttx-apps.git/testing/drivers
mkkconfig in /XXX/nuttx-apps.git/testing/fs
mkkconfig in /XXX/nuttx-apps.git/testing/libc
mkkconfig in /XXX/nuttx-apps.git/testing/mm
mkkconfig in /XXX/nuttx-apps.git/testing/sched
mkkconfig in /XXX/nuttx-apps.git/testing
mkkconfig in /XXX/nuttx-apps.git/videoutils
mkkconfig in /XXX/nuttx-apps.git/wireless/bluetooth
mkkconfig in /XXX/nuttx-apps.git/wireless/ieee802154
mkkconfig in /XXX/nuttx-apps.git/wireless
mkkconfig in /XXX/nuttx-apps.git
#
# configuration written to .config
#

% gmake -j8
Create version.h
Cloning Espressif HAL for 3rd Party Platforms
Clone: chip/esp-hal-3rdparty LN: platform/board to /XXX/nuttx-apps.git/platform/dummy
Register: nsh
Register: sh
Register: bt
Espressif HAL for 3rd Party Platforms: a461ca0750d1a3deca6b10a283064dbcd2b76fb1
Espressif HAL for 3rd Party Platforms: initializing submodules...
Applying patches...
CPP:  /XXX/nuttx.git/arch/xtensa/src/chip/esp-hal-3rdparty/components/esp_rom/esp32/ld/esp32.rom.newlib-funcs.ld-> /XXX/nuttx.git/arch/xtensa/src/chip/esp-hal-3rdparty/components/esp_rom/esp32/ld/esp32.rom.newlib-funcs.ld.tmpLD: nuttx
Memory region         Used Size  Region Size  %age Used
             ROM:      406628 B    4194272 B      9.69%
     iram0_0_seg:       95772 B       168 KB     55.67%
     irom0_0_seg:      275556 B    3342304 B      8.24%
     dram0_0_seg:      101968 B     115200 B     88.51%
     drom0_0_seg:       90064 B    4194272 B      2.15%
    rtc_iram_seg:          0 GB         8 KB      0.00%
    rtc_slow_seg:          0 GB         4 KB      0.00%
      extmem_seg:          0 GB         4 MB      0.00%
CP: nuttx.hex
MKIMAGE: ESP32 binary
esptool.py -c esp32 elf2image --ram-only-header -fs 4MB -fm dio -ff "40m" -o nuttx.bin nuttx
esptool.py v4.8.1
Creating esp32 image...
Image has only RAM segments visible. ROM segments are hidden and SHA256 digest is not appended.
Merged 1 ELF section
Successfully created esp32 image.
Generated: nuttx.bin

% ./tools/configure.sh -B nucleo-wb55rg:ble
  Copy files
  Select CONFIG_HOST_BSD=y
  Refreshing...
CP: arch/dummy/Kconfig to /XXX/nuttx.git/arch/dummy/dummy_kconfig
CP: boards/dummy/Kconfig to /XXX/nuttx.git/boards/dummy/dummy_kconfig
LN: platform/board to /XXX/nuttx-apps.git/platform/dummy
LN: include/arch to arch/arm/include
LN: include/arch/board to /XXX/nuttx.git/boards/arm/stm32wb/nucleo-wb55rg/include
LN: drivers/platform to /XXX/nuttx.git/drivers/dummy
LN: include/arch/chip to /XXX/nuttx.git/arch/arm/include/stm32wb
LN: arch/arm/src/chip to /XXX/nuttx.git/arch/arm/src/stm32wb
LN: arch/arm/src/board to /XXX/nuttx.git/boards/arm/stm32wb/nucleo-wb55rg/src
mkkconfig in /XXX/nuttx-apps.git/audioutils
mkkconfig in /XXX/nuttx-apps.git/benchmarks
mkkconfig in /XXX/nuttx-apps.git/boot
mkkconfig in /XXX/nuttx-apps.git/canutils
mkkconfig in /XXX/nuttx-apps.git/crypto
mkkconfig in /XXX/nuttx-apps.git/database
mkkconfig in /XXX/nuttx-apps.git/examples/mcuboot
mkkconfig in /XXX/nuttx-apps.git/examples/module
mkkconfig in /XXX/nuttx-apps.git/examples/rust
mkkconfig in /XXX/nuttx-apps.git/examples/sotest
mkkconfig in /XXX/nuttx-apps.git/examples
mkkconfig in /XXX/nuttx-apps.git/fsutils
mkkconfig in /XXX/nuttx-apps.git/games
mkkconfig in /XXX/nuttx-apps.git/graphics
mkkconfig in /XXX/nuttx-apps.git/industry
mkkconfig in /XXX/nuttx-apps.git/inertial
mkkconfig in /XXX/nuttx-apps.git/interpreters/luamodules
mkkconfig in /XXX/nuttx-apps.git/interpreters
mkkconfig in /XXX/nuttx-apps.git/logging
mkkconfig in /XXX/nuttx-apps.git/lte
mkkconfig in /XXX/nuttx-apps.git/math
mkkconfig in /XXX/nuttx-apps.git/mlearning
mkkconfig in /XXX/nuttx-apps.git/netutils
mkkconfig in /XXX/nuttx-apps.git/sdr
mkkconfig in /XXX/nuttx-apps.git/system
mkkconfig in /XXX/nuttx-apps.git/testing/arch
mkkconfig in /XXX/nuttx-apps.git/testing/cxx
mkkconfig in /XXX/nuttx-apps.git/testing/drivers
mkkconfig in /XXX/nuttx-apps.git/testing/fs
mkkconfig in /XXX/nuttx-apps.git/testing/libc
mkkconfig in /XXX/nuttx-apps.git/testing/mm
mkkconfig in /XXX/nuttx-apps.git/testing/sched
mkkconfig in /XXX/nuttx-apps.git/testing
mkkconfig in /XXX/nuttx-apps.git/videoutils
mkkconfig in /XXX/nuttx-apps.git/wireless/bluetooth
mkkconfig in /XXX/nuttx-apps.git/wireless/ieee802154
mkkconfig in /XXX/nuttx-apps.git/wireless
mkkconfig in /XXX/nuttx-apps.git
#
# configuration written to .config
#

% gmake -j8
Create version.h
LN: platform/board to /XXX/nuttx-apps.git/platform/dummy
Register: nsh
Register: sh
Register: ostest
Register: bt
chip/stm32wb_gpio.c:44:11: note: '#pragma message: CONFIG_STM32WB_USE_LEGACY_PINMAP will be deprecated migrate board.h see tools/stm32_pinmap_tool.py'
   44 | #  pragma message "CONFIG_STM32WB_USE_LEGACY_PINMAP will be deprecated migrate board.h see tools/stm32_pinmap_tool.py"
      |           ^~~~~~~
LD: nuttx
Memory region         Used Size  Region Size  %age Used
           flash:      298692 B       512 KB     56.97%
           sram1:       18160 B       192 KB      9.24%
          sram2a:           0 B        32 KB      0.00%
          sram2b:           0 B        32 KB      0.00%
CP: nuttx.hex
CP: nuttx.bin

nuttxpr · 2025-04-10T19:10:06Z

[Experimental Bot, please feedback here]

This PR does NOT fully meet the NuttX requirements. While it provides some information, it is missing crucial details.

Here's a breakdown of what's missing and how it can be improved:

Summary: "Bluetooth HCI and UART driver fixes" is too vague. It needs to explain what was broken and how these fixes address the issues. For example: "Fixes buffer overflow vulnerabilities in the Bluetooth HCI and UART drivers by correctly verifying buffer lengths before data is copied. This prevents potential crashes or memory corruption." Also, any related NuttX or NuttX Apps issues should be linked here.
Impact: While the area affected is mentioned, the specifics are lacking. Each "Impact" item needs a NO/YES answer and a description if YES. For instance:
- Is new feature added? NO
- Is existing feature changed? YES (Fixes bugs in existing Bluetooth HCI and UART drivers which could lead to system instability.)
- Impact on user: Possibly YES (If users were relying on the buggy behavior, their applications might need adjustments.) - This needs more detail. How exactly will the user be impacted?
- Impact on build: Likely NO (If the changes are purely code fixes within existing drivers, the build process shouldn't change. However, this needs confirmation.)
- Impact on hardware: NO (Assuming no hardware changes are required.) - This needs explicit confirmation.
- Impact on documentation: Possibly YES (If the bugs fixed have documentation implications, they should be updated.) - Needs more detail or confirmation that none is required.
- Impact on security: YES (Fixes buffer overflows which are security vulnerabilities.) - Needs more detail about the potential severity of these vulnerabilities.
- Impact on compatibility: Potentially YES (Clarify if this fix introduces any backward compatibility issues or changes the interoperability with other Bluetooth devices).
Testing: While build logs are provided, they do not show testing logs. The PR states "Runtime verification by external party," which is insufficient. The PR needs to include actual test results demonstrating that the fixes work as intended. This could include logs of Bluetooth communication before and after the fix, demonstrating the absence of the previous error. Simply building the code isn't testing. Furthermore, if an external party did the testing, their results need to be included in the PR. Also, the provided build logs are long and don't highlight the relevant sections. Trim them down to only show the important parts (e.g., compiler warnings or errors related to the changes, successful completion messages). The output of tests needs to be front and center.

In short, this PR provides evidence of building the code, but not of testing the changes. It also lacks the specific details required to understand the impact of the changes. Without this information, reviewers cannot adequately assess the correctness and safety of the proposed changes.

Driver now validates ACL provided buffer length agaist the size of the data buffer which is defined by CONFIG_BLUETOOTH_UART_RXBUFSIZE. Signed-off-by: Tomasz 'CeDeROM' CEDRO <tomek@cedro.info>

Fix possible stack corruption on missing command response. Signed-off-by: Tomasz 'CeDeROM' CEDRO <tomek@cedro.info>

Driver now validates data and H4 header length against CONFIG_IOB_BUFSIZE. Signed-off-by: Tomasz 'CeDeROM' CEDRO <tomek@cedro.info>

cederom · 2025-04-11T12:33:15Z

@jerpelea the fixes are confirmed and merged we can backport them to 12.9-RC1 thank you! :-)

jerpelea · 2025-04-11T12:45:44Z

@cederom already backported!

cederom assigned jerpelea Apr 10, 2025

github-actions bot added Area: Bluetooth Area: Drivers Drivers issues Size: S The size of the change in this PR is small labels Apr 10, 2025

cederom requested review from acassis, jerpelea, lupyuen, raiden00pl, tmedicci and xiaoxiang781216 April 10, 2025 19:07

VoodooChild99 added 2 commits April 10, 2025 21:24

wireless/bt_uart: Fix ACL data buffer length verification.

c42cca0

Driver now validates ACL provided buffer length agaist the size of the data buffer which is defined by CONFIG_BLUETOOTH_UART_RXBUFSIZE. Signed-off-by: Tomasz 'CeDeROM' CEDRO <tomek@cedro.info>

wireless/bt_hcicore: Fix buffer type confusion on missing response.

e0e3c72

Fix possible stack corruption on missing command response. Signed-off-by: Tomasz 'CeDeROM' CEDRO <tomek@cedro.info>

cederom force-pushed the 20250325-cederom-wireless_bluetooth_fixes branch from 2209ce8 to 0286bce Compare April 10, 2025 19:26

wireless/bt_hcicore: Fix H4 header and data buffer length verification.

3499555

Driver now validates data and H4 header length against CONFIG_IOB_BUFSIZE. Signed-off-by: Tomasz 'CeDeROM' CEDRO <tomek@cedro.info>

cederom force-pushed the 20250325-cederom-wireless_bluetooth_fixes branch from 0286bce to 3499555 Compare April 10, 2025 19:32

lupyuen approved these changes Apr 11, 2025

View reviewed changes

xiaoxiang781216 approved these changes Apr 11, 2025

View reviewed changes

xiaoxiang781216 merged commit 05358e6 into apache:master Apr 11, 2025
39 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

wireless/bluetooth: bt_hcicore and bt_uart related fixes. #16179

wireless/bluetooth: bt_hcicore and bt_uart related fixes. #16179

Uh oh!

cederom commented Apr 10, 2025

Uh oh!

nuttxpr commented Apr 10, 2025

Uh oh!

Uh oh!

cederom commented Apr 11, 2025

Uh oh!

jerpelea commented Apr 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wireless/bluetooth: bt_hcicore and bt_uart related fixes. #16179

wireless/bluetooth: bt_hcicore and bt_uart related fixes. #16179

Uh oh!

Conversation

cederom commented Apr 10, 2025

Summary

Impact

Testing

Uh oh!

nuttxpr commented Apr 10, 2025

Uh oh!

Uh oh!

cederom commented Apr 11, 2025

Uh oh!

jerpelea commented Apr 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants