This is an automated email from the ASF dual-hosted git repository.

janc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-nimble.git


The following commit(s) were added to refs/heads/master by this push:
     new 1f65494e2 nimble/host: Refactor L2CAP signaling PDU parsing
1f65494e2 is described below

commit 1f65494e22312b145ed74f45a573572ca5470856
Author: Szymon Janc <[email protected]>
AuthorDate: Tue Mar 4 11:50:13 2025 +0100

    nimble/host: Refactor L2CAP signaling PDU parsing
    
    Add helper that validates if PDU size is valid. This fixes
    L2CAP/COS/CED/BI-11-C qualification test.
---
 nimble/host/src/ble_l2cap_sig.c | 87 +++++++++++++++++++++++++----------------
 1 file changed, 54 insertions(+), 33 deletions(-)

diff --git a/nimble/host/src/ble_l2cap_sig.c b/nimble/host/src/ble_l2cap_sig.c
index 2188b5913..f360ad708 100644
--- a/nimble/host/src/ble_l2cap_sig.c
+++ b/nimble/host/src/ble_l2cap_sig.c
@@ -407,6 +407,38 @@ ble_l2cap_sig_check_conn_params(const struct 
ble_gap_upd_params *params)
     return 0;
 }
 
+/* This helper does validation of sizes and (if requested) CIDs sizes */
+static int
+ble_l2cap_sig_mbuf_pullup_base(struct os_mbuf **om, int base_len, unsigned int 
*cids_count)
+{
+    if (cids_count) {
+        if (OS_MBUF_PKTLEN(*om) < base_len) {
+            return BLE_HS_EBADDATA;
+        }
+
+        if ((OS_MBUF_PKTLEN(*om) - base_len) % sizeof(uint16_t)) {
+            return BLE_HS_EBADDATA;
+        }
+
+        *cids_count = (OS_MBUF_PKTLEN(*om) - base_len) / sizeof(uint16_t);
+        if (*cids_count == 0 || *cids_count > BLE_L2CAP_MAX_COC_CONN_REQ) {
+            return BLE_HS_EBADDATA;
+        }
+    } else {
+        if (OS_MBUF_PKTLEN(*om) != base_len) {
+            return BLE_HS_EBADDATA;
+        }
+    }
+
+    /* data sizes validated, just pullup all */
+    *om = os_mbuf_pullup(*om, OS_MBUF_PKTLEN(*om));
+    if (*om == NULL) {
+        return BLE_HS_ENOMEM;
+    }
+
+    return 0;
+}
+
 int
 ble_l2cap_sig_update_req_rx(uint16_t conn_handle,
                             struct ble_l2cap_sig_hdr *hdr,
@@ -423,7 +455,7 @@ ble_l2cap_sig_update_req_rx(uint16_t conn_handle,
 
     l2cap_result = 0; /* Silence spurious gcc warning. */
 
-    rc = ble_hs_mbuf_pullup_base(om, BLE_L2CAP_SIG_UPDATE_REQ_SZ);
+    rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*req), NULL);
     if (rc != 0) {
         return rc;
     }
@@ -500,7 +532,7 @@ ble_l2cap_sig_update_rsp_rx(uint16_t conn_handle,
         return 0;
     }
 
-    rc = ble_hs_mbuf_pullup_base(om, BLE_L2CAP_SIG_UPDATE_RSP_SZ);
+    rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*rsp), NULL);
     if (rc != 0) {
         cb_status = rc;
         goto done;
@@ -766,12 +798,12 @@ ble_l2cap_sig_credit_base_reconfig_req_rx(uint16_t 
conn_handle,
     struct ble_l2cap_sig_credit_base_reconfig_rsp *rsp;
     struct ble_hs_conn *conn;
     struct os_mbuf *txom;
-    int i;
+    unsigned int i;
     int rc;
-    uint8_t cid_cnt;
+    unsigned int cid_cnt;
     uint8_t reduction_mps = 0;
 
-    rc = ble_hs_mbuf_pullup_base(om, hdr->length);
+    rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*req), &cid_cnt);
     if (rc != 0) {
         return rc;
     }
@@ -794,11 +826,6 @@ ble_l2cap_sig_credit_base_reconfig_req_rx(uint16_t 
conn_handle,
         return 0;
     }
 
-    if (hdr->length <= sizeof(*req)) {
-        rsp->result = htole16(BLE_L2CAP_ERR_RECONFIG_UNACCEPTED_PARAM);
-        goto failed;
-    }
-
     req = (struct ble_l2cap_sig_credit_base_reconfig_req *)(*om)->om_data;
 
     if ((req->mps < BLE_L2CAP_ECOC_MIN_MTU) || (req->mtu < 
BLE_L2CAP_ECOC_MIN_MTU)) {
@@ -809,12 +836,6 @@ ble_l2cap_sig_credit_base_reconfig_req_rx(uint16_t 
conn_handle,
     /* Assume request will succeed. If not, result will be updated */
     rsp->result = htole16(BLE_L2CAP_ERR_RECONFIG_SUCCEED);
 
-    cid_cnt = (hdr->length - sizeof(*req)) / sizeof(uint16_t);
-    if (cid_cnt > BLE_L2CAP_MAX_COC_CONN_REQ) {
-        rsp->result = htole16(BLE_L2CAP_ERR_RECONFIG_UNACCEPTED_PARAM);
-        goto failed;
-    }
-
     for (i = 0; i < cid_cnt; i++) {
         chan[i] = ble_hs_conn_chan_find_by_dcid(conn, req->dcids[i]);
         if (!chan[i]) {
@@ -899,7 +920,7 @@ ble_l2cap_sig_credit_base_reconfig_rsp_rx(uint16_t 
conn_handle,
         return 0;
     }
 
-    rc = ble_hs_mbuf_pullup_base(om, hdr->length);
+    rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*rsp), NULL);
     if (rc != 0) {
         return rc;
     }
@@ -923,17 +944,17 @@ ble_l2cap_sig_credit_base_con_req_rx(uint16_t conn_handle,
     struct ble_l2cap_chan *chans[5] = { 0 };
     struct ble_hs_conn *conn;
     uint16_t scid;
-    uint8_t num_of_scids;
+    unsigned int num_of_scids;
     uint8_t chan_created = 0;
     int i;
-    uint8_t len;
+    unsigned int len;
 
-    rc = ble_hs_mbuf_pullup_base(om, hdr->length);
+    rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*req), &num_of_scids);
     if (rc != 0) {
         return rc;
     }
 
-    len = (hdr->length > sizeof(*req)) ? hdr->length : sizeof(*req);
+    len = sizeof(*rsp) + (num_of_scids * sizeof(rsp->dcids[0]));
 
     rsp = ble_l2cap_sig_cmd_get(BLE_L2CAP_SIG_OP_CREDIT_CONNECT_RSP,
                                 hdr->identifier, len , &txom);
@@ -960,12 +981,6 @@ ble_l2cap_sig_credit_base_con_req_rx(uint16_t conn_handle,
 
     req = (struct ble_l2cap_sig_credit_base_connect_req *)(*om)->om_data;
 
-    num_of_scids = (hdr->length - sizeof(*req)) / sizeof(uint16_t);
-    if (num_of_scids > 5) {
-        rsp->result = htole16(BLE_L2CAP_COC_ERR_INVALID_PARAMETERS);
-        goto failed;
-    }
-
     if ((req->mtu < BLE_L2CAP_ECOC_MIN_MTU) || (req->mps < 
BLE_L2CAP_ECOC_MIN_MTU)) {
         rsp->result = htole16(BLE_L2CAP_COC_ERR_INVALID_PARAMETERS);
         goto failed;
@@ -1089,6 +1104,7 @@ ble_l2cap_sig_credit_base_con_rsp_rx(uint16_t conn_handle,
     int rc;
     int i;
     uint16_t duplicated_cids[5] = {};
+    unsigned int num_of_dcids;
 
 #if !BLE_MONITOR
     BLE_HS_LOG(DEBUG, "L2CAP LE COC connection response received\n");
@@ -1101,11 +1117,16 @@ ble_l2cap_sig_credit_base_con_rsp_rx(uint16_t 
conn_handle,
         return 0;
     }
 
-    rc = ble_hs_mbuf_pullup_base(om, hdr->length);
+    rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*rsp), &num_of_dcids);
     if (rc != 0) {
         goto done;
     }
 
+    /* spec doesn't say what to do in that case so just fail */
+    if (proc->connect.chan_cnt != num_of_dcids) {
+        goto done;
+    }
+
     rsp = (struct ble_l2cap_sig_credit_base_connect_rsp *)(*om)->om_data;
 
     if (rsp->result) {
@@ -1181,7 +1202,7 @@ ble_l2cap_sig_coc_req_rx(uint16_t conn_handle, struct 
ble_l2cap_sig_hdr *hdr,
     struct ble_hs_conn *conn;
     uint16_t scid;
 
-    rc = ble_hs_mbuf_pullup_base(om, sizeof(*req));
+    rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*req), NULL);
     if (rc != 0) {
         return rc;
     }
@@ -1295,7 +1316,7 @@ ble_l2cap_sig_coc_rsp_rx(uint16_t conn_handle, struct 
ble_l2cap_sig_hdr *hdr,
         return 0;
     }
 
-    rc = ble_hs_mbuf_pullup_base(om, sizeof(*rsp));
+    rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*rsp), NULL);
     if (rc != 0) {
         goto done;
     }
@@ -1577,7 +1598,7 @@ ble_l2cap_sig_disc_req_rx(uint16_t conn_handle, struct 
ble_l2cap_sig_hdr *hdr,
     uint16_t scid;
     uint16_t dcid;
 
-    rc = ble_hs_mbuf_pullup_base(om, sizeof(*req));
+    rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*req), NULL);
     if (rc != 0) {
         return rc;
     }
@@ -1690,7 +1711,7 @@ ble_l2cap_sig_disc_rsp_rx(uint16_t conn_handle, struct 
ble_l2cap_sig_hdr *hdr,
         return 0;
     }
 
-    rc = ble_hs_mbuf_pullup_base(om, sizeof(*rsp));
+    rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*rsp), NULL);
     if (rc != 0) {
         goto done;
     }
@@ -1770,7 +1791,7 @@ ble_l2cap_sig_le_credits_rx(uint16_t conn_handle, struct 
ble_l2cap_sig_hdr *hdr,
     struct ble_l2cap_sig_le_credits *req;
     int rc;
 
-    rc = ble_hs_mbuf_pullup_base(om, sizeof(*req));
+    rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*req), NULL);
     if (rc != 0) {
         return 0;
     }

Reply via email to