Skip to content

host/l2cap: disconnect if received packet is larger than MPS#1003

Merged
sjanc merged 1 commit intoapache:masterfrom
KKopyscinski:ble_l2cap_rx_fix
Apr 13, 2022
Merged

host/l2cap: disconnect if received packet is larger than MPS#1003
sjanc merged 1 commit intoapache:masterfrom
KKopyscinski:ble_l2cap_rx_fix

Conversation

@KKopyscinski
Copy link
Copy Markdown
Contributor

Peer sending packet larger than MPS is invalid, and should be met
with L2CAP channel disconnection.

This affects L2CAP/LE/CFC/BV-27-C

Comment thread nimble/host/src/ble_l2cap.c Outdated
goto err;
}

if (l2cap_hdr.len > chan->my_coc_mps) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what about ATT ? in such a case chan->my_coc_mps is 0 I believe. Or not?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think you are right. What's more, I think only dynamic range CIDs use my_coc_mps field, and for others MPS=MTU of channel, which varies from channel to channel. I added 3 cases for BLE_L2CAP_CID_ATT, BLE_L2CAP_CID_SIG and BLE_L2CAP_CID_SM.

@KKopyscinski KKopyscinski force-pushed the ble_l2cap_rx_fix branch 2 times, most recently from 23e7100 to 3892443 Compare July 22, 2021 07:01
Comment thread nimble/host/src/ble_l2cap.c Outdated
goto err;
}

if ((chan->dcid >= 0x0040 && chan->dcid <= 0x007F && l2cap_hdr.len > chan->my_coc_mps) ||
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@KKopyscinski don't you want just add ble_l2cap_disconnect(chan); after line 413 ?

@KKopyscinski KKopyscinski requested a review from sjanc September 21, 2021 11:25
Peer sending packet larger than MPS is invalid, and should be met
with L2CAP channel disconnection.

This affects L2CAP/LE/CFC/BV-27-C
@KKopyscinski
Copy link
Copy Markdown
Contributor Author

@sjanc as discussed offline, only CIDs from dynamic range are checked now

@sjanc sjanc merged commit 09466ab into apache:master Apr 13, 2022
@KKopyscinski KKopyscinski deleted the ble_l2cap_rx_fix branch February 13, 2024 07:58
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.

3 participants