Skip to content

Commit 5beb5b5

Browse files
Vudentzcarlescufi
Vudentz
authored and
carlescufi
committed
Bluetooth: ATT: Fix not handling errors properly
Since bt_conn_send_cb can fail to send buffer causing it to unref this may cause buffer leaks as the caller is not aware of the error assuming the buffer could be sent. Signed-off-by: Luiz Augusto von Dentz <[email protected]>
1 parent c9e0876 commit 5beb5b5

File tree

4 files changed

+57
-52
lines changed

4 files changed

+57
-52
lines changed

‎subsys/bluetooth/host/att.c‎

Lines changed: 42 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -130,14 +130,14 @@ static int att_send(struct bt_conn *conn, struct net_buf *buf,
130130
err = bt_smp_sign(conn, buf);
131131
if (err) {
132132
BT_ERR("Error signing data");
133+
net_buf_unref(buf);
133134
return err;
134135
}
135136
}
136137

137-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, buf, cb ? cb : att_cb(buf),
138-
user_data);
139-
140-
return 0;
138+
return bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, buf,
139+
cb ? cb : att_cb(buf),
140+
user_data);
141141
}
142142

143143
void att_pdu_sent(struct bt_conn *conn, void *user_data)
@@ -158,8 +158,6 @@ void att_pdu_sent(struct bt_conn *conn, void *user_data)
158158
if (!att_send(conn, buf, NULL, NULL)) {
159159
return;
160160
}
161-
/* If the buffer cannot be send unref it */
162-
net_buf_unref(buf);
163161
}
164162

165163
k_sem_give(&att->tx_sem);
@@ -241,7 +239,7 @@ static void send_err_rsp(struct bt_conn *conn, u8_t req, u16_t handle,
241239
rsp->handle = sys_cpu_to_le16(handle);
242240
rsp->error = err;
243241

244-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, buf, att_rsp_sent, NULL);
242+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, buf, att_rsp_sent, NULL);
245243
}
246244

247245
static u8_t att_mtu_req(struct bt_att *att, struct net_buf *buf)
@@ -275,7 +273,7 @@ static u8_t att_mtu_req(struct bt_att *att, struct net_buf *buf)
275273
rsp = net_buf_add(pdu, sizeof(*rsp));
276274
rsp->mtu = sys_cpu_to_le16(mtu_server);
277275

278-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, pdu, att_rsp_sent, NULL);
276+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, pdu, att_rsp_sent, NULL);
279277

280278
/* BLUETOOTH SPECIFICATION Version 4.2 [Vol 3, Part F] page 484:
281279
*
@@ -297,6 +295,8 @@ static inline bool att_is_connected(struct bt_att *att)
297295

298296
static int att_send_req(struct bt_att *att, struct bt_att_req *req)
299297
{
298+
int err;
299+
300300
__ASSERT_NO_MSG(req);
301301
__ASSERT_NO_MSG(req->func);
302302
__ASSERT_NO_MSG(!att->req);
@@ -314,8 +314,13 @@ static int att_send_req(struct bt_att *att, struct bt_att_req *req)
314314
net_buf_simple_save(&req->buf->b, &req->state);
315315

316316
/* Keep a reference for resending in case of an error */
317-
bt_l2cap_send_cb(att->chan.chan.conn, BT_L2CAP_CID_ATT,
318-
net_buf_ref(req->buf), att_cb(req->buf), NULL);
317+
err = bt_l2cap_send_cb(att->chan.chan.conn, BT_L2CAP_CID_ATT,
318+
net_buf_ref(req->buf), att_cb(req->buf), NULL);
319+
if (err) {
320+
net_buf_unref(req->buf);
321+
req->buf = NULL;
322+
return err;
323+
}
319324

320325
return 0;
321326
}
@@ -523,7 +528,8 @@ static u8_t att_find_info_rsp(struct bt_att *att, u16_t start_handle,
523528
return 0;
524529
}
525530

526-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent, NULL);
531+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent,
532+
NULL);
527533

528534
return 0;
529535
}
@@ -669,7 +675,8 @@ static u8_t att_find_type_rsp(struct bt_att *att, u16_t start_handle,
669675
return 0;
670676
}
671677

672-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent, NULL);
678+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent,
679+
NULL);
673680

674681
return 0;
675682
}
@@ -830,7 +837,8 @@ static u8_t att_read_type_rsp(struct bt_att *att, struct bt_uuid *uuid,
830837
return 0;
831838
}
832839

833-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent, NULL);
840+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent,
841+
NULL);
834842

835843
return 0;
836844
}
@@ -953,7 +961,8 @@ static u8_t att_read_rsp(struct bt_att *att, u8_t op, u8_t rsp, u16_t handle,
953961
return 0;
954962
}
955963

956-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent, NULL);
964+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent,
965+
NULL);
957966

958967
return 0;
959968
}
@@ -1031,7 +1040,8 @@ static u8_t att_read_mult_req(struct bt_att *att, struct net_buf *buf)
10311040
}
10321041
}
10331042

1034-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent, NULL);
1043+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent,
1044+
NULL);
10351045

10361046
return 0;
10371047
}
@@ -1136,7 +1146,8 @@ static u8_t att_read_group_rsp(struct bt_att *att, struct bt_uuid *uuid,
11361146
return 0;
11371147
}
11381148

1139-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent, NULL);
1149+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent,
1150+
NULL);
11401151

11411152
return 0;
11421153
}
@@ -1280,8 +1291,8 @@ static u8_t att_write_rsp(struct bt_conn *conn, u8_t req, u8_t rsp,
12801291
}
12811292

12821293
if (data.buf) {
1283-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf,
1284-
att_rsp_sent, NULL);
1294+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf,
1295+
att_rsp_sent, NULL);
12851296
}
12861297

12871298
return 0;
@@ -1406,7 +1417,8 @@ static u8_t att_prep_write_rsp(struct bt_att *att, u16_t handle, u16_t offset,
14061417
net_buf_add(data.buf, len);
14071418
memcpy(rsp->value, value, len);
14081419

1409-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent, NULL);
1420+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, data.buf, att_rsp_sent,
1421+
NULL);
14101422

14111423
return 0;
14121424
}
@@ -1469,7 +1481,7 @@ static u8_t att_exec_write_rsp(struct bt_att *att, u8_t flags)
14691481
return BT_ATT_ERR_UNLIKELY;
14701482
}
14711483

1472-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, buf, att_rsp_sent, NULL);
1484+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, buf, att_rsp_sent, NULL);
14731485

14741486
return 0;
14751487
}
@@ -1739,7 +1751,7 @@ static u8_t att_indicate(struct bt_att *att, struct net_buf *buf)
17391751
return 0;
17401752
}
17411753

1742-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, buf, att_cfm_sent, NULL);
1754+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, buf, att_cfm_sent, NULL);
17431755

17441756
return 0;
17451757
}
@@ -2187,8 +2199,8 @@ static void bt_att_encrypt_change(struct bt_l2cap_chan *chan,
21872199
BT_DBG("Retrying");
21882200

21892201
/* Resend buffer */
2190-
bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, att->req->buf,
2191-
att_cb(att->req->buf), NULL);
2202+
(void)bt_l2cap_send_cb(conn, BT_L2CAP_CID_ATT, att->req->buf,
2203+
att_cb(att->req->buf), NULL);
21922204
att->req->buf = NULL;
21932205
}
21942206
#endif /* CONFIG_BT_SMP */
@@ -2255,12 +2267,12 @@ int bt_att_send(struct bt_conn *conn, struct net_buf *buf, bt_conn_tx_cb_t cb,
22552267
struct bt_att *att;
22562268
int err;
22572269

2258-
if (!conn || !buf) {
2259-
return -EINVAL;
2260-
}
2270+
__ASSERT_NO_MSG(conn);
2271+
__ASSERT_NO_MSG(buf);
22612272

22622273
att = att_chan_get(conn);
22632274
if (!att) {
2275+
net_buf_unref(buf);
22642276
return -ENOTCONN;
22652277
}
22662278

@@ -2290,12 +2302,13 @@ int bt_att_req_send(struct bt_conn *conn, struct bt_att_req *req)
22902302

22912303
BT_DBG("conn %p req %p", conn, req);
22922304

2293-
if (!conn || !req) {
2294-
return -EINVAL;
2295-
}
2305+
__ASSERT_NO_MSG(conn);
2306+
__ASSERT_NO_MSG(req);
22962307

22972308
att = att_chan_get(conn);
22982309
if (!att) {
2310+
net_buf_unref(req->buf);
2311+
req->buf = NULL;
22992312
return -ENOTCONN;
23002313
}
23012314

‎subsys/bluetooth/host/gatt.c‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1552,20 +1552,6 @@ struct notify_data {
15521552
};
15531553
};
15541554

1555-
static int gatt_send_cb(struct bt_conn *conn, struct net_buf *buf,
1556-
bt_conn_tx_cb_t cb, void *user_data)
1557-
{
1558-
int err;
1559-
1560-
err = bt_att_send(conn, buf, cb, user_data);
1561-
if (err) {
1562-
net_buf_unref(buf);
1563-
return err;
1564-
}
1565-
1566-
return 0;
1567-
}
1568-
15691555
static int gatt_notify(struct bt_conn *conn, u16_t handle,
15701556
struct bt_gatt_notify_params *params)
15711557
{
@@ -1598,7 +1584,7 @@ static int gatt_notify(struct bt_conn *conn, u16_t handle,
15981584
net_buf_add(buf, params->len);
15991585
memcpy(nfy->value, params->data, params->len);
16001586

1601-
return gatt_send_cb(conn, buf, params->func, params->user_data);
1587+
return bt_att_send(conn, buf, params->func, params->user_data);
16021588
}
16031589

16041590
static void gatt_indicate_rsp(struct bt_conn *conn, u8_t err,
@@ -1629,7 +1615,6 @@ static int gatt_send(struct bt_conn *conn, struct net_buf *buf,
16291615

16301616
if (err) {
16311617
BT_ERR("Error sending ATT PDU: %d", err);
1632-
net_buf_unref(buf);
16331618
}
16341619

16351620
return err;
@@ -3286,7 +3271,7 @@ int bt_gatt_write_without_response_cb(struct bt_conn *conn, u16_t handle,
32863271

32873272
BT_DBG("handle 0x%04x length %u", handle, length);
32883273

3289-
return gatt_send_cb(conn, buf, func, user_data);
3274+
return bt_att_send(conn, buf, func, user_data);
32903275
}
32913276

32923277
static int gatt_exec_write(struct bt_conn *conn,

‎subsys/bluetooth/host/l2cap.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,8 @@ struct net_buf *bt_l2cap_create_pdu_timeout(struct net_buf_pool *pool,
483483
timeout);
484484
}
485485

486-
void bt_l2cap_send_cb(struct bt_conn *conn, u16_t cid, struct net_buf *buf,
487-
bt_conn_tx_cb_t cb, void *user_data)
486+
int bt_l2cap_send_cb(struct bt_conn *conn, u16_t cid, struct net_buf *buf,
487+
bt_conn_tx_cb_t cb, void *user_data)
488488
{
489489
struct bt_l2cap_hdr *hdr;
490490

@@ -494,7 +494,7 @@ void bt_l2cap_send_cb(struct bt_conn *conn, u16_t cid, struct net_buf *buf,
494494
hdr->len = sys_cpu_to_le16(buf->len - sizeof(*hdr));
495495
hdr->cid = sys_cpu_to_le16(cid);
496496

497-
bt_conn_send_cb(conn, buf, cb, user_data);
497+
return bt_conn_send_cb(conn, buf, cb, user_data);
498498
}
499499

500500
static void l2cap_send_reject(struct bt_conn *conn, u8_t ident,

‎subsys/bluetooth/host/l2cap_internal.h‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,16 @@ struct net_buf *bt_l2cap_create_pdu_timeout(struct net_buf_pool *pool,
269269
/* Prepare a L2CAP Response PDU to be sent over a connection */
270270
struct net_buf *bt_l2cap_create_rsp(struct net_buf *buf, size_t reserve);
271271

272-
/* Send L2CAP PDU over a connection */
273-
void bt_l2cap_send_cb(struct bt_conn *conn, u16_t cid, struct net_buf *buf,
274-
bt_conn_tx_cb_t cb, void *user_data);
272+
/* Send L2CAP PDU over a connection
273+
*
274+
* Buffer ownership is transferred to stack so either in case of success
275+
* or error the buffer will be unref internally.
276+
*
277+
* Calling this from RX thread is assumed to never fail so the return can be
278+
* ignored.
279+
*/
280+
int bt_l2cap_send_cb(struct bt_conn *conn, u16_t cid, struct net_buf *buf,
281+
bt_conn_tx_cb_t cb, void *user_data);
275282

276283
static inline void bt_l2cap_send(struct bt_conn *conn, u16_t cid,
277284
struct net_buf *buf)

0 commit comments

Comments
 (0)