From 8576c799efed4ae5f11abd112eeba7a12a33a64c Mon Sep 17 00:00:00 2001
From: 94xhn <87560781+94xhn@users.noreply.github.com>
Date: Sun, 12 Jul 2026 00:53:50 +0800
Subject: [PATCH] nimble/transport: Fix double free in hci_h4_sm_completed()

hci_h4_sm_completed() (nimble/transport/common/hci_h4/src/hci_h4.c)
is the shared HCI-H4 framing state machine used by several
transports (apollo3, dialog_cmac host role, uart_ll, cdc). Once an
ACL/ISO frame is fully received it does:

  rc = h4sm->frame_cb(h4sm->pkt_type, h4sm->om);
  if (rc != 0) {
      os_mbuf_free_chain(h4sm->om);
  }
  h4sm->om = NULL;

For the LL->HS direction (apollo3_ble_hci_frame_cb,
hci_cmac_hs_frame_cb, uart_ll's hci_uart_frame_cb) frame_cb()
forwards ACL data to ble_transport_to_hs_acl(), which reaches
ble_hs_rx_data() in nimble/host/src/ble_hs.c. That function is
documented to "consume the supplied mbuf, regardless of the
outcome": on ble_mqueue_put() failure it already frees om via
os_mbuf_free_chain() before returning a non-zero status.

hci_h4_sm_completed() then frees the same h4sm->om a second time,
corrupting the mbuf pool. This is the same double-free pattern
already fixed for ble_hci_emspi_rx_acl() in PR #2265 / issue #2260,
but here it lives in the shared state machine and is reachable from
apollo3, dialog_cmac (host role) and uart_ll instead of a single
transport.

The other frame_cb() implementations reached from this same switch
case (ble_ll_hci_acl_rx()/ble_ll_hci_iso_rx() in the controller, for
the HS->LL direction used by uart/cdc/dialog_cmac controller role)
always return 0 and always take ownership of om themselves, so
removing the conditional free does not change behavior for them.
The HCI_H4_CMD/HCI_H4_EVT branch right above is intentionally left
untouched: ble_ll_hci_cmd_rx() can legitimately return non-zero
*without* freeing the buffer (the "command busy" path), so that
branch's cleanup-on-error is correct as-is and follows a different
ownership contract than the ACL/ISO one.

Verified with a standalone host-side program that reproduces the
real struct layouts and control flow of hci_h4_sm_completed(),
ble_hs_rx_data() and ble_mqueue_put() against a real malloc()/
free()-backed mbuf: the current code frees the same pointer twice
and the process aborts with STATUS_HEAP_CORRUPTION (0xC0000374);
with this change applied only a single free() occurs and the
process exits cleanly (code 0).

Disclosure: this fix, its analysis and the accompanying
reproduction program were produced with the assistance of an AI
coding agent (Claude), reviewed and submitted by the human author.

Signed-off-by: 94xhn <87560781+94xhn@users.noreply.github.com>
---
 nimble/transport/common/hci_h4/src/hci_h4.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/nimble/transport/common/hci_h4/src/hci_h4.c b/nimble/transport/common/hci_h4/src/hci_h4.c
index fd89154a34..9dbd815428 100644
--- a/nimble/transport/common/hci_h4/src/hci_h4.c
+++ b/nimble/transport/common/hci_h4/src/hci_h4.c
@@ -257,10 +257,17 @@ hci_h4_sm_completed(struct hci_h4_sm *h4sm)
     case HCI_H4_ISO:
         if (h4sm->om) {
             assert(h4sm->frame_cb);
-            rc = h4sm->frame_cb(h4sm->pkt_type, h4sm->om);
-            if (rc != 0) {
-                os_mbuf_free_chain(h4sm->om);
-            }
+            /*
+             * Unlike the HCI_H4_CMD/HCI_H4_EVT case above, frame_cb() always
+             * takes ownership of h4sm->om here, regardless of the status it
+             * returns. For the LL->HS direction (e.g. apollo3, dialog_cmac,
+             * uart_ll) frame_cb() forwards to ble_transport_to_hs_acl(),
+             * which reaches ble_hs_rx_data(); that function is documented to
+             * consume the supplied mbuf "regardless of the outcome" and
+             * already frees it on failure. Freeing h4sm->om again here would
+             * be a double free, so just propagate the status instead.
+             */
+            h4sm->frame_cb(h4sm->pkt_type, h4sm->om);
             h4sm->om = NULL;
         }
         break;