You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by an...@apache.org on 2022/09/29 17:23:00 UTC

[mynewt-nimble] branch master updated (ef03c7c1 -> ef7c30dc)

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

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


    from ef03c7c1 nimble/ll: Always send assert vs event if enabled
     new 9257bfec nimble/ll: Fix backoff handling for aux scan
     new 0c3e1b72 nimble/ll: Fix active scan on aux
     new ef7c30dc nimble/ll: Fix aux_data leak

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 nimble/controller/src/ble_ll_scan.c     |  6 ++---
 nimble/controller/src/ble_ll_scan_aux.c | 43 ++++++++++++++++-----------------
 2 files changed, 23 insertions(+), 26 deletions(-)


[mynewt-nimble] 02/03: nimble/ll: Fix active scan on aux

Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 0c3e1b728c43250bfc73f1ebde68a28c79047597
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Thu Sep 29 01:01:26 2022 +0200

    nimble/ll: Fix active scan on aux
    
    We should not allow to free aux_data from pkt_in if there was
    AUX_SCAN_REQ sent for that pdu as this will result in use-after-free of
    aux_data when AUX_SCAN_RSP is received.
    
    This can happen if we send AUX_SCAN_REQ from isr and then in pkt_in
    figure out this is a duplicate so we don't want to scan this chain
    anymore. We should just wait for AUX_SCAN_RSP and stop there.
---
 nimble/controller/src/ble_ll_scan_aux.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/nimble/controller/src/ble_ll_scan_aux.c b/nimble/controller/src/ble_ll_scan_aux.c
index 57d79f92..60b4b5c1 100644
--- a/nimble/controller/src/ble_ll_scan_aux.c
+++ b/nimble/controller/src/ble_ll_scan_aux.c
@@ -1724,12 +1724,14 @@ ble_ll_scan_aux_rx_pkt_in(struct os_mbuf *rxpdu, struct ble_mbuf_hdr *rxhdr)
         aux->hci_state |= BLE_LL_SCAN_AUX_H_DONE;
     }
 
-    /*
-     * If we are done processing this chain and aux scan was not scheduled or
-     * we removed it from scheduler, we can remove aux_data now. Otherwise we
-     * will remove on next pkt_in.
+    /* If we are done processing this chain we can remove aux_data now if:
+     * - we did not send AUX_SCAN_REQ for this PDU
+     * - there was no aux scan scheduled from this PDU
+     * - there was aux scan scheduled from this PDU but we removed it
+     * In other cases, we'll remove aux_data on next pkt_in.
      */
     if ((aux->hci_state & BLE_LL_SCAN_AUX_H_DONE) &&
+        !(rxinfo->flags & BLE_MBUF_HDR_F_SCAN_REQ_TXD) &&
         (!(rxinfo->flags & BLE_MBUF_HDR_F_AUX_PTR_WAIT) ||
          (ble_ll_sched_rmv_elem(&aux->sch) == 0))) {
         ble_ll_scan_aux_free(aux);


[mynewt-nimble] 03/03: nimble/ll: Fix aux_data leak

Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit ef7c30dcbe0af4d3cc8802281341088bb7dd2c0a
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Thu Sep 29 10:16:51 2022 +0200

    nimble/ll: Fix aux_data leak
    
    We should mark hci stream if ext adv report event is either disabled or
    cannot be allocated, otherwise we will never free aux_data.
---
 nimble/controller/src/ble_ll_scan_aux.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/nimble/controller/src/ble_ll_scan_aux.c b/nimble/controller/src/ble_ll_scan_aux.c
index 60b4b5c1..63e2feb2 100644
--- a/nimble/controller/src/ble_ll_scan_aux.c
+++ b/nimble/controller/src/ble_ll_scan_aux.c
@@ -587,6 +587,7 @@ ble_ll_hci_ev_send_ext_adv_report_for_aux(struct os_mbuf *rxpdu,
     int rc;
 
     if (!ble_ll_hci_is_le_event_enabled(BLE_HCI_LE_SUBEV_EXT_ADV_RPT)) {
+        aux->hci_state = BLE_LL_SCAN_AUX_H_DONE;
         return -1;
     }
 
@@ -601,6 +602,7 @@ ble_ll_hci_ev_send_ext_adv_report_for_aux(struct os_mbuf *rxpdu,
     } else {
         hci_ev = ble_ll_hci_ev_alloc_ext_adv_report_for_aux(addrd, aux);
         if (!hci_ev) {
+            aux->hci_state = BLE_LL_SCAN_AUX_H_DONE;
             return -1;
         }
     }


[mynewt-nimble] 01/03: nimble/ll: Fix backoff handling for aux scan

Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 9257bfecf20f413ad3c3da540b22d319105cd846
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Thu Sep 29 00:53:02 2022 +0200

    nimble/ll: Fix backoff handling for aux scan
    
    We should update backoff after receiving scan response PDU instead of
    waiting for complete chain to be received.
    
    This also fixes problem where we try to update backoff in an invalid
    state, i.e. backoff_count is non-zero. It happens if we start to scan
    response chain with backoff_count=0 and before complete chain is scanned
    we scan another pdu which fails. This updates backoff_count to non-zero
    value so when we finish scanning chain and try to update backoff, the
    backoff_count value is non-zero which is considered an invalid state.
    
    [Core 5.3, Vol 6, Part B, 4.4.3.2]
---
 nimble/controller/src/ble_ll_scan.c     |  6 ++----
 nimble/controller/src/ble_ll_scan_aux.c | 31 +++++++++++++------------------
 2 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/nimble/controller/src/ble_ll_scan.c b/nimble/controller/src/ble_ll_scan.c
index 010fe150..75441bb5 100644
--- a/nimble/controller/src/ble_ll_scan.c
+++ b/nimble/controller/src/ble_ll_scan.c
@@ -1535,10 +1535,8 @@ ble_ll_scan_send_scan_req(uint8_t pdu_type, uint8_t *rxbuf,
     BLE_LL_ASSERT(scansm->scan_rsp_pending == 0);
 
     /* We want to send a request. See if backoff allows us */
-    if (scansm->backoff_count > 0) {
-        if (--scansm->backoff_count != 0) {
-            return false;
-        }
+    if (ble_ll_scan_backoff_kick() != 0) {
+        return false;
     }
 
 #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PRIVACY)
diff --git a/nimble/controller/src/ble_ll_scan_aux.c b/nimble/controller/src/ble_ll_scan_aux.c
index 01e516fd..57d79f92 100644
--- a/nimble/controller/src/ble_ll_scan_aux.c
+++ b/nimble/controller/src/ble_ll_scan_aux.c
@@ -198,21 +198,6 @@ ble_ll_scan_aux_free(struct ble_ll_scan_aux_data *aux)
     os_memblock_put(&aux_data_pool, aux);
 }
 
-static void
-ble_ll_scan_aux_update_scan_backoff(struct ble_ll_scan_aux_data *aux)
-{
-    if (!(aux->flags & BLE_LL_SCAN_AUX_F_W4_SCAN_RSP) &&
-        !(aux->flags & BLE_LL_SCAN_AUX_F_SCANNED)) {
-        return;
-    }
-
-    if ((aux->hci_state & BLE_LL_SCAN_AUX_H_DONE) &&
-        !(aux->hci_state & BLE_LL_SCAN_AUX_H_TRUNCATED)) {
-        ble_ll_scan_backoff_update(1);
-    } else {
-        ble_ll_scan_backoff_update(0);
-    }
-}
 
 static inline bool
 ble_ll_scan_aux_need_truncation(struct ble_ll_scan_aux_data *aux)
@@ -670,7 +655,10 @@ ble_ll_scan_aux_break_ev(struct ble_npl_event *ev)
         ble_ll_hci_ev_send_ext_adv_truncated_report(aux);
     }
 
-    ble_ll_scan_aux_update_scan_backoff(aux);
+    /* Update backoff if we were waiting for scan response */
+    if (aux->flags & BLE_LL_SCAN_AUX_F_W4_SCAN_RSP) {
+        ble_ll_scan_backoff_update(0);
+    }
 
     ble_ll_scan_aux_free(aux);
     ble_ll_scan_chk_resume();
@@ -1658,7 +1646,15 @@ ble_ll_scan_aux_rx_pkt_in(struct os_mbuf *rxpdu, struct ble_mbuf_hdr *rxhdr)
             aux->hci_state |= BLE_LL_SCAN_AUX_H_DONE;
         }
 
-        ble_ll_scan_aux_update_scan_backoff(aux);
+        /* Update backoff if we were waiting for scan response */
+        if (aux->flags & BLE_LL_SCAN_AUX_F_W4_SCAN_RSP) {
+            ble_ll_scan_backoff_update(0);
+        }
+    } else if (rxinfo->flags & BLE_MBUF_HDR_F_SCAN_RSP_RXD) {
+        /* We assume scan success when AUX_SCAN_RSP is received, no need to
+         * wait for complete chain (Core 5.3, Vol 6, Part B, 4.4.3.1).
+         */
+        ble_ll_scan_backoff_update(1);
     }
 
     if (aux->hci_state & BLE_LL_SCAN_AUX_H_DONE) {
@@ -1736,7 +1732,6 @@ ble_ll_scan_aux_rx_pkt_in(struct os_mbuf *rxpdu, struct ble_mbuf_hdr *rxhdr)
     if ((aux->hci_state & BLE_LL_SCAN_AUX_H_DONE) &&
         (!(rxinfo->flags & BLE_MBUF_HDR_F_AUX_PTR_WAIT) ||
          (ble_ll_sched_rmv_elem(&aux->sch) == 0))) {
-        ble_ll_scan_aux_update_scan_backoff(aux);
         ble_ll_scan_aux_free(aux);
     }