Bluetooth: ISO: Fix issue with CIG being terminated #97285
+1
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the case of a CIG having multiple CIS, and all CIS has been requested to being disconnected (i.e. they all enter the BT_ISO_STATE_DISCONNECTING state), then when the first disconnect complete is handled in bt_iso_chan_disconnected, then the cig->state was prematurely set to BT_ISO_CIG_STATE_INACTIVE. This meant that if the application called bt_iso_cig_terminate when the 2nd CIS entered bt_iso_chan_disconnected and called chan->ops->disconnected(chan, reason) then the CIG would be removed. When the CIS then entered bt_iso_cleanup_acl, it would access removed data from cleanup_cig. Change bt_iso_chan_disconnected to not allow the termination of the CIG until all CIS have entered the BT_ISO_STATE_DISCONNECTED state. Signed-off-by: Emil Gydesen <[email protected]>
zephyrbot
added
area: Bluetooth
area: Bluetooth ISO
zephyrbot
requested review from
alwa-nordic,
cvinayak,
HaavardRei,
hermabe,
jhedberg,
nashif,
PavelVPV and
rugeGerritsen
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a race condition in Bluetooth ISO CIG (Connected Isochronous Group) termination where a CIG could be prematurely terminated while CIS (Connected Isochronous Stream) channels were still disconnecting, leading to potential access of freed memory.
- Modified the CIG termination logic to wait for all CIS channels to reach
BT_ISO_STATE_DISCONNECTEDbefore allowing termination - Added test coverage to verify that
bt_iso_cig_terminatecorrectly fails when CIS channels are still connected
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| subsys/bluetooth/host/iso.c | Fixed the state check to prevent premature CIG termination by waiting for all channels to be fully disconnected |
| tests/bsim/bluetooth/host/iso/cis/src/cis_central.c | Added test logic to track connected channels and verify CIG termination behavior |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Thalley
force-pushed
the
iso_disconnect_cig_state_fix
branch
from
94a2905 to
b75f069
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the case of a CIG having multiple CIS, and all CIS has been requested to being disconnected (i.e. they all enter the BT_ISO_STATE_DISCONNECTING state), then when the first disconnect complete is handled in bt_iso_chan_disconnected, then the cig->state was prematurely set to BT_ISO_CIG_STATE_INACTIVE. This meant that if the application called bt_iso_cig_terminate when the 2nd CIS entered bt_iso_chan_disconnected and called chan->ops->disconnected(chan, reason) then the CIG would be removed. When the CIS then entered bt_iso_cleanup_acl, it would access removed data from cleanup_cig.
Change bt_iso_chan_disconnected to not allow the termination of the CIG until all CIS have entered the BT_ISO_STATE_DISCONNECTED state.
fixes #96155