You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by cc...@apache.org on 2016/05/04 01:45:07 UTC

[2/2] incubator-mynewt-core git commit: Fix l2cap-sm thread-safety.

Fix l2cap-sm thread-safety.

Prior to this change, thread-saftey was achieved as follows:

The only resource protected by the mutex is the list of active
procedures (ble_l2cap_sm_procs).  Thread-safety is achieved by locking
the mutex during removal and insertion operations.  Procedure objects
are only modified while they are not in the list.

The problem with this approach is that it is not always possible to
determine if an SM operation is currently in progress for a particular
connection.  If the relevant proc entry is is currently in use (and
therefore not in the list), a conflicting entry could be inserted into
the list.

The new behavior is:
    * Don't remove proc entries from the list until they are no longer
      needed.
    * Keep the host mutex locked whenever:
        o An entry is read from or written to.
        o The list is read or modified.


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/7bd699bd
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/7bd699bd
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/7bd699bd

Branch: refs/heads/develop
Commit: 7bd699bd1109e6204fecd63147bb3e8f22a0ecec
Parents: bfb7f44
Author: Christopher Collins <cc...@apache.org>
Authored: Wed Apr 27 17:10:21 2016 -0700
Committer: Christopher Collins <cc...@apache.org>
Committed: Tue May 3 16:44:12 2016 -0700

----------------------------------------------------------------------
 net/nimble/host/include/host/ble_hs.h        |   2 +-
 net/nimble/host/src/ble_l2cap_sm.c           | 257 +++++++++++++---------
 net/nimble/host/src/ble_l2cap_sm_cmd.c       |   6 +-
 net/nimble/host/src/test/ble_l2cap_sm_test.c |  12 +-
 4 files changed, 167 insertions(+), 110 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/7bd699bd/net/nimble/host/include/host/ble_hs.h
----------------------------------------------------------------------
diff --git a/net/nimble/host/include/host/ble_hs.h b/net/nimble/host/include/host/ble_hs.h
index 72cde41..5ae2d0f 100644
--- a/net/nimble/host/include/host/ble_hs.h
+++ b/net/nimble/host/include/host/ble_hs.h
@@ -65,7 +65,7 @@ struct os_event;
 #define BLE_HS_ERR_SM_US_BASE       0x400   /* 1024 */
 #define BLE_HS_SM_US_ERR(x)         ((x) ? BLE_HS_ERR_SM_US_BASE + (x) : 0)
 
-#define BLE_HS_ERR_SM_THEM_BASE     0x400   /* 1024 */
+#define BLE_HS_ERR_SM_THEM_BASE     0x500   /* 1280 */
 #define BLE_HS_SM_THEM_ERR(x)       ((x) ? BLE_HS_ERR_SM_THEM_BASE + (x) : 0)
 
 struct ble_hs_cfg {

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/7bd699bd/net/nimble/host/src/ble_l2cap_sm.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/ble_l2cap_sm.c b/net/nimble/host/src/ble_l2cap_sm.c
index 2460048..484deb8 100644
--- a/net/nimble/host/src/ble_l2cap_sm.c
+++ b/net/nimble/host/src/ble_l2cap_sm.c
@@ -36,10 +36,9 @@
  * Notes on thread-safety:
  * 1. The ble_hs mutex must never be locked when an application callback is
  *    executed.  A callback is free to initiate additional host procedures.
- * 2. The only resource protected by the mutex is the list of active procedures
- *    (ble_l2cap_sm_procs).  Thread-safety is achieved by locking the mutex
- *    during removal and insertion operations.  Procedure objects are only
- *    modified while they are not in the list.
+ * 2. Keep the host mutex locked whenever:
+ *      o A proc entry is read from or written to.
+ *      o The proc list is read or modified.
  */
 
 #include <string.h>
@@ -313,11 +312,16 @@ ble_l2cap_sm_proc_free(struct ble_l2cap_sm_proc *proc)
 }
 
 static void
-ble_l2cap_sm_proc_insert(struct ble_l2cap_sm_proc *proc)
+ble_l2cap_sm_proc_remove(struct ble_l2cap_sm_proc *proc,
+                         struct ble_l2cap_sm_proc *prev)
 {
-    ble_hs_lock();
-    STAILQ_INSERT_HEAD(&ble_l2cap_sm_procs, proc, next);
-    ble_hs_unlock();
+    if (prev == NULL) {
+        BLE_HS_DBG_ASSERT(STAILQ_FIRST(&ble_l2cap_sm_procs) == proc);
+        STAILQ_REMOVE_HEAD(&ble_l2cap_sm_procs, next);
+    } else {
+        BLE_HS_DBG_ASSERT(STAILQ_NEXT(prev, next) == proc);
+        STAILQ_REMOVE_AFTER(&ble_l2cap_sm_procs, prev, next);
+    }
 }
 
 static void
@@ -340,13 +344,13 @@ ble_l2cap_sm_gap_event(struct ble_l2cap_sm_proc *proc, int status,
     ble_gap_security_event(proc->conn_handle, status, &sec_params);
 }
 
-static void
+static int
 ble_l2cap_sm_process_status(struct ble_l2cap_sm_proc *proc, int status,
                             uint8_t sm_status, int call_cb, int tx_fail)
 {
-    if (status == 0) {
-        ble_l2cap_sm_proc_insert(proc);
-    } else {
+    if (proc == NULL) {
+        status = BLE_HS_ENOENT;
+    } else if (status != 0) {
         if (tx_fail) {
             ble_l2cap_sm_pair_fail_tx(proc->conn_handle, sm_status);
         }
@@ -355,6 +359,8 @@ ble_l2cap_sm_process_status(struct ble_l2cap_sm_proc *proc, int status,
         }
         ble_l2cap_sm_proc_free(proc);
     }
+
+    return status;
 }
 
 static int
@@ -380,33 +386,36 @@ ble_l2cap_sm_proc_matches(struct ble_l2cap_sm_proc *proc, uint16_t conn_handle,
 }
 
 /**
- * Searches the main proc list for an entry whose connection handle and state code
- * match those specified.  If a matching entry is found, it is removed from the
- * list and returned.
+ * Searches the main proc list for an entry whose connection handle and state
+ * code match those specified.
  *
  * @param conn_handle           The connection handle to match against.
- * @param state                    The state code to match against.
- * @param is_initiator
+ * @param state                 The state code to match against.
+ * @param is_initiator          Matches on the proc's initiator flag:
+ *                                   0=non-initiator only
+ *                                   1=initiator only
+ *                                  -1=don't care
+ * @param out_prev              On success, the entry previous to the result is
+ *                                  written here.
  *
  * @return                      The matching proc entry on success;
  *                                  null on failure.
  */
 static struct ble_l2cap_sm_proc *
-ble_l2cap_sm_proc_extract(uint16_t conn_handle, uint8_t state,
-                          int is_initiator)
+ble_l2cap_sm_proc_find(uint16_t conn_handle, uint8_t state, int is_initiator,
+                       struct ble_l2cap_sm_proc **out_prev)
 {
     struct ble_l2cap_sm_proc *proc;
     struct ble_l2cap_sm_proc *prev;
 
-    ble_hs_lock();
+    BLE_HS_DBG_ASSERT(ble_hs_thread_safe());
 
     prev = NULL;
     STAILQ_FOREACH(proc, &ble_l2cap_sm_procs, next) {
-        if (ble_l2cap_sm_proc_matches(proc, conn_handle, state, is_initiator)) {
-            if (prev == NULL) {
-                STAILQ_REMOVE_HEAD(&ble_l2cap_sm_procs, next);
-            } else {
-                STAILQ_REMOVE_AFTER(&ble_l2cap_sm_procs, prev, next);
+        if (ble_l2cap_sm_proc_matches(proc, conn_handle, state,
+                                      is_initiator)) {
+            if (out_prev != NULL) {
+                *out_prev = prev;
             }
             break;
         }
@@ -414,8 +423,6 @@ ble_l2cap_sm_proc_extract(uint16_t conn_handle, uint8_t state,
         prev = proc;
     }
 
-    ble_hs_unlock();
-
     return proc;
 }
 
@@ -662,7 +669,7 @@ ble_l2cap_sm_confirm_prepare_args(struct ble_l2cap_sm_proc *proc,
 {
     struct ble_hs_conn *conn;
 
-    ble_hs_lock();
+    BLE_HS_DBG_ASSERT(ble_hs_thread_safe());
 
     conn = ble_hs_conn_find(proc->conn_handle);
     if (conn != NULL) {
@@ -681,8 +688,6 @@ ble_l2cap_sm_confirm_prepare_args(struct ble_l2cap_sm_proc *proc,
         }
     }
 
-    ble_hs_unlock();
-
     if (conn == NULL) {
         return BLE_HS_ENOTCONN;
     }
@@ -870,6 +875,7 @@ ble_l2cap_sm_rx_pair_req(uint16_t conn_handle, uint8_t state,
 {
     struct ble_l2cap_sm_pair_cmd req;
     struct ble_l2cap_sm_proc *proc;
+    struct ble_l2cap_sm_proc *prev;
     uint8_t sm_status;
     int rc;
 
@@ -886,25 +892,31 @@ ble_l2cap_sm_rx_pair_req(uint16_t conn_handle, uint8_t state,
                req.io_cap, req.oob_data_flag, req.authreq,
                req.max_enc_key_size, req.init_key_dist, req.resp_key_dist);
 
+    ble_hs_lock();
     /* XXX: Check connection state; reject if not appropriate. */
-
-    proc = ble_l2cap_sm_proc_extract(conn_handle, BLE_L2CAP_SM_PROC_STATE_NONE,
-                                     -1);
+    proc = ble_l2cap_sm_proc_find(conn_handle, BLE_L2CAP_SM_PROC_STATE_NONE,
+                                  -1, &prev);
     if (proc != NULL) {
         /* Pairing already in progress; abort old procedure and start new. */
         /* XXX: Check the spec on this. */
+        ble_l2cap_sm_proc_remove(proc, prev);
         ble_l2cap_sm_proc_free(proc);
     }
 
     proc = ble_l2cap_sm_proc_alloc();
     if (proc == NULL) {
-        return BLE_HS_ENOMEM;
+        rc = BLE_HS_ENOMEM;
+    } else {
+        proc->conn_handle = conn_handle;
+        proc->state = BLE_L2CAP_SM_PROC_STATE_PAIR;
+        rc = ble_l2cap_sm_pair_req_handle(proc, &req, &sm_status);
+        rc = ble_l2cap_sm_process_status(proc, rc, sm_status, 0, 1);
+        if (rc == 0) {
+            STAILQ_INSERT_HEAD(&ble_l2cap_sm_procs, proc, next);
+        }
     }
-    proc->conn_handle = conn_handle;
-    proc->state = BLE_L2CAP_SM_PROC_STATE_PAIR;
 
-    rc = ble_l2cap_sm_pair_req_handle(proc, &req, &sm_status);
-    ble_l2cap_sm_process_status(proc, rc, sm_status, 1, 1);
+    ble_hs_unlock();
 
     return rc;
 }
@@ -915,6 +927,7 @@ ble_l2cap_sm_rx_pair_rsp(uint16_t conn_handle, uint8_t state,
 {
     struct ble_l2cap_sm_pair_cmd rsp;
     struct ble_l2cap_sm_proc *proc;
+    struct ble_l2cap_sm_proc *prev;
     uint8_t sm_status;
     int rc;
 
@@ -931,16 +944,19 @@ ble_l2cap_sm_rx_pair_rsp(uint16_t conn_handle, uint8_t state,
                rsp.io_cap, rsp.oob_data_flag, rsp.authreq,
                rsp.max_enc_key_size, rsp.init_key_dist, rsp.resp_key_dist);
 
-    proc = ble_l2cap_sm_proc_extract(conn_handle, BLE_L2CAP_SM_PROC_STATE_PAIR,
-                                     1);
-    if (proc == NULL) {
-        return BLE_HS_ENOTCONN;
+    ble_hs_lock();
+    proc = ble_l2cap_sm_proc_find(conn_handle, BLE_L2CAP_SM_PROC_STATE_PAIR, 1,
+                                  &prev);
+    if (proc != NULL) {
+        rc = ble_l2cap_sm_pair_rsp_handle(proc, &rsp, &sm_status);
+        if (rc != 0) {
+            ble_l2cap_sm_proc_remove(proc, prev);
+        }
     }
+    ble_hs_unlock();
 
-    rc = ble_l2cap_sm_pair_rsp_handle(proc, &rsp, &sm_status);
-    ble_l2cap_sm_process_status(proc, rc, sm_status, 1, 1);
-
-    return 0;
+    rc = ble_l2cap_sm_process_status(proc, rc, sm_status, 1, 1);
+    return rc;
 }
 
 static int
@@ -949,6 +965,7 @@ ble_l2cap_sm_rx_pair_confirm(uint16_t conn_handle, uint8_t state,
 {
     struct ble_l2cap_sm_pair_confirm cmd;
     struct ble_l2cap_sm_proc *proc;
+    struct ble_l2cap_sm_proc *prev;
     uint8_t sm_status;
     int rc;
 
@@ -961,16 +978,19 @@ ble_l2cap_sm_rx_pair_confirm(uint16_t conn_handle, uint8_t state,
 
     BLE_HS_LOG(DEBUG, "rxed sm confirm cmd\n");
 
-    proc = ble_l2cap_sm_proc_extract(conn_handle,
-                                     BLE_L2CAP_SM_PROC_STATE_CONFIRM, -1);
-    if (proc == NULL) {
-        return BLE_HS_ENOTCONN;
+    ble_hs_lock();
+    proc = ble_l2cap_sm_proc_find(conn_handle, BLE_L2CAP_SM_PROC_STATE_CONFIRM,
+                                  -1, &prev);
+    if (proc != NULL) {
+        rc = ble_l2cap_sm_confirm_handle(proc, &cmd, &sm_status);
+        if (rc != 0) {
+            ble_l2cap_sm_proc_remove(proc, prev);
+        }
     }
+    ble_hs_unlock();
 
-    rc = ble_l2cap_sm_confirm_handle(proc, &cmd, &sm_status);
-    ble_l2cap_sm_process_status(proc, rc, sm_status, 1, 1);
-
-    return 0;
+    rc = ble_l2cap_sm_process_status(proc, rc, sm_status, 1, 1);
+    return rc;
 }
 
 static int
@@ -979,9 +999,12 @@ ble_l2cap_sm_rx_pair_random(uint16_t conn_handle, uint8_t state,
 {
     struct ble_l2cap_sm_pair_random cmd;
     struct ble_l2cap_sm_proc *proc;
+    struct ble_l2cap_sm_proc *prev;
     uint8_t sm_status;
     int rc;
 
+    sm_status = 0;  /* Silence gcc warning. */
+
     rc = ble_hs_misc_pullup_base(om, BLE_L2CAP_SM_PAIR_RANDOM_SZ);
     if (rc != 0) {
         return rc;
@@ -991,16 +1014,19 @@ ble_l2cap_sm_rx_pair_random(uint16_t conn_handle, uint8_t state,
 
     BLE_HS_LOG(DEBUG, "rxed sm random cmd\n");
 
-    proc = ble_l2cap_sm_proc_extract(conn_handle,
-                                     BLE_L2CAP_SM_PROC_STATE_RANDOM, -1);
-    if (proc == NULL) {
-        return BLE_HS_ENOTCONN;
+    ble_hs_lock();
+    proc = ble_l2cap_sm_proc_find(conn_handle, BLE_L2CAP_SM_PROC_STATE_RANDOM,
+                                  -1, &prev);
+    if (proc != NULL) {
+        rc = ble_l2cap_sm_random_handle(proc, &cmd, &sm_status);
+        if (rc != 0) {
+            ble_l2cap_sm_proc_remove(proc, prev);
+        }
     }
+    ble_hs_unlock();
 
-    rc = ble_l2cap_sm_random_handle(proc, &cmd, &sm_status);
-    ble_l2cap_sm_process_status(proc, rc, sm_status, 1, 1);
-
-    return 0;
+    rc = ble_l2cap_sm_process_status(proc, rc, sm_status, 1, 1);
+    return rc;
 }
 
 static int
@@ -1009,6 +1035,7 @@ ble_l2cap_sm_rx_pair_fail(uint16_t conn_handle, uint8_t state,
 {
     struct ble_l2cap_sm_pair_fail cmd;
     struct ble_l2cap_sm_proc *proc;
+    struct ble_l2cap_sm_proc *prev;
     int rc;
 
     rc = ble_hs_misc_pullup_base(om, BLE_L2CAP_SM_PAIR_FAIL_SZ);
@@ -1020,53 +1047,65 @@ ble_l2cap_sm_rx_pair_fail(uint16_t conn_handle, uint8_t state,
 
     BLE_HS_LOG(DEBUG, "rxed sm fail cmd; reason=%d\n", cmd.reason);
 
-    proc = ble_l2cap_sm_proc_extract(conn_handle, BLE_L2CAP_SM_PROC_STATE_NONE,
-                                     -1);
-    if (proc == NULL) {
-        return BLE_HS_ENOTCONN;
+    ble_hs_lock();
+    proc = ble_l2cap_sm_proc_find(conn_handle, BLE_L2CAP_SM_PROC_STATE_NONE,
+                                  -1, &prev);
+    if (proc != NULL) {
+        ble_l2cap_sm_proc_remove(proc, prev);
     }
+    ble_hs_unlock();
 
-    ble_l2cap_sm_process_status(proc, BLE_HS_SM_THEM_ERR(cmd.reason), 0, 1, 0);
-
-    return 0;
+    rc = ble_l2cap_sm_process_status(proc, rc, 0, 1, 0);
+    return rc;
 }
 
 int
 ble_l2cap_sm_rx_lt_key_req(struct hci_le_lt_key_req *evt)
 {
     struct ble_l2cap_sm_proc *proc;
+    struct ble_l2cap_sm_proc *prev;
     uint8_t sm_status;
     int rc;
 
-    proc = ble_l2cap_sm_proc_extract(evt->connection_handle,
-                                     BLE_L2CAP_SM_PROC_STATE_LTK, 0);
+    ble_hs_lock();
+    proc = ble_l2cap_sm_proc_find(evt->connection_handle,
+                                  BLE_L2CAP_SM_PROC_STATE_LTK, 0, &prev);
     if (proc == NULL) {
-        return BLE_HS_ENOTCONN;
+        rc = BLE_HS_ENOTCONN;
+    } else {
+        rc = ble_l2cap_sm_lt_key_req_handle(proc, evt, &sm_status);
+        if (rc != 0) {
+            ble_l2cap_sm_proc_remove(proc, prev);
+        }
     }
+    ble_hs_unlock();
 
-    rc = ble_l2cap_sm_lt_key_req_handle(proc, evt, &sm_status);
-    ble_l2cap_sm_process_status(proc, rc, sm_status, 1, 1);
-
-    return 0;
+    rc = ble_l2cap_sm_process_status(proc, rc, sm_status, 1, 1);
+    return rc;
 }
 
 void
 ble_l2cap_sm_rx_encryption_change(struct hci_encrypt_change *evt)
 {
     struct ble_l2cap_sm_proc *proc;
+    struct ble_l2cap_sm_proc *prev;
     int enc_enabled;
 
-    proc = ble_l2cap_sm_proc_extract(evt->connection_handle,
-                                     BLE_L2CAP_SM_PROC_STATE_ENC_CHANGE, -1);
-    if (proc == NULL) {
-        return;
+    ble_hs_lock();
+    proc = ble_l2cap_sm_proc_find(evt->connection_handle,
+                                  BLE_L2CAP_SM_PROC_STATE_ENC_CHANGE, -1,
+                                  &prev);
+    if (proc != NULL) {
+        ble_l2cap_sm_proc_remove(proc, prev);
     }
+    ble_hs_unlock();
 
-    enc_enabled = evt->encryption_enabled & 0x01; /* LE bit. */
-    ble_l2cap_sm_gap_event(proc, BLE_HS_HCI_ERR(evt->status), enc_enabled);
-
-    /* The pairing procedure is now complete. */
-    ble_l2cap_sm_proc_free(proc);
+    if (proc != NULL) {
+        /* The pairing procedure is now complete. */
+        enc_enabled = evt->encryption_enabled & 0x01; /* LE bit. */
+        ble_l2cap_sm_gap_event(proc, BLE_HS_HCI_ERR(evt->status), enc_enabled);
+        ble_l2cap_sm_proc_free(proc);
+    }
 }
 
 static int
@@ -1134,23 +1173,33 @@ ble_l2cap_sm_initiate(uint16_t conn_handle)
     /* Make sure a pairing operation for this connection is not already in
      * progress.
      */
-    proc = ble_l2cap_sm_proc_extract(conn_handle, BLE_L2CAP_SM_PROC_STATE_NONE,
-                                     -1);
+    ble_hs_lock();
+    proc = ble_l2cap_sm_proc_find(conn_handle, BLE_L2CAP_SM_PROC_STATE_NONE,
+                                  -1, NULL);
     if (proc != NULL) {
-        return BLE_HS_EALREADY;
+        rc = BLE_HS_EALREADY;
+        goto done;
     }
 
     proc = ble_l2cap_sm_proc_alloc();
     if (proc == NULL) {
-        return BLE_HS_ENOMEM;
+        rc = BLE_HS_ENOMEM;
+        goto done;
     }
     proc->conn_handle = conn_handle;
     proc->state = BLE_L2CAP_SM_PROC_STATE_PAIR;
     proc->flags |= BLE_L2CAP_SM_PROC_F_INITIATOR;
 
     rc = ble_l2cap_sm_pair_go(proc);
-    ble_l2cap_sm_process_status(proc, rc, 0, 0, 0);
+    if (rc != 0) {
+        ble_l2cap_sm_proc_free(proc);
+        goto done;
+    }
 
+    STAILQ_INSERT_HEAD(&ble_l2cap_sm_procs, proc, next);
+
+done:
+    ble_hs_unlock();
     return rc;
 }
 
@@ -1177,33 +1226,41 @@ ble_l2cap_sm_set_tk(uint16_t conn_handle, uint8_t *tk)
 {
     struct ble_l2cap_sm_proc *proc;
 
-    proc = ble_l2cap_sm_proc_extract(conn_handle,
-                                     BLE_L2CAP_SM_PROC_STATE_CONFIRM, -1);
-    if (proc == NULL) {
-        return BLE_HS_ENOENT;
+    ble_hs_lock();
+    proc = ble_l2cap_sm_proc_find(conn_handle, BLE_L2CAP_SM_PROC_STATE_CONFIRM,
+                                  -1, NULL);
+    if (proc != NULL) {
+        memcpy(proc->tk, tk, 16);
     }
-
-    memcpy(proc->tk, tk, 16);
+    ble_hs_unlock();
 
     /* XXX: Proceed with pairing; send confirm command. */
 
-    return 0;
+    if (proc == NULL) {
+        return BLE_HS_ENOENT;
+    } else {
+        return 0;
+    }
 }
 
 void
 ble_l2cap_sm_connection_broken(uint16_t conn_handle)
 {
     struct ble_l2cap_sm_proc *proc;
+    struct ble_l2cap_sm_proc *prev;
 
-    proc = ble_l2cap_sm_proc_extract(conn_handle, BLE_L2CAP_SM_PROC_STATE_NONE,
-                                     -1);
+    ble_hs_lock();
+    proc = ble_l2cap_sm_proc_find(conn_handle, BLE_L2CAP_SM_PROC_STATE_NONE,
+                                  -1, &prev);
     if (proc != NULL) {
-        /* Free thw affected procedure object.  There is no need to notify the
+        /* Free the affected procedure object.  There is no need to notify the
          * application, as it has already been notified of the connection
          * failure.
          */
+        ble_l2cap_sm_proc_remove(proc, prev);
         ble_l2cap_sm_proc_free(proc);
     }
+    ble_hs_unlock();
 }
 
 int

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/7bd699bd/net/nimble/host/src/ble_l2cap_sm_cmd.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/ble_l2cap_sm_cmd.c b/net/nimble/host/src/ble_l2cap_sm_cmd.c
index 843b1e5..7d81a07 100644
--- a/net/nimble/host/src/ble_l2cap_sm_cmd.c
+++ b/net/nimble/host/src/ble_l2cap_sm_cmd.c
@@ -33,9 +33,9 @@ ble_l2cap_sm_tx(uint16_t conn_handle, struct os_mbuf *txom)
     struct ble_hs_conn *conn;
     int rc;
 
-    STATS_INC(ble_l2cap_stats, sm_tx);
+    BLE_HS_DBG_ASSERT(ble_hs_thread_safe());
 
-    ble_hs_lock();
+    STATS_INC(ble_l2cap_stats, sm_tx);
 
     rc = ble_hs_misc_conn_chan_find_reqd(conn_handle, BLE_L2CAP_CID_SM,
                                          &conn, &chan);
@@ -46,8 +46,6 @@ ble_l2cap_sm_tx(uint16_t conn_handle, struct os_mbuf *txom)
         rc = ble_l2cap_tx(conn, chan, txom);
     }
 
-    ble_hs_unlock();
-
     return rc;
 }
 

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/7bd699bd/net/nimble/host/src/test/ble_l2cap_sm_test.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/test/ble_l2cap_sm_test.c b/net/nimble/host/src/test/ble_l2cap_sm_test.c
index 86b69c3..648676e 100644
--- a/net/nimble/host/src/test/ble_l2cap_sm_test.c
+++ b/net/nimble/host/src/test/ble_l2cap_sm_test.c
@@ -146,7 +146,8 @@ ble_l2cap_sm_test_util_rx_confirm(struct ble_hs_conn *conn,
 
 static void
 ble_l2cap_sm_test_util_rx_random(struct ble_hs_conn *conn,
-                                 struct ble_l2cap_sm_pair_random *cmd)
+                                 struct ble_l2cap_sm_pair_random *cmd,
+                                 int exp_status)
 {
     struct hci_data_hdr hci_hdr;
     struct os_mbuf *om;
@@ -170,7 +171,7 @@ ble_l2cap_sm_test_util_rx_random(struct ble_hs_conn *conn,
 
     rc = ble_hs_test_util_l2cap_rx_first_frag(conn, BLE_L2CAP_CID_SM, &hci_hdr,
                                               om);
-    TEST_ASSERT_FATAL(rc == 0);
+    TEST_ASSERT_FATAL(rc == exp_status);
 }
 
 static void
@@ -402,7 +403,7 @@ ble_l2cap_sm_test_util_peer_lgcy_good(
     TEST_ASSERT(ble_l2cap_sm_dbg_num_procs() == 1);
 
     /* Receive a pair random from the peer. */
-    ble_l2cap_sm_test_util_rx_random(conn, random_req);
+    ble_l2cap_sm_test_util_rx_random(conn, random_req, 0);
     TEST_ASSERT(!conn->bhc_sec_params.enc_enabled);
     TEST_ASSERT(ble_l2cap_sm_dbg_num_procs() == 1);
 
@@ -488,7 +489,8 @@ ble_l2cap_sm_test_util_peer_lgcy_fail(
     TEST_ASSERT(ble_l2cap_sm_dbg_num_procs() == 1);
 
     /* Receive a pair random from the peer. */
-    ble_l2cap_sm_test_util_rx_random(conn, random_req);
+    ble_l2cap_sm_test_util_rx_random(
+        conn, random_req, BLE_HS_SM_US_ERR(BLE_L2CAP_SM_ERR_CONFIRM_MISMATCH));
 
     /* Ensure we sent the expected pair fail. */
     ble_hs_test_util_tx_all();
@@ -691,7 +693,7 @@ ble_l2cap_sm_test_util_us_lgcy_good(
     TEST_ASSERT(ble_l2cap_sm_dbg_num_procs() == 1);
 
     /* Receive a pair random from the peer. */
-    ble_l2cap_sm_test_util_rx_random(conn, random_rsp);
+    ble_l2cap_sm_test_util_rx_random(conn, random_rsp, 0);
     TEST_ASSERT(!conn->bhc_sec_params.enc_enabled);
     TEST_ASSERT(ble_l2cap_sm_dbg_num_procs() == 1);