incubator-mynewt-core git commit: MYNEWT-702 BLE Host - duplicate update entries

[cc...@apache.org]

Repository: incubator-mynewt-core
Updated Branches:
  refs/heads/develop 8a6b33bb5 -> 091fbe124


MYNEWT-702 BLE Host - duplicate update entries

If the application calls ble_gap_update_params() while an update
connection procedure for that connection is already in progress, the
existing entry gets re-inserted in the ble_gap_update_entries list. This
yields a cycle in the list, causing the host task to loop endlessly
during iteration.

More details:

1. Host initiates a connection update procedure; creates an entry and
   inserts it into the list (ble_gap_update_entries).

2. Host attempts to initiate a second connection update procedure for
   the same connection. Since an existing update procedure is ongoing,
   this attempt fails with a status code of BLE_HS_EALREADY.

3. On detecting the error, the ble_gap_update_params() function tries
   to clean up (goto done). Part of this cleanup involves freeing the
   update entry that got allocated earlier in the function but never got
   inserted into the list. In this case, no entry was allocated, but it
   looks like one was, because the entry pointer was used to detect a
   duplicate entry. Consequently, the entry is freed but never removed
   from the list!

4. The host initiates a third connection update procedure for the same
   connection. This time, no duplicate is detected because the entry in
   the list got corrupted when it was freed, making its connection
   handle value indeterminate. The host allocates the same entry from
   the pool, populates it, and inserts it into the list. Now the same
   entry is in the list twice, creating a cycle. When the host iterates
   this list, it loops forever.


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

Branch: refs/heads/develop
Commit: 091fbe124365ee8b109368928526ed86301b5af8
Parents: 8a6b33b
Author: Christopher Collins <cc...@apache.org>
Authored: Fri Mar 31 15:20:06 2017 -0700
Committer: Christopher Collins <cc...@apache.org>
Committed: Fri Mar 31 15:21:42 2017 -0700

----------------------------------------------------------------------
 net/nimble/host/src/ble_gap.c           | 10 ++++++++--
 net/nimble/host/test/src/ble_gap_test.c |  9 +++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/091fbe12/net/nimble/host/src/ble_gap.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/ble_gap.c b/net/nimble/host/src/ble_gap.c
index ed71374..b494613 100644
--- a/net/nimble/host/src/ble_gap.c
+++ b/net/nimble/host/src/ble_gap.c
@@ -2869,6 +2869,7 @@ ble_gap_update_params(uint16_t conn_handle,
 
     struct ble_l2cap_sig_update_params l2cap_params;
     struct ble_gap_update_entry *entry;
+    struct ble_gap_update_entry *dup;
     struct ble_hs_conn *conn;
     int rc;
 
@@ -2889,8 +2890,9 @@ ble_gap_update_params(uint16_t conn_handle,
         goto done;
     }
 
-    entry = ble_gap_update_entry_find(conn_handle, NULL);
-    if (entry != NULL) {
+    /* Don't allow two concurrent updates to the same connection. */
+    dup = ble_gap_update_entry_find(conn_handle, NULL);
+    if (dup != NULL) {
         rc = BLE_HS_EALREADY;
         goto done;
     }
@@ -2928,6 +2930,10 @@ done:
     } else {
         ble_gap_update_entry_free(entry);
 
+        /* If the l2cap_params struct is populated, the only error is that the
+         * controller doesn't support the connection parameters request
+         * procedure.  In this case, fallback to the L2CAP update procedure.
+         */
         if (l2cap_params.itvl_min != 0) {
             rc = ble_l2cap_sig_update(conn_handle,
                                       &l2cap_params,

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/091fbe12/net/nimble/host/test/src/ble_gap_test.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/test/src/ble_gap_test.c b/net/nimble/host/test/src/ble_gap_test.c
index ac02343..8471d5b 100644
--- a/net/nimble/host/test/src/ble_gap_test.c
+++ b/net/nimble/host/test/src/ble_gap_test.c
@@ -1771,6 +1771,15 @@ ble_gap_test_util_update_no_l2cap(struct ble_gap_upd_params *params,
     if (rc == 0) {
         TEST_ASSERT(ble_gap_dbg_update_active(2));
 
+        /* Attempt two duplicate updates; ensure BLE_HS_EALREADY gets returned
+         * both times.  Make sure initial update still completes successfully
+         * (MYNEWT-702).
+         */
+        rc = ble_hs_test_util_conn_update(2, params, 0);
+        TEST_ASSERT(rc == BLE_HS_EALREADY);
+        rc = ble_hs_test_util_conn_update(2, params, 0);
+        TEST_ASSERT(rc == BLE_HS_EALREADY);
+
         /* Receive connection update complete event. */
         ble_gap_test_util_rx_update_complete(event_status, params);