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: Set length correctly in perdioic_set_adv_data #1539

Merged

Conversation

rahult-github
Copy link
Contributor

This change sets length field only if data is non NULL, else sets length to zero.

Core 5.4, Vol 4, Part E, section 7.8.62 states

"If Operation is not 0x03 or 0x04 and Advertising_Data_Length is zero, then the Controller shall return the error code Invalid HCI Command Parameters (0x12).."

Current code sets length to size of data. However, to validate above, need to send 0 length . So calculate length only if data is present.

@rahult-github rahult-github force-pushed the bugfix/handle_NULL_data_periodic_adv branch from 75f77d2 to 898af6f Compare June 29, 2023 10:47
Copy link
Contributor

@sjanc sjanc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently ble_gap_periodic_adv_set() assumes that data is always valid and I'm not sure on how this patch would work since *data is dereferenced mutliple times later on in the function

@rahult-github rahult-github force-pushed the bugfix/handle_NULL_data_periodic_adv branch from 898af6f to c04b6ae Compare July 4, 2023 10:08
@rahult-github
Copy link
Contributor Author

currently ble_gap_periodic_adv_set() assumes that data is always valid and I'm not sure on how this patch would work since *data is dereferenced mutliple times later on in the function

Thanks for pointing this out. I have updated the patch to copy data only if len is valid . ( in the cases where length can be zero )

@rahult-github rahult-github requested a review from sjanc July 4, 2023 10:20
@rahult-github rahult-github force-pushed the bugfix/handle_NULL_data_periodic_adv branch from c04b6ae to 4bc16db Compare September 5, 2023 07:23
@rahult-github
Copy link
Contributor Author

Hi @sjanc , please let me know if there are any more comments.

@rahult-github rahult-github force-pushed the bugfix/handle_NULL_data_periodic_adv branch from 4bc16db to 8f97e63 Compare September 21, 2023 05:46
   This change sets length field only if data is non NULL, else sets
   length to zero.
@rahult-github rahult-github force-pushed the bugfix/handle_NULL_data_periodic_adv branch from 8f97e63 to e298bd8 Compare October 3, 2023 06:05
@sjanc sjanc merged commit 05dd20c into apache:master Oct 3, 2023
8 checks passed
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.

2 participants