From 147ee3daaf4a71750fae6b4a1552568921fe6d67 Mon Sep 17 00:00:00 2001 From: Jonathan Rico Date: Thu, 11 Jul 2024 15:04:25 +0200 Subject: [PATCH] Bluetooth: host: Send host num completes as early as possible The Softdevice Controller now sends the disconnect event only after receiving all Host Num Completes for the packets it sent to the host. This is done for security reasons. In our current reassembly logic, it does not really matter when we withhold the num complete. Before this patch, it's the first fragment that is withheld, and after the patch it will be the last fragment that is withheld until the host is done processing. The flow control properties are maintained, just in a different way. Co-authored-by: Aleksander Wasaznik Signed-off-by: Jonathan Rico --- subsys/bluetooth/host/conn.c | 20 ++++++++++++++++---- subsys/bluetooth/host/conn_internal.h | 7 ++++++- subsys/bluetooth/host/hci_core.c | 9 +++++++++ subsys/bluetooth/host/hci_core.h | 2 ++ 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index daf78689320..6c8ca998f39 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -339,11 +339,12 @@ void bt_conn_reset_rx_state(struct bt_conn *conn) conn->rx = NULL; } -static void bt_acl_recv(struct bt_conn *conn, struct net_buf *buf, - uint8_t flags) +static void bt_acl_recv(struct bt_conn *conn, struct net_buf *buf, uint8_t flags) { uint16_t acl_total_len; + bt_acl_set_ncp_sent(buf, false); + /* Check packet boundary flags */ switch (flags) { case BT_ACL_START: @@ -355,7 +356,7 @@ static void bt_acl_recv(struct bt_conn *conn, struct net_buf *buf, LOG_DBG("First, len %u final %u", buf->len, (buf->len < sizeof(uint16_t)) ? 0 : sys_get_le16(buf->data)); - conn->rx = buf; + conn->rx = net_buf_ref(buf); break; case BT_ACL_CONT: if (!conn->rx) { @@ -385,7 +386,6 @@ static void bt_acl_recv(struct bt_conn *conn, struct net_buf *buf, } net_buf_add_mem(conn->rx, buf->data, buf->len); - net_buf_unref(buf); break; default: /* BT_ACL_START_NO_FLUSH and BT_ACL_COMPLETE are not allowed on @@ -402,6 +402,10 @@ static void bt_acl_recv(struct bt_conn *conn, struct net_buf *buf, /* Still not enough data received to retrieve the L2CAP header * length field. */ + bt_send_one_host_num_completed_packets(conn->handle); + bt_acl_set_ncp_sent(buf, true); + net_buf_unref(buf); + return; } @@ -409,9 +413,15 @@ static void bt_acl_recv(struct bt_conn *conn, struct net_buf *buf, if (conn->rx->len < acl_total_len) { /* L2CAP frame not complete. */ + bt_send_one_host_num_completed_packets(conn->handle); + bt_acl_set_ncp_sent(buf, true); + net_buf_unref(buf); + return; } + net_buf_unref(buf); + if (conn->rx->len > acl_total_len) { LOG_ERR("ACL len mismatch (%u > %u)", conn->rx->len, acl_total_len); bt_conn_reset_rx_state(conn); @@ -422,6 +432,8 @@ static void bt_acl_recv(struct bt_conn *conn, struct net_buf *buf, buf = conn->rx; conn->rx = NULL; + __ASSERT(buf->ref == 1, "buf->ref %d", buf->ref); + LOG_DBG("Successfully parsed %u byte L2CAP packet", buf->len); bt_l2cap_recv(conn, buf, true); } diff --git a/subsys/bluetooth/host/conn_internal.h b/subsys/bluetooth/host/conn_internal.h index 38be652cc71..31af420630d 100644 --- a/subsys/bluetooth/host/conn_internal.h +++ b/subsys/bluetooth/host/conn_internal.h @@ -187,7 +187,12 @@ struct acl_data { struct bt_buf_data buf_data; /* Index into the bt_conn storage array */ - uint8_t index; + uint8_t index; + + /** Host has already sent a Host Number of Completed Packets + * for this buffer. + */ + bool host_ncp_sent; /** ACL connection handle */ uint16_t handle; diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index 7ed86ecb922..b3f003b8c20 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/subsys/bluetooth/host/hci_core.c @@ -228,6 +228,11 @@ static void handle_vs_event(uint8_t event, struct net_buf *buf, /* Other possible errors are handled by handle_event_common function */ } +void bt_acl_set_ncp_sent(struct net_buf *packet, bool value) +{ + acl(packet)->host_ncp_sent = value; +} + void bt_send_one_host_num_completed_packets(uint16_t handle) { if (!IS_ENABLED(CONFIG_BT_HCI_ACL_FLOW_CONTROL)) { @@ -266,6 +271,10 @@ void bt_hci_host_num_completed_packets(struct net_buf *buf) net_buf_destroy(buf); + if (acl(buf)->host_ncp_sent) { + return; + } + /* Do nothing if controller to host flow control is not supported */ if (!BT_CMD_TEST(bt_dev.supported_commands, 10, 5)) { return; diff --git a/subsys/bluetooth/host/hci_core.h b/subsys/bluetooth/host/hci_core.h index 323b448323b..827d7a4b919 100644 --- a/subsys/bluetooth/host/hci_core.h +++ b/subsys/bluetooth/host/hci_core.h @@ -598,3 +598,5 @@ void bt_hci_le_per_adv_subevent_data_request(struct net_buf *buf); void bt_hci_le_per_adv_response_report(struct net_buf *buf); void bt_tx_irq_raise(void); +void bt_send_one_host_num_completed_packets(uint16_t handle); +void bt_acl_set_ncp_sent(struct net_buf *packet, bool value);