Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

darshan7patel
Copy link

@darshan7patel darshan7patel commented Feb 13, 2024

  • 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
Copy link
Contributor

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

@darshan7patel darshan7patel changed the title fix(nimble): Change 'move_count' to uint8_t type nimble/host: Change 'move_count' to uint8_t type to prevent out-of-bounds errors Feb 14, 2024
@sjanc
Copy link
Contributor

sjanc commented Mar 20, 2024

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

what is exactly fixed here?

@sjanc
Copy link
Contributor

sjanc commented Mar 21, 2024

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
Copy link
Contributor

sjanc commented Mar 21, 2024

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
Copy link
Author

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
Copy link
Contributor

sjanc commented Apr 3, 2024

Hi,

please check if #1737 solves this issue for you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants