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

BLE: No Valid Parameter check in send_conn_le_param_update() #21057

Closed
kipm-ot opened this issue Nov 28, 2019 · 4 comments · Fixed by #21220
Closed

BLE: No Valid Parameter check in send_conn_le_param_update() #21057

kipm-ot opened this issue Nov 28, 2019 · 4 comments · Fixed by #21220
Assignees
Labels
area: Bluetooth bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@kipm-ot
Copy link
Contributor

kipm-ot commented Nov 28, 2019

According to Core spec v5.1 vol 3 part A section 4.20: "CONNECTION PARAMETER UPDATE REQUEST", data fields in the Connection Parameter update request are Interval Min , Interval Max, Slave Latency and Timeout Multiplier. And their possible value ranges are as follows.

Interval Min : 6 to 3200
Interval Max : 6 to 3200
Slave Latency : 0 to 499
Timeout Multiplier : 10 to 3200.

But in send_conn_le_param_update() in conn.c, we are not checking the input parameter value ranges which cause issues as below.

Problem:
In conn_update_timeout(), When CONFIG_BT_GAP_PERIPHERAL_PREF_PARAMS is enabled it will take the above parameters (Interval Min , Interval Max, Slave Latency and Timeout Multiplier) from the Characteristics and accordingto Peripheral Preferred Connection Parameters characteristic Interval Min and Interval Max can have a value 0xFFFF (which means server do not have specific min/max values, master can decide).And when 0xFFFF is invalid in send_conn_le_param_update() which causes issues as Master do not expects 0xFFFF.

Solution:
In, send_conn_le_param_update(), we will have to add checks for the param values and then only send the update request. Ideally in the above case, if the values are fetched from
CONFIG_BT_GAP_PERIPHERAL_PREF_PARAMS which contains 0xFFFF then probabaly we need to send request with internal values (eg. conn->le.interval_min)

@kipm-ot kipm-ot added the bug The issue is a bug, or the PR is fixing a bug label Nov 28, 2019
@jhedberg
Copy link
Member

@Vudentz do you wanna look at this? It seems you introduced the 0xffff values in commit 593d856

@jhedberg
Copy link
Member

@kipm-ot @aescolar what do you think the right policy should be? If any of the values is set to 0xffff we don't initiate param update triggered by the timer?

@jhedberg jhedberg added the priority: low Low impact/importance bug label Nov 28, 2019
@jhedberg
Copy link
Member

Sorry, just saw your proposed solution in the description @kipm-ot. Sounds reasonable to me. Are you planning to submit a PR?

@kipm-ot
Copy link
Contributor Author

kipm-ot commented Nov 29, 2019

@jhedberg No worries, yes we will do a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants