Re: [PATCH 3/3] Bluetooth: Don't leave abandoned SCO links up in the controller

[Luiz Augusto von Dentz]

Hi Hitalo,

On Wed, Oct 7, 2026 at 11:44 AM Hitalo Souza <enghitalo@xxxxxxxxx> wrote:
>
> Since commit a13f316e90fd ("Bluetooth: hci_conn: Consolidate code for
> aborting connections"), aborting a SCO/eSCO connection whose setup is
> pending, e.g. because its socket was closed, sends Create Connection
> Cancel and deletes the hci_conn. That command cannot cancel a
> synchronous connection, controllers just fail it (ACL Connection Already
> Exists or Unknown Connection Identifier), and the link may still
> complete afterwards. Unless another connection to the device takes it
> over, its completion event then finds no connection waiting for it and
> is ignored; if it comes while the abort is still queued, the connection
> refuses the handle as it is being aborted and is deleted all the same.
> Either way the link stays up in the controller, which rejects every
> later setup for the device until the ACL drops (with Unsupported LMP
> Parameter Value on a MediaTek MT7921). The same happens to a second
> link completing for a connection that is already up, e.g. its own setup
> after it took over the abandoned one.
>
> There is no command to cancel a pending SCO/eSCO setup, so don't send
> Create Connection Cancel for one, and disconnect a SCO/eSCO link that
> completes with no connection to take it.
>
> With the disable_esco parameter of sco.c, a SCO_LINK connection can get
> an eSCO link, which it does not take: its connect times out and the link
> stays up until the ACL drops. That is not changed here, and such a link
> is not disconnected while a SCO_LINK connection waits for its link.
>
> Link: https://github.com/bluez/bluez/issues/2562
> Fixes: a13f316e90fd ("Bluetooth: hci_conn: Consolidate code for aborting connections")
> Assisted-by: LLM
> Signed-off-by: Hitalo Souza <enghitalo@xxxxxxxxx>
> ---
>  net/bluetooth/hci_event.c | 61 +++++++++++++++++++++++++++++++++++----
>  net/bluetooth/hci_sync.c  |  8 +++++
>  2 files changed, 64 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index cbe53e19e..102a0a4d1 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3217,6 +3217,27 @@ static int hci_read_enc_key_size(struct hci_dev *hdev, struct hci_conn *conn)
>         return hci_send_cmd(hdev, HCI_OP_READ_ENC_KEY_SIZE, sizeof(cp), &cp);
>  }
>
> +/* A SCO/eSCO link that completes with no connection to take it, e.g.
> + * because its setup was abandoned while pending, must be disconnected:
> + * otherwise it stays up in the controller, which then rejects every further
> + * setup for the device.
> + */
> +static void hci_sco_disconnect_orphan(struct hci_dev *hdev, __le16 handle)
> +{
> +       struct hci_cp_disconnect cp;
> +       u16 h = __le16_to_cpu(handle);
> +
> +       /* Never for an invalid handle or one that a connection uses */
> +       if (h > HCI_CONN_HANDLE_MAX || hci_conn_hash_lookup_handle(hdev, h))
> +               return;
> +
> +       bt_dev_dbg(hdev, "handle 0x%4.4x", h);
> +
> +       cp.handle = handle;
> +       cp.reason = HCI_ERROR_REMOTE_USER_TERM;
> +       hci_send_cmd(hdev, HCI_OP_DISCONNECT, sizeof(cp), &cp);
> +}
> +
>  static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
>                                   struct sk_buff *skb)
>  {
> @@ -3273,8 +3294,10 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
>
>                         conn = hci_conn_hash_lookup_ba(hdev, ESCO_LINK,
>                                                        &ev->bdaddr);
> -                       if (!conn)
> +                       if (!conn) {
> +                               hci_sco_disconnect_orphan(hdev, ev->handle);
>                                 goto unlock;
> +                       }
>
>                         conn->type = SCO_LINK;
>                 }
> @@ -3293,8 +3316,12 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
>
>         if (!status) {
>                 status = hci_conn_set_handle(conn, __le16_to_cpu(ev->handle));
> -               if (status)
> +               if (status) {
> +                       /* e.g. it is being aborted: the link is not taken */
> +                       if (ev->link_type == SCO_LINK)
> +                               hci_sco_disconnect_orphan(hdev, ev->handle);
>                         goto done;
> +               }
>
>                 if (conn->type == ACL_LINK) {
>                         conn->state = BT_CONFIG;
> @@ -5178,8 +5205,18 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
>
>         conn = hci_conn_hash_lookup_ba(hdev, ev->link_type, &ev->bdaddr);
>         if (!conn) {
> -               if (ev->link_type == ESCO_LINK)
> -                       goto unlock;
> +               if (ev->link_type == ESCO_LINK) {
> +                       /* A SCO_LINK connection still waiting for its link can
> +                        * get an eSCO one, see disable_esco in sco.c. It does
> +                        * not take it, as before, and the link is left alone.
> +                        */
> +                       conn = hci_conn_hash_lookup_ba(hdev, SCO_LINK,
> +                                                      &ev->bdaddr);
> +                       if (conn && HCI_CONN_HANDLE_UNSET(conn->handle))
> +                               goto unlock;
> +
> +                       goto orphan;
> +               }
>
>                 /* When the link type in the event indicates SCO connection
>                  * and lookup of the connection object fails, then check
> @@ -5192,7 +5229,7 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
>                  */
>                 conn = hci_conn_hash_lookup_ba(hdev, ESCO_LINK, &ev->bdaddr);
>                 if (!conn)
> -                       goto unlock;
> +                       goto orphan;
>         }
>
>         /* The HCI_Synchronous_Connection_Complete event is only sent once per connection.
> @@ -5202,6 +5239,13 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
>          * whether the connection is already set up.
>          */
>         if (!HCI_CONN_HANDLE_UNSET(conn->handle)) {
> +               /* Another link for a connection that is already up, e.g. its
> +                * own setup completing after it took over an abandoned one,
> +                * has no connection waiting for it either.
> +                */
> +               if (__le16_to_cpu(ev->handle) != conn->handle)
> +                       goto orphan;
> +
>                 bt_dev_err(hdev, "Ignoring HCI_Sync_Conn_Complete event for existing connection");
>                 goto unlock;
>         }
> @@ -5210,6 +5254,8 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
>         case 0x00:
>                 status = hci_conn_set_handle(conn, __le16_to_cpu(ev->handle));
>                 if (status) {
> +                       /* e.g. it is being aborted: the link is not taken */
> +                       hci_sco_disconnect_orphan(hdev, ev->handle);
>                         conn->state = BT_CLOSED;
>                         break;
>                 }
> @@ -5260,6 +5306,11 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
>         hci_connect_cfm(conn, status);
>         if (status)
>                 hci_conn_del(conn);
> +       goto unlock;
> +
> +orphan:
> +       if (!status)
> +               hci_sco_disconnect_orphan(hdev, ev->handle);
>
>  unlock:
>         hci_dev_unlock(hdev);
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 9eca6757d..4e1c72fd2 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -5940,6 +5940,14 @@ static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn,
>                 return 0;
>         }
>
> +       if (conn->type == SCO_LINK || conn->type == ESCO_LINK) {
> +               /* There is no command to cancel a pending SCO/eSCO setup. If
> +                * the link completes anyway, it is disconnected then, unless
> +                * a new connection to the device takes it.
> +                */
> +               return 0;
> +       }
> +
>         if (hdev->hci_ver < BLUETOOTH_VER_1_2)
>                 return 0;
>
> --
> 2.55.0

Sashiko flagged quite a few problems:

https://sashiko.dev/#/patchset/20261007154353.148223-1-enghitalo%40gmail.com

We need to check if the logic of hci_sco_disconnect_orphan couldn't be
made more generically, so in case the connection was aborted but we
received the connection complete that shall always result in
HCI_OP_DISCONNECT so the handle don't stay active in the controller.


-- 
Luiz Augusto von Dentz