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/08/09 21:49:32 UTC

[1/3] incubator-mynewt-core git commit: os_mbuf_appendfrom - Detect source mbuf overrun.

Repository: incubator-mynewt-core
Updated Branches:
  refs/heads/develop 48ef7aea7 -> 96b7f79f2


os_mbuf_appendfrom - Detect source mbuf overrun.


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

Branch: refs/heads/develop
Commit: f1e3467586b60f23219baef5aeaecf2f9cf9ee50
Parents: 48ef7ae
Author: Christopher Collins <cc...@apache.org>
Authored: Tue Aug 9 12:29:28 2016 -0700
Committer: Christopher Collins <cc...@apache.org>
Committed: Tue Aug 9 12:29:28 2016 -0700

----------------------------------------------------------------------
 libs/os/src/os_mbuf.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/f1e34675/libs/os/src/os_mbuf.c
----------------------------------------------------------------------
diff --git a/libs/os/src/os_mbuf.c b/libs/os/src/os_mbuf.c
index eda58ef..a2302df 100644
--- a/libs/os/src/os_mbuf.c
+++ b/libs/os/src/os_mbuf.c
@@ -520,6 +520,10 @@ err:
  * @param src_off               The absolute offset within the source mbuf
  *                                  chain to read from.
  * @param len                   The number of bytes to append.
+ *
+ * @return                      0 on success;
+ *                              OS_EINVAL if the specified range extends beyond
+ *                                  the end of the source mbuf chain.
  */
 int
 os_mbuf_appendfrom(struct os_mbuf *dst, const struct os_mbuf *src,
@@ -531,11 +535,11 @@ os_mbuf_appendfrom(struct os_mbuf *dst, const struct os_mbuf *src,
     int rc;
 
     src_cur_om = os_mbuf_off(src, src_off, &src_cur_off);
-    if (src_cur_om == NULL) {
-        return OS_EINVAL;
-    }
-
     while (len > 0) {
+        if (src_cur_om == NULL) {
+            return OS_EINVAL;
+        }
+
         chunk_sz = min(len, src_cur_om->om_len - src_cur_off);
         rc = os_mbuf_append(dst, src_cur_om->om_data + src_cur_off, chunk_sz);
         if (rc != 0) {


[2/3] incubator-mynewt-core git commit: BLE Host - reuse exec-write-req mbuf for rsp.

Posted by cc...@apache.org.
BLE Host - reuse exec-write-req mbuf for rsp.

The response has the same contents as the request; only the op code is
different.  By reusing the mbuf, it makes failure due to mbuf exhaustion
less likely.


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

Branch: refs/heads/develop
Commit: 5877bada3719bf1d31cad810a9c58f27c3c25e3f
Parents: f1e3467
Author: Christopher Collins <cc...@apache.org>
Authored: Tue Aug 9 12:41:53 2016 -0700
Committer: Christopher Collins <cc...@apache.org>
Committed: Tue Aug 9 12:41:53 2016 -0700

----------------------------------------------------------------------
 net/nimble/host/src/ble_att_svr.c | 41 +++++++++++-----------------------
 1 file changed, 13 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/5877bada/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 f282093..860794d 100644
--- a/net/nimble/host/src/ble_att_svr.c
+++ b/net/nimble/host/src/ble_att_svr.c
@@ -588,6 +588,8 @@ ble_att_svr_tx_rsp(uint16_t conn_handle, int rc, struct os_mbuf *om,
             rc = BLE_HS_ENOTCONN;
         } else {
             if (rc == 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);
@@ -2372,7 +2374,6 @@ ble_att_svr_rx_prep_write(uint16_t conn_handle, struct os_mbuf **rxom)
     struct ble_hs_conn *conn;
     struct os_mbuf *txom;
     uint16_t err_handle;
-    uint8_t *buf;
     uint8_t att_err;
     int rc;
 
@@ -2394,16 +2395,13 @@ ble_att_svr_rx_prep_write(uint16_t conn_handle, struct os_mbuf **rxom)
                     ble_att_prep_write_cmd_log, &req);
     err_handle = req.bapc_handle;
 
-    /* Strip the request base from the front of the mbuf. */
-    os_mbuf_adj(*rxom, BLE_ATT_PREP_WRITE_CMD_BASE_SZ);
-
     attr_entry = ble_att_svr_find_by_handle(req.bapc_handle);
 
     /* A prepare write request gets rejected for the following reasons:
      * 1. Insufficient authorization.
      * 2. Insufficient authentication.
      * 3. Insufficient encryption key size (XXX: Not checked).
-     * 4. Insufficient encryption.
+     * 4. Insufficient encryption (XXX: Not checked).
      * 5. Invalid handle.
      * 6. Write not permitted.
      */
@@ -2447,8 +2445,10 @@ ble_att_svr_rx_prep_write(uint16_t conn_handle, struct os_mbuf **rxom)
         }
 
         /* Append attribute value from request onto prep mbuf. */
-        rc = os_mbuf_appendfrom(prep_entry->bape_value, *rxom, 0,
-                                OS_MBUF_PKTLEN(*rxom));
+        rc = os_mbuf_appendfrom(prep_entry->bape_value, *rxom,
+                                BLE_ATT_PREP_WRITE_CMD_BASE_SZ,
+                                OS_MBUF_PKTLEN(*rxom) -
+                                    BLE_ATT_PREP_WRITE_CMD_BASE_SZ);
         if (rc != 0) {
             att_err = BLE_ATT_ERR_PREPARE_QUEUE_FULL;
         }
@@ -2460,27 +2460,12 @@ ble_att_svr_rx_prep_write(uint16_t conn_handle, struct os_mbuf **rxom)
         goto done;
     }
 
-    txom = ble_hs_mbuf_l2cap_pkt();
-    if (txom == NULL) {
-        att_err = BLE_ATT_ERR_INSUFFICIENT_RES;
-        rc = BLE_HS_ENOMEM;
-        goto done;
-    }
-
-    buf = os_mbuf_extend(txom, BLE_ATT_PREP_WRITE_CMD_BASE_SZ);
-    if (buf == NULL) {
-        att_err = BLE_ATT_ERR_INSUFFICIENT_RES;
-        rc = BLE_HS_ENOMEM;
-        goto done;
-    }
-    ble_att_prep_write_rsp_write(buf, BLE_ATT_PREP_WRITE_CMD_BASE_SZ, &req);
-
-    rc = os_mbuf_appendfrom(txom, *rxom, 0, OS_MBUF_PKTLEN(*rxom));
-    if (rc != 0) {
-        att_err = BLE_ATT_ERR_INSUFFICIENT_RES;
-        rc = BLE_HS_ENOMEM;
-        goto done;
-    }
+    /* Reuse rxom for response.  Response is identical to request except for op
+     * code.
+     */
+    txom = *rxom;
+    *rxom = NULL;
+    txom->om_data[0] = BLE_ATT_OP_PREP_WRITE_RSP;
 
     BLE_ATT_LOG_CMD(1, "prep write rsp", conn_handle,
                     ble_att_prep_write_cmd_log, &req);


[3/3] incubator-mynewt-core git commit: BLE Host - Cleanup att_svr rx prep / exec write.

Posted by cc...@apache.org.
BLE Host - Cleanup att_svr rx prep / exec write.

While debugging an mbuf leak, I noticed this code was much more
complicated than it needs to be.


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

Branch: refs/heads/develop
Commit: 96b7f79f2c6348b257f4b43cc153b0c5604a5434
Parents: 5877bad
Author: Christopher Collins <cc...@apache.org>
Authored: Tue Aug 9 12:46:50 2016 -0700
Committer: Christopher Collins <cc...@apache.org>
Committed: Tue Aug 9 14:48:18 2016 -0700

----------------------------------------------------------------------
 net/nimble/host/src/ble_att_svr.c | 123 +++++++++++++++------------------
 1 file changed, 57 insertions(+), 66 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/96b7f79f/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 860794d..93c57ff 100644
--- a/net/nimble/host/src/ble_att_svr.c
+++ b/net/nimble/host/src/ble_att_svr.c
@@ -2182,8 +2182,10 @@ ble_att_svr_write_local(uint16_t attr_handle, struct os_mbuf *om)
 static void
 ble_att_svr_prep_free(struct ble_att_prep_entry *entry)
 {
-    os_mbuf_free_chain(entry->bape_value);
-    os_memblock_put(&ble_att_svr_prep_entry_pool, entry);
+    if (entry != NULL) {
+        os_mbuf_free_chain(entry->bape_value);
+        os_memblock_put(&ble_att_svr_prep_entry_pool, entry);
+    }
 }
 
 static struct ble_att_prep_entry *
@@ -2360,6 +2362,56 @@ ble_att_svr_prep_write(uint16_t conn_handle,
     return 0;
 }
 
+static int
+ble_att_svr_insert_prep_entry(uint16_t conn_handle,
+                              const struct ble_att_prep_write_cmd *req,
+                              const struct os_mbuf *rxom,
+                              uint8_t *out_att_err)
+{
+    struct ble_att_prep_entry *prep_entry;
+    struct ble_att_prep_entry *prep_prev;
+    struct ble_hs_conn *conn;
+    int rc;
+
+    conn = ble_hs_conn_find(conn_handle);
+    if (conn == NULL) {
+        *out_att_err = 0;
+        return BLE_HS_ENOTCONN;
+    }
+
+    prep_entry = ble_att_svr_prep_alloc();
+    if (prep_entry == NULL) {
+        *out_att_err = BLE_ATT_ERR_PREPARE_QUEUE_FULL;
+        return BLE_HS_ENOMEM;
+    }
+    prep_entry->bape_handle = req->bapc_handle;
+    prep_entry->bape_offset = req->bapc_offset;
+
+    /* Append attribute value from request onto prep mbuf. */
+    rc = os_mbuf_appendfrom(
+        prep_entry->bape_value,
+        rxom,
+        BLE_ATT_PREP_WRITE_CMD_BASE_SZ,
+        OS_MBUF_PKTLEN(rxom) - BLE_ATT_PREP_WRITE_CMD_BASE_SZ);
+    if (rc != 0) {
+        ble_att_svr_prep_free(prep_entry);
+        *out_att_err = BLE_ATT_ERR_PREPARE_QUEUE_FULL;
+        return rc;
+    }
+
+    prep_prev = ble_att_svr_prep_find_prev(&conn->bhc_att_svr,
+                                           req->bapc_handle,
+                                           req->bapc_offset);
+    if (prep_prev == NULL) {
+        SLIST_INSERT_HEAD(&conn->bhc_att_svr.basc_prep_list, prep_entry,
+                          bape_next);
+    } else {
+        SLIST_INSERT_AFTER(prep_prev, prep_entry, bape_next);
+    }
+
+    return 0;
+}
+
 int
 ble_att_svr_rx_prep_write(uint16_t conn_handle, struct os_mbuf **rxom)
 {
@@ -2368,17 +2420,13 @@ ble_att_svr_rx_prep_write(uint16_t conn_handle, struct os_mbuf **rxom)
 #endif
 
     struct ble_att_prep_write_cmd req;
-    struct ble_att_prep_entry *prep_entry;
-    struct ble_att_prep_entry *prep_prev;
     struct ble_att_svr_entry *attr_entry;
-    struct ble_hs_conn *conn;
     struct os_mbuf *txom;
     uint16_t err_handle;
     uint8_t att_err;
     int rc;
 
     /* Initialize some values in case of early error. */
-    prep_entry = NULL;
     txom = NULL;
     att_err = 0;
     err_handle = 0;
@@ -2419,49 +2467,16 @@ ble_att_svr_rx_prep_write(uint16_t conn_handle, struct os_mbuf **rxom)
         goto done;
     }
 
-    prep_entry = ble_att_svr_prep_alloc();
-    if (prep_entry == NULL) {
-        att_err = BLE_ATT_ERR_PREPARE_QUEUE_FULL;
-        rc = BLE_HS_ENOMEM;
-        goto done;
-    }
-    prep_entry->bape_handle = req.bapc_handle;
-    prep_entry->bape_offset = req.bapc_offset;
-
     ble_hs_lock();
-
-    conn = ble_hs_conn_find(conn_handle);
-    if (conn == NULL) {
-        rc = BLE_HS_ENOTCONN;
-    } else {
-        prep_prev = ble_att_svr_prep_find_prev(&conn->bhc_att_svr,
-                                               req.bapc_handle,
-                                               req.bapc_offset);
-        if (prep_prev == NULL) {
-            SLIST_INSERT_HEAD(&conn->bhc_att_svr.basc_prep_list, prep_entry,
-                              bape_next);
-        } else {
-            SLIST_INSERT_AFTER(prep_prev, prep_entry, bape_next);
-        }
-
-        /* Append attribute value from request onto prep mbuf. */
-        rc = os_mbuf_appendfrom(prep_entry->bape_value, *rxom,
-                                BLE_ATT_PREP_WRITE_CMD_BASE_SZ,
-                                OS_MBUF_PKTLEN(*rxom) -
-                                    BLE_ATT_PREP_WRITE_CMD_BASE_SZ);
-        if (rc != 0) {
-            att_err = BLE_ATT_ERR_PREPARE_QUEUE_FULL;
-        }
-    }
-
+    rc = ble_att_svr_insert_prep_entry(conn_handle, &req, *rxom, &att_err);
     ble_hs_unlock();
 
     if (rc != 0) {
         goto done;
     }
 
-    /* Reuse rxom for response.  Response is identical to request except for op
-     * code.
+    /* Reuse rxom for response.  Response is identical to request except for
+     * op code.
      */
     txom = *rxom;
     *rxom = NULL;
@@ -2473,32 +2488,8 @@ ble_att_svr_rx_prep_write(uint16_t conn_handle, struct os_mbuf **rxom)
     rc = 0;
 
 done:
-    if (rc != 0 && rc != BLE_HS_ENOTCONN) {
-        ble_hs_lock();
-
-        conn = ble_hs_conn_find(conn_handle);
-        if (conn == NULL) {
-            rc = BLE_HS_ENOTCONN;
-        } else {
-            if (prep_entry != NULL) {
-                if (prep_prev == NULL) {
-                    SLIST_REMOVE_HEAD(&conn->bhc_att_svr.basc_prep_list,
-                                      bape_next);
-                } else {
-                    SLIST_NEXT(prep_prev, bape_next) =
-                        SLIST_NEXT(prep_entry, bape_next);
-                }
-
-                ble_att_svr_prep_free(prep_entry);
-            }
-        }
-
-        ble_hs_unlock();
-    }
-
     rc = ble_att_svr_tx_rsp(conn_handle, rc, txom, BLE_ATT_OP_PREP_WRITE_REQ,
                             att_err, err_handle);
-
     return rc;
 }