Skip to content

[nrf fromlist] Bluetooth: Host: Add req/rsp l2cap validation #3172

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
+49 −19
Closed

[nrf fromlist] Bluetooth: Host: Add req/rsp l2cap validation #3172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/zephyr/bluetooth/l2cap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,9 @@
uint16_t psm;
/** Helps match request context during CoC */
uint8_t ident;
/** Opcode of the pending request. Used to match responses with requests. */
uint8_t pending_req;
bt_security_t required_sec_level;

Check notice on line 269 in include/zephyr/bluetooth/l2cap.h

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

include/zephyr/bluetooth/l2cap.h:269 
-	uint8_t                         pending_req;
+	uint8_t pending_req;

/* Response Timeout eXpired (RTX) timer */
struct k_work_delayable rtx_work;
Expand Down
66 changes: 47 additions & 19 deletions subsys/bluetooth/host/l2cap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,11 @@
sizeof(struct bt_l2cap_disconn_req)),
CONFIG_BT_CONN_TX_USER_DATA_SIZE, NULL);

#define l2cap_lookup_ident(conn, ident) __l2cap_lookup_ident(conn, ident, false)
#define l2cap_remove_ident(conn, ident) __l2cap_lookup_ident(conn, ident, true)
#define ANY_OPCODE 0x100
#define l2cap_lookup_ident(conn, ident, req_opcode) \
__l2cap_lookup_ident(conn, ident, req_opcode, false)
#define l2cap_remove_ident(conn, ident, req_opcode) \
__l2cap_lookup_ident(conn, ident, req_opcode, true)

static sys_slist_t servers = SYS_SLIST_STATIC_INIT(&servers);
#endif /* CONFIG_BT_L2CAP_DYNAMIC_CHANNEL */
Expand Down Expand Up @@ -139,13 +142,15 @@
}

static struct bt_l2cap_le_chan *
__l2cap_lookup_ident(struct bt_conn *conn, uint16_t ident, bool remove)
__l2cap_lookup_ident(struct bt_conn *conn, uint16_t ident, uint16_t req_opcode, bool remove)
{

Check notice on line 146 in subsys/bluetooth/host/l2cap.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

subsys/bluetooth/host/l2cap.c:146 
-static struct bt_l2cap_le_chan *
-__l2cap_lookup_ident(struct bt_conn *conn, uint16_t ident, uint16_t req_opcode, bool remove)
+static struct bt_l2cap_le_chan *__l2cap_lookup_ident(struct bt_conn *conn, uint16_t ident,
+						     uint16_t req_opcode, bool remove)
struct bt_l2cap_chan *chan;
sys_snode_t *prev = NULL;

SYS_SLIST_FOR_EACH_CONTAINER(&conn->channels, chan, node) {
if (BT_L2CAP_LE_CHAN(chan)->ident == ident) {
if ((BT_L2CAP_LE_CHAN(chan)->ident == ident) &&
((BT_L2CAP_LE_CHAN(chan)->pending_req == req_opcode) ||
(req_opcode == ANY_OPCODE))) {
if (remove) {
sys_slist_remove(&conn->channels, prev,
&chan->node);
Expand Down Expand Up @@ -303,7 +308,7 @@
bt_l2cap_chan_del(&chan->chan);

/* Remove other channels if pending on the same ident */
while ((chan = l2cap_remove_ident(conn, chan->ident))) {
while ((chan = l2cap_remove_ident(conn, chan->ident, chan->pending_req))) {
bt_l2cap_chan_del(&chan->chan);
}
}
Expand Down Expand Up @@ -523,15 +528,23 @@
{
struct net_buf *buf;
struct bt_l2cap_le_conn_req *req;
uint8_t ident;

ch->ident = get_ident();
ident = get_ident();

buf = l2cap_create_le_sig_pdu(BT_L2CAP_LE_CONN_REQ,
ch->ident, sizeof(*req));
buf = l2cap_create_le_sig_pdu(BT_L2CAP_LE_CONN_REQ, ident, sizeof(*req));
if (!buf) {
return -ENOMEM;
}

/* TODO Ident handling/setting should ideally be done in l2cap_chan_send_req after the
* request is successfully sent on the channel but will require special considerations for
* functions such as l2cap_ecred_conn_req and bt_l2cap_ecred_chan_reconfigure where the
* ident is set for multiple channels, but the request is only sent on one.
*/
ch->ident = ident;
ch->pending_req = BT_L2CAP_LE_CONN_REQ;

req = net_buf_add(buf, sizeof(*req));
req->psm = sys_cpu_to_le16(ch->psm);
req->scid = sys_cpu_to_le16(ch->rx.cid);
Expand Down Expand Up @@ -590,6 +603,7 @@
"The MTU shall be the same for channels in the same request.");

ch->ident = ident;
ch->pending_req = BT_L2CAP_ECRED_CONN_REQ;

net_buf_add_le16(buf, ch->rx.cid);
}
Expand Down Expand Up @@ -1785,7 +1799,7 @@
rsp = net_buf_pull_mem(buf, sizeof(*rsp));
result = sys_le16_to_cpu(rsp->result);

while ((ch = l2cap_lookup_ident(conn, ident))) {
while ((ch = l2cap_lookup_ident(conn, ident, BT_L2CAP_ECRED_RECONF_REQ))) {
/* Stop timer started on REQ send. The timer is only set on one
* of the channels, but we don't want to make assumptions on
* which one it is.
Expand All @@ -1798,6 +1812,7 @@

ch->pending_rx_mtu = 0;
ch->ident = 0U;
ch->pending_req = 0U;

if (ch->chan.ops->reconfigured) {
ch->chan.ops->reconfigured(&ch->chan);
Expand Down Expand Up @@ -1943,7 +1958,7 @@

LOG_DBG("mtu 0x%04x mps 0x%04x credits 0x%04x result %u", mtu, mps, credits, result);

chan = l2cap_lookup_ident(conn, ident);
chan = l2cap_lookup_ident(conn, ident, BT_L2CAP_ECRED_CONN_REQ);
if (chan) {
psm = chan->psm;
} else {
Expand All @@ -1953,7 +1968,7 @@
switch (result) {
case BT_L2CAP_LE_ERR_AUTHENTICATION:
case BT_L2CAP_LE_ERR_ENCRYPTION:
while ((chan = l2cap_lookup_ident(conn, ident))) {
while ((chan = l2cap_lookup_ident(conn, ident, BT_L2CAP_ECRED_CONN_REQ))) {

/* Cancel RTX work */
k_work_cancel_delayable(&chan->rtx_work);
Expand All @@ -1973,7 +1988,7 @@
case BT_L2CAP_LE_ERR_SCID_IN_USE:
/* Some connections refused – not enough resources available */
case BT_L2CAP_LE_ERR_NO_RESOURCES:
while ((chan = l2cap_lookup_ident(conn, ident))) {
while ((chan = l2cap_lookup_ident(conn, ident, BT_L2CAP_ECRED_CONN_REQ))) {
struct bt_l2cap_chan *c;

/* Cancel RTX work */
Expand Down Expand Up @@ -2029,6 +2044,7 @@
chan->tx.cid = dcid;

chan->ident = 0U;
chan->pending_req = 0U;

chan->tx.mtu = mtu;
chan->tx.mps = mps;
Expand All @@ -2049,7 +2065,7 @@
break;
case BT_L2CAP_LE_ERR_PSM_NOT_SUPP:
default:
while ((chan = l2cap_remove_ident(conn, ident))) {
while ((chan = l2cap_remove_ident(conn, ident, BT_L2CAP_ECRED_CONN_REQ))) {
bt_l2cap_chan_del(&chan->chan);
}
break;
Expand Down Expand Up @@ -2088,9 +2104,9 @@
if (result == BT_L2CAP_LE_SUCCESS ||
result == BT_L2CAP_LE_ERR_AUTHENTICATION ||
result == BT_L2CAP_LE_ERR_ENCRYPTION) {
chan = l2cap_lookup_ident(conn, ident);
chan = l2cap_lookup_ident(conn, ident, BT_L2CAP_LE_CONN_REQ);
} else {
chan = l2cap_remove_ident(conn, ident);
chan = l2cap_remove_ident(conn, ident, BT_L2CAP_LE_CONN_REQ);
}

if (!chan) {
Expand All @@ -2103,6 +2119,7 @@

/* Reset ident since it got a response */
chan->ident = 0U;
chan->pending_req = 0U;

switch (result) {
case BT_L2CAP_LE_SUCCESS:
Expand Down Expand Up @@ -2162,6 +2179,12 @@
return;
}

chan = l2cap_lookup_ident(conn, ident, BT_L2CAP_DISCONN_REQ);
if (!chan) {
LOG_ERR("Cannot find channel for ident %u", ident);
return;
}

scid = sys_le16_to_cpu(rsp->scid);

LOG_DBG("dcid 0x%04x scid 0x%04x", sys_le16_to_cpu(rsp->dcid), scid);
Expand Down Expand Up @@ -2230,7 +2253,7 @@
struct bt_conn *conn = l2cap->chan.chan.conn;
struct bt_l2cap_le_chan *chan;

while ((chan = l2cap_remove_ident(conn, ident))) {
while ((chan = l2cap_remove_ident(conn, ident, ANY_OPCODE))) {
bt_l2cap_chan_del(&chan->chan);
}
}
Expand Down Expand Up @@ -3097,6 +3120,7 @@
ch = BT_L2CAP_LE_CHAN(chans[j]);

ch->ident = ident;
ch->pending_req = BT_L2CAP_ECRED_RECONF_REQ;
ch->pending_rx_mtu = mtu;

net_buf_add_le16(buf, ch->rx.cid);
Expand Down Expand Up @@ -3184,6 +3208,7 @@
ch = BT_L2CAP_LE_CHAN(chans[i]);

ch->ident = ident;
ch->pending_req = BT_L2CAP_ECRED_RECONF_REQ;
ch->pending_rx_mtu = mtu;

net_buf_add_le16(buf, ch->rx.cid);
Expand Down Expand Up @@ -3236,6 +3261,7 @@
struct net_buf *buf;
struct bt_l2cap_disconn_req *req;
struct bt_l2cap_le_chan *le_chan;
uint8_t ident;

if (!conn) {
return -ENOTCONN;
Expand All @@ -3250,14 +3276,16 @@

LOG_DBG("chan %p scid 0x%04x dcid 0x%04x", chan, le_chan->rx.cid, le_chan->tx.cid);

le_chan->ident = get_ident();
ident = get_ident();

buf = l2cap_create_le_sig_pdu(BT_L2CAP_DISCONN_REQ,
le_chan->ident, sizeof(*req));
buf = l2cap_create_le_sig_pdu(BT_L2CAP_DISCONN_REQ, ident, sizeof(*req));
if (!buf) {
return -ENOMEM;
}

le_chan->ident = ident;
le_chan->pending_req = BT_L2CAP_DISCONN_REQ;

req = net_buf_add(buf, sizeof(*req));
req->dcid = sys_cpu_to_le16(le_chan->tx.cid);
req->scid = sys_cpu_to_le16(le_chan->rx.cid);
Expand Down
Loading