Skip to content

Commit 7accb1c

Browse files
committed
Bluetooth: L2CAP: Fix invalid response to L2CAP_ECRED_RECONF_REQ
This fixes responding with an invalid result caused by checking the wrong size of CID which should have been (cmd_len - sizeof(*req)) and on top of it the wrong result was use L2CAP_CR_LE_INVALID_PARAMS which is invalid/reserved for reconf when running test like L2CAP/ECFC/BI-03-C: > ACL Data RX: Handle 64 flags 0x02 dlen 14 LE L2CAP: Enhanced Credit Reconfigure Request (0x19) ident 2 len 6 MTU: 64 MPS: 64 Source CID: 64 < ACL Data TX: Handle 64 flags 0x00 dlen 10 LE L2CAP: Enhanced Credit Reconfigure Respond (0x1a) ident 2 len 2 ! Result: Reserved (0x000c) Result: Reconfiguration failed - one or more Destination CIDs invalid (0x0003) Fiix L2CAP/ECFC/BI-04-C which expects L2CAP_RECONF_INVALID_MPS (0x0002) when more than one channel gets its MPS reduced: > ACL Data RX: Handle 64 flags 0x02 dlen 16 LE L2CAP: Enhanced Credit Reconfigure Request (0x19) ident 2 len 8 MTU: 264 MPS: 99 Source CID: 64 ! Source CID: 65 < ACL Data TX: Handle 64 flags 0x00 dlen 10 LE L2CAP: Enhanced Credit Reconfigure Respond (0x1a) ident 2 len 2 ! Result: Reconfiguration successful (0x0000) Result: Reconfiguration failed - reduction in size of MPS not allowed for more than one channel at a time (0x0002) Fix L2CAP/ECFC/BI-05-C when SCID is invalid (85 unconnected): > ACL Data RX: Handle 64 flags 0x02 dlen 14 LE L2CAP: Enhanced Credit Reconfigure Request (0x19) ident 2 len 6 MTU: 65 MPS: 64 ! Source CID: 85 < ACL Data TX: Handle 64 flags 0x00 dlen 10 LE L2CAP: Enhanced Credit Reconfigure Respond (0x1a) ident 2 len 2 ! Result: Reconfiguration successful (0x0000) Result: Reconfiguration failed - one or more Destination CIDs invalid (0x0003) Fix L2CAP/ECFC/BI-06-C when MPS < L2CAP_ECRED_MIN_MPS (64): > ACL Data RX: Handle 64 flags 0x02 dlen 14 LE L2CAP: Enhanced Credit Reconfigure Request (0x19) ident 2 len 6 MTU: 672 ! MPS: 63 Source CID: 64 < ACL Data TX: Handle 64 flags 0x00 dlen 10 LE L2CAP: Enhanced Credit Reconfigure Respond (0x1a) ident 2 len 2 ! Result: Reconfiguration failed - reduction in size of MPS not allowed for more than one channel at a time (0x0002) Result: Reconfiguration failed - other unacceptable parameters (0x0004) Fix L2CAP/ECFC/BI-07-C when MPS reduced for more than one channel: > ACL Data RX: Handle 64 flags 0x02 dlen 16 LE L2CAP: Enhanced Credit Reconfigure Request (0x19) ident 3 len 8 MTU: 84 ! MPS: 71 Source CID: 64 ! Source CID: 65 < ACL Data TX: Handle 64 flags 0x00 dlen 10 LE L2CAP: Enhanced Credit Reconfigure Respond (0x1a) ident 2 len 2 ! Result: Reconfiguration successful (0x0000) Result: Reconfiguration failed - reduction in size of MPS not allowed for more than one channel at a time (0x0002) Link: bluez/bluez#1865 Fixes: 15f02b9 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode") Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
1 parent d4f687f commit 7accb1c

2 files changed

Lines changed: 47 additions & 18 deletions

File tree

include/net/bluetooth/l2cap.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,8 @@ struct l2cap_ecred_reconf_req {
493493
#define L2CAP_RECONF_SUCCESS 0x0000
494494
#define L2CAP_RECONF_INVALID_MTU 0x0001
495495
#define L2CAP_RECONF_INVALID_MPS 0x0002
496+
#define L2CAP_RECONF_INVALID_CID 0x0003
497+
#define L2CAP_RECONF_INVALID_PARAMS 0x0004
496498

497499
struct l2cap_ecred_reconf_rsp {
498500
__le16 result;

net/bluetooth/l2cap_core.c

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5310,14 +5310,14 @@ static inline int l2cap_ecred_reconf_req(struct l2cap_conn *conn,
53105310
struct l2cap_ecred_reconf_req *req = (void *) data;
53115311
struct l2cap_ecred_reconf_rsp rsp;
53125312
u16 mtu, mps, result;
5313-
struct l2cap_chan *chan;
5313+
struct l2cap_chan *chan[L2CAP_ECRED_MAX_CID] = {};
53145314
int i, num_scid;
53155315

53165316
if (!enable_ecred)
53175317
return -EINVAL;
53185318

5319-
if (cmd_len < sizeof(*req) || cmd_len - sizeof(*req) % sizeof(u16)) {
5320-
result = L2CAP_CR_LE_INVALID_PARAMS;
5319+
if (cmd_len < sizeof(*req) || (cmd_len - sizeof(*req)) % sizeof(u16)) {
5320+
result = L2CAP_RECONF_INVALID_CID;
53215321
goto respond;
53225322
}
53235323

@@ -5327,42 +5327,69 @@ static inline int l2cap_ecred_reconf_req(struct l2cap_conn *conn,
53275327
BT_DBG("mtu %u mps %u", mtu, mps);
53285328

53295329
if (mtu < L2CAP_ECRED_MIN_MTU) {
5330-
result = L2CAP_RECONF_INVALID_MTU;
5330+
result = L2CAP_RECONF_INVALID_PARAMS;
53315331
goto respond;
53325332
}
53335333

53345334
if (mps < L2CAP_ECRED_MIN_MPS) {
5335-
result = L2CAP_RECONF_INVALID_MPS;
5335+
result = L2CAP_RECONF_INVALID_PARAMS;
53365336
goto respond;
53375337
}
53385338

53395339
cmd_len -= sizeof(*req);
53405340
num_scid = cmd_len / sizeof(u16);
5341+
5342+
if (num_scid > L2CAP_ECRED_MAX_CID) {
5343+
result = L2CAP_RECONF_INVALID_PARAMS;
5344+
goto respond;
5345+
}
5346+
53415347
result = L2CAP_RECONF_SUCCESS;
53425348

5349+
/* Check if each SCID, MTU and MPS are valid */
53435350
for (i = 0; i < num_scid; i++) {
53445351
u16 scid;
53455352

53465353
scid = __le16_to_cpu(req->scid[i]);
5347-
if (!scid)
5348-
return -EPROTO;
5354+
if (!scid) {
5355+
result = L2CAP_RECONF_INVALID_CID;
5356+
goto respond;
5357+
}
53495358

5350-
chan = __l2cap_get_chan_by_dcid(conn, scid);
5351-
if (!chan)
5352-
continue;
5359+
chan[i] = __l2cap_get_chan_by_dcid(conn, scid);
5360+
if (!chan[i]) {
5361+
result = L2CAP_RECONF_INVALID_CID;
5362+
goto respond;
5363+
}
53535364

5354-
/* If the MTU value is decreased for any of the included
5355-
* channels, then the receiver shall disconnect all
5356-
* included channels.
5365+
/* The MTU field shall be greater than or equal to the greatest
5366+
* current MTU size of these channels.
53575367
*/
5358-
if (chan->omtu > mtu) {
5359-
BT_ERR("chan %p decreased MTU %u -> %u", chan,
5360-
chan->omtu, mtu);
5368+
if (chan[i]->omtu > mtu) {
5369+
BT_ERR("chan %p decreased MTU %u -> %u", chan[i],
5370+
chan[i]->omtu, mtu);
53615371
result = L2CAP_RECONF_INVALID_MTU;
5372+
goto respond;
53625373
}
53635374

5364-
chan->omtu = mtu;
5365-
chan->remote_mps = mps;
5375+
/* If more than one channel is being configured, the MPS field
5376+
* shall be greater than or equal to the current MPS size of
5377+
* each of these channels. If only one channel is being
5378+
* configured, the MPS field may be less than the current MPS
5379+
* of that channel.
5380+
*/
5381+
if (chan[i]->remote_mps >= mps && i) {
5382+
BT_ERR("chan %p decreased MPS %u -> %u", chan[i],
5383+
chan[i]->remote_mps, mps);
5384+
result = L2CAP_RECONF_INVALID_MPS;
5385+
goto respond;
5386+
}
5387+
}
5388+
5389+
/* Commit the new MTU and MPS values after checking they are valid */
5390+
for (i = 0; i < num_scid; i++) {
5391+
chan[i]->omtu = mtu;
5392+
chan[i]->remote_mps = mps;
53665393
}
53675394

53685395
respond:

0 commit comments

Comments
 (0)