nimble/host: Change 'move_count' to uint8_t type to prevent out-of-bounds errors

apache / mynewt-nimble

Apache mynewt

https://mynewt.apache.org/

Apache License 2.0

680 stars 392 forks source link

nimble/host: Change 'move_count' to uint8_t type to prevent out-of-bounds errors #1693

Open darshan7patel opened 6 months ago

darshan7patel commented 6 months ago

Change 'move_count' to uint8_t type to prevent out-of-bounds errors during memmove with compiler optimization enabled.
Fixed compilation issue in nimble for -O2 flag.
error: 'memmove' offset [88, 175] is out of the bounds [0, 88] of object 'ble_store_config_peer_secs' with type 'struct ble_store_value_sec[1]' [-Werror=array-bounds=] memmove(dst, src, move_count * value_size);

andrzej-kaczmarek commented 6 months ago

please add proper prefix to commit title (see other commits as guideline) and include error message in commit message

sjanc commented 5 months ago

I'm having trouble understanding this fix, this basically limit move_count to 255

what is exactly fixed here?

sjanc commented 5 months ago

okay, so I looked at this and it appears that GCC is not able to properly track relations between globals (eg ble_store_config_num_peer_secs) and memmove in ble_store_config_delete_obj

sjanc commented 5 months ago

I'd suggest to use assert() or if() to hint GCC

BTW BLE_STORE_MAX_CCCDS() might have similar issues (if also configured to 1)

darshan7patel commented 5 months ago

Basically, the size has been increased since, when compiler optimization was enabled, it was previously creating out-of-bounds errors. So, now it will prevent these errors.

sjanc commented 4 months ago

Hi,

please check if https://github.com/apache/mynewt-nimble/pull/1737 solves this issue for you