You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by we...@apache.org on 2017/03/30 21:12:49 UTC
[32/37] incubator-mynewt-core git commit: MYNEWT-700 BLE Host - Race
condition: discon + att
MYNEWT-700 BLE Host - Race condition: discon + att
Some parts of the ATT code assume a peer is still connected after an
initial check. This assumption leads to a race condition when a task
other than the host task is doing the transmitting (e.g., tx of
unsolicited notification). It is possible that the peer gets
disconnected after the tx function is called, but before it completes.
When this occurs, an assertion fails (ble_att_conn_chan_find()).
The fix is to remove this assumption. Always check that the connection /
channel lookup succeeds before accessing the returned pointers.
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/9b6c2847
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/9b6c2847
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/9b6c2847
Branch: refs/heads/nrf_cputime
Commit: 9b6c2847d4fef37e2cbe47b5dfc4b85555c8e3a7
Parents: 612f8ef
Author: Christopher Collins <cc...@apache.org>
Authored: Thu Mar 30 12:09:48 2017 -0700
Committer: Christopher Collins <cc...@apache.org>
Committed: Thu Mar 30 12:10:49 2017 -0700
----------------------------------------------------------------------
net/nimble/host/src/ble_att.c | 11 ++--
net/nimble/host/src/ble_att_clt.c | 26 +++++---
net/nimble/host/src/ble_att_priv.h | 4 +-
net/nimble/host/src/ble_att_svr.c | 82 ++++++++++++++----------
net/nimble/host/src/ble_gattc.c | 10 ++-
net/nimble/host/test/src/ble_att_svr_test.c | 6 +-
net/nimble/host/test/src/ble_hs_test_util.c | 6 +-
7 files changed, 91 insertions(+), 54 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/9b6c2847/net/nimble/host/src/ble_att.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/ble_att.c b/net/nimble/host/src/ble_att.c
index 0eaed34..1756fc8 100644
--- a/net/nimble/host/src/ble_att.c
+++ b/net/nimble/host/src/ble_att.c
@@ -143,12 +143,12 @@ ble_att_rx_dispatch_entry_find(uint8_t op)
return NULL;
}
-void
+int
ble_att_conn_chan_find(uint16_t conn_handle, struct ble_hs_conn **out_conn,
struct ble_l2cap_chan **out_chan)
{
- ble_hs_misc_conn_chan_find_reqd(conn_handle, BLE_L2CAP_CID_ATT,
- out_conn, out_chan);
+ return ble_hs_misc_conn_chan_find(conn_handle, BLE_L2CAP_CID_ATT,
+ out_conn, out_chan);
}
void
@@ -414,11 +414,12 @@ ble_att_mtu(uint16_t conn_handle)
struct ble_l2cap_chan *chan;
struct ble_hs_conn *conn;
uint16_t mtu;
+ int rc;
ble_hs_lock();
- ble_att_conn_chan_find(conn_handle, &conn, &chan);
- if (chan != NULL) {
+ rc = ble_att_conn_chan_find(conn_handle, &conn, &chan);
+ if (rc == 0) {
mtu = ble_att_chan_mtu(chan);
} else {
mtu = 0;
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/9b6c2847/net/nimble/host/src/ble_att_clt.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/ble_att_clt.c b/net/nimble/host/src/ble_att_clt.c
index f310a6f..8c92893 100644
--- a/net/nimble/host/src/ble_att_clt.c
+++ b/net/nimble/host/src/ble_att_clt.c
@@ -70,8 +70,8 @@ ble_att_clt_tx_req(uint16_t conn_handle, struct os_mbuf *txom)
ble_hs_lock();
- ble_att_conn_chan_find(conn_handle, &conn, &chan);
- if (chan == NULL) {
+ rc = ble_att_conn_chan_find(conn_handle, &conn, &chan);
+ if (rc != 0) {
rc = BLE_HS_ENOTCONN;
} else {
ble_att_truncate_to_mtu(chan, txom);
@@ -129,8 +129,8 @@ ble_att_clt_tx_mtu(uint16_t conn_handle, const struct ble_att_mtu_cmd *req)
ble_hs_lock();
- ble_att_conn_chan_find(conn_handle, &conn, &chan);
- if (chan == NULL) {
+ rc = ble_att_conn_chan_find(conn_handle, &conn, &chan);
+ if (rc != 0) {
rc = BLE_HS_ENOTCONN;
} else if (chan->flags & BLE_L2CAP_CHAN_F_TXED_MTU) {
rc = BLE_HS_EALREADY;
@@ -158,8 +158,10 @@ ble_att_clt_tx_mtu(uint16_t conn_handle, const struct ble_att_mtu_cmd *req)
ble_hs_lock();
- ble_att_conn_chan_find(conn_handle, &conn, &chan);
- chan->flags |= BLE_L2CAP_CHAN_F_TXED_MTU;
+ rc = ble_att_conn_chan_find(conn_handle, &conn, &chan);
+ if (rc == 0) {
+ chan->flags |= BLE_L2CAP_CHAN_F_TXED_MTU;
+ }
ble_hs_unlock();
@@ -183,13 +185,17 @@ ble_att_clt_rx_mtu(uint16_t conn_handle, struct os_mbuf **rxom)
ble_hs_lock();
- ble_att_conn_chan_find(conn_handle, NULL, &chan);
- ble_att_set_peer_mtu(chan, cmd.bamc_mtu);
- mtu = ble_att_chan_mtu(chan);
+ rc = ble_att_conn_chan_find(conn_handle, NULL, &chan);
+ if (rc == 0) {
+ ble_att_set_peer_mtu(chan, cmd.bamc_mtu);
+ mtu = ble_att_chan_mtu(chan);
+ }
ble_hs_unlock();
- ble_gap_mtu_event(conn_handle, BLE_L2CAP_CID_ATT, mtu);
+ if (rc == 0) {
+ ble_gap_mtu_event(conn_handle, BLE_L2CAP_CID_ATT, mtu);
+ }
}
ble_gattc_rx_mtu(conn_handle, rc, mtu);
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/9b6c2847/net/nimble/host/src/ble_att_priv.h
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/ble_att_priv.h b/net/nimble/host/src/ble_att_priv.h
index af05684..918ca9b 100644
--- a/net/nimble/host/src/ble_att_priv.h
+++ b/net/nimble/host/src/ble_att_priv.h
@@ -160,8 +160,8 @@ SLIST_HEAD(ble_att_clt_entry_list, ble_att_clt_entry);
/*** @gen */
struct ble_l2cap_chan *ble_att_create_chan(uint16_t conn_handle);
-void ble_att_conn_chan_find(uint16_t conn_handle, struct ble_hs_conn **out_conn,
- struct ble_l2cap_chan **out_chan);
+int ble_att_conn_chan_find(uint16_t conn_handle, struct ble_hs_conn **out_conn,
+ struct ble_l2cap_chan **out_chan);
void ble_att_inc_tx_stat(uint8_t att_op);
void ble_att_truncate_to_mtu(const struct ble_l2cap_chan *att_chan,
struct os_mbuf *txom);
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/9b6c2847/net/nimble/host/src/ble_att_svr.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/ble_att_svr.c b/net/nimble/host/src/ble_att_svr.c
index 1c2d4a2..1775292 100644
--- a/net/nimble/host/src/ble_att_svr.c
+++ b/net/nimble/host/src/ble_att_svr.c
@@ -619,7 +619,7 @@ err:
* sent instead.
*
* @param conn_handle The handle of the connection to send over.
- * @param rc The status indicating whether to transmit an
+ * @param hs_status The status indicating whether to transmit an
* affirmative response or an error.
* @param txom Contains the affirmative response payload.
* @param att_op If an error is transmitted, this is the value
@@ -631,14 +631,15 @@ err:
* field.
*/
static int
-ble_att_svr_tx_rsp(uint16_t conn_handle, int rc, struct os_mbuf *om,
+ble_att_svr_tx_rsp(uint16_t conn_handle, int hs_status, struct os_mbuf *om,
uint8_t att_op, uint8_t err_status, uint16_t err_handle)
{
struct ble_l2cap_chan *chan;
struct ble_hs_conn *conn;
int do_tx;
+ int rc;
- if (rc != 0 && err_status == 0) {
+ if (hs_status != 0 && err_status == 0) {
/* Processing failed, but err_status of 0 means don't send error. */
do_tx = 0;
} else {
@@ -648,32 +649,37 @@ ble_att_svr_tx_rsp(uint16_t conn_handle, int rc, struct os_mbuf *om,
if (do_tx) {
ble_hs_lock();
- ble_att_conn_chan_find(conn_handle, &conn, &chan);
- if (rc == 0) {
- BLE_HS_DBG_ASSERT(om != NULL);
+ rc = ble_att_conn_chan_find(conn_handle, &conn, &chan);
+ if (rc != 0) {
+ /* No longer connected. */
+ hs_status = rc;
+ } else {
+ if (hs_status == 0) {
+ BLE_HS_DBG_ASSERT(om != NULL);
- ble_att_inc_tx_stat(om->om_data[0]);
- ble_att_truncate_to_mtu(chan, om);
- rc = ble_l2cap_tx(conn, chan, om);
- om = NULL;
- if (rc != 0) {
- err_status = BLE_ATT_ERR_UNLIKELY;
+ ble_att_inc_tx_stat(om->om_data[0]);
+ ble_att_truncate_to_mtu(chan, om);
+ hs_status = ble_l2cap_tx(conn, chan, om);
+ om = NULL;
+ if (hs_status != 0) {
+ err_status = BLE_ATT_ERR_UNLIKELY;
+ }
}
- }
- if (rc != 0) {
- STATS_INC(ble_att_stats, error_rsp_tx);
+ if (hs_status != 0) {
+ STATS_INC(ble_att_stats, error_rsp_tx);
- /* Reuse om for error response. */
- if (om == NULL) {
- om = ble_hs_mbuf_l2cap_pkt();
- } else {
- os_mbuf_adj(om, OS_MBUF_PKTLEN(om));
- }
- if (om != NULL) {
- ble_att_svr_tx_error_rsp(conn, chan, om, att_op,
- err_handle, err_status);
- om = NULL;
+ /* Reuse om for error response. */
+ if (om == NULL) {
+ om = ble_hs_mbuf_l2cap_pkt();
+ } else {
+ os_mbuf_adj(om, OS_MBUF_PKTLEN(om));
+ }
+ if (om != NULL) {
+ ble_att_svr_tx_error_rsp(conn, chan, om, att_op,
+ err_handle, err_status);
+ om = NULL;
+ }
}
}
@@ -683,7 +689,7 @@ ble_att_svr_tx_rsp(uint16_t conn_handle, int rc, struct os_mbuf *om,
/* Free mbuf if it was not consumed (i.e., if the send failed). */
os_mbuf_free_chain(om);
- return rc;
+ return hs_status;
}
static int
@@ -701,10 +707,16 @@ ble_att_svr_build_mtu_rsp(uint16_t conn_handle, struct os_mbuf **rxom,
txom = NULL;
ble_hs_lock();
- ble_att_conn_chan_find(conn_handle, NULL, &chan);
- mtu = chan->my_mtu;
+ rc = ble_att_conn_chan_find(conn_handle, NULL, &chan);
+ if (rc == 0) {
+ mtu = chan->my_mtu;
+ }
ble_hs_unlock();
+ if (rc != 0) {
+ goto done;
+ }
+
/* Just reuse the request buffer for the response. */
txom = *rxom;
*rxom = NULL;
@@ -763,14 +775,18 @@ done:
if (rc == 0) {
ble_hs_lock();
- ble_att_conn_chan_find(conn_handle, &conn, &chan);
- ble_att_set_peer_mtu(chan, cmd.bamc_mtu);
- chan->flags |= BLE_L2CAP_CHAN_F_TXED_MTU;
- mtu = ble_att_chan_mtu(chan);
+ rc = ble_att_conn_chan_find(conn_handle, &conn, &chan);
+ if (rc == 0) {
+ ble_att_set_peer_mtu(chan, cmd.bamc_mtu);
+ chan->flags |= BLE_L2CAP_CHAN_F_TXED_MTU;
+ mtu = ble_att_chan_mtu(chan);
+ }
ble_hs_unlock();
- ble_gap_mtu_event(conn_handle, BLE_L2CAP_CID_ATT, mtu);
+ if (rc == 0) {
+ ble_gap_mtu_event(conn_handle, BLE_L2CAP_CID_ATT, mtu);
+ }
}
return rc;
}
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/9b6c2847/net/nimble/host/src/ble_gattc.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/ble_gattc.c b/net/nimble/host/src/ble_gattc.c
index fabb52d..40f4c5c 100644
--- a/net/nimble/host/src/ble_gattc.c
+++ b/net/nimble/host/src/ble_gattc.c
@@ -1270,10 +1270,16 @@ ble_gattc_mtu_tx(struct ble_gattc_proc *proc)
int rc;
ble_hs_lock();
- ble_att_conn_chan_find(proc->conn_handle, &conn, &chan);
- req.bamc_mtu = chan->my_mtu;
+ rc = ble_att_conn_chan_find(proc->conn_handle, &conn, &chan);
+ if (rc == 0) {
+ req.bamc_mtu = chan->my_mtu;
+ }
ble_hs_unlock();
+ if (rc != 0) {
+ return rc;
+ }
+
rc = ble_att_clt_tx_mtu(proc->conn_handle, &req);
if (rc != 0) {
return rc;
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/9b6c2847/net/nimble/host/test/src/ble_att_svr_test.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/test/src/ble_att_svr_test.c b/net/nimble/host/test/src/ble_att_svr_test.c
index b22e754..d25118e 100644
--- a/net/nimble/host/test/src/ble_att_svr_test.c
+++ b/net/nimble/host/test/src/ble_att_svr_test.c
@@ -449,10 +449,14 @@ ble_att_svr_test_misc_verify_tx_mtu_rsp(uint16_t conn_handle)
struct ble_l2cap_chan *chan;
struct ble_hs_conn *conn;
uint16_t my_mtu;
+ int rc;
ble_hs_lock();
- ble_att_conn_chan_find(conn_handle, &conn, &chan);
+
+ rc = ble_att_conn_chan_find(conn_handle, &conn, &chan);
+ assert(rc == 0);
my_mtu = chan->my_mtu;
+
ble_hs_unlock();
ble_hs_test_util_verify_tx_mtu_cmd(0, my_mtu);
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/9b6c2847/net/nimble/host/test/src/ble_hs_test_util.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/test/src/ble_hs_test_util.c b/net/nimble/host/test/src/ble_hs_test_util.c
index d084c39..26783ef 100644
--- a/net/nimble/host/test/src/ble_hs_test_util.c
+++ b/net/nimble/host/test/src/ble_hs_test_util.c
@@ -994,16 +994,20 @@ ble_hs_test_util_set_att_mtu(uint16_t conn_handle, uint16_t mtu)
{
struct ble_l2cap_chan *chan;
struct ble_hs_conn *conn;
+ int rc;
if (mtu <= BLE_ATT_MTU_DFLT) {
return;
}
ble_hs_lock();
- ble_att_conn_chan_find(conn_handle, &conn, &chan);
+
+ rc = ble_att_conn_chan_find(conn_handle, &conn, &chan);
+ assert(rc == 0);
chan->my_mtu = mtu;
chan->peer_mtu = mtu;
chan->flags |= BLE_L2CAP_CHAN_F_TXED_MTU;
+
ble_hs_unlock();
}