Re: [PATCH 2/2] Bluetooth: hci_core: Handle skb_clone() failure in hci_send_cmd_sync()

Hi Hans, Ibrahim,

On Mon, Aug 17, 2026 at 4:50 PM Hans de Goede
<[email protected]> wrote:
>
> From: Ibrahim Abdelkader <[email protected]>
>
> hci_send_cmd_sync() does not handle the second skb_clone() failure as it
> does the first one. At that point the command has already been sent and
> HCI_CMD_PENDING is set, so an allocation failure leaves the flag set with
> req_skb NULL. The response then matches nothing and the caller blocks for
> HCI_INIT_TIMEOUT before getting -ETIMEDOUT, reporting a memory shortage as
> a controller timeout.
>
> Clear the flag, cancel the pending request with -ENOMEM so the caller fails
> immediately instead of waiting out the timeout, and return -ENOMEM so
> hci_cmd_work() does not arm cmd_timer for a request that no longer exists.
>
> Clearing HCI_CMD_PENDING here is safe: all code touching that flag runs
> from hdev->workqueue, which is an ordered workqueue, so those paths are
> serialised and cannot race. The one exception is the cleanup added by
> commit ed516e3f1aa6 ("Bluetooth: hci_sync: Clear HCI_CMD_PENDING when
> dropping the last request"), which runs on hci_dev_open_sync() failure and
> from hci_dev_close_sync(), and the workqueue is stopped before that runs.
>
> Fixes: 2615fd9a7c25 ("Bluetooth: hci_sync: Fix overwriting request callback")
> Signed-off-by: Ibrahim Abdelkader <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>
> ---
>  net/bluetooth/hci_core.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8aeb3024428b..9585b42e7970 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -4092,6 +4092,11 @@ static int hci_send_cmd_sync(struct hci_dev *hdev, struct sk_buff *skb)
>             !hci_dev_test_and_set_flag(hdev, HCI_CMD_PENDING)) {
>                 kfree_skb(hdev->req_skb);
>                 hdev->req_skb = skb_clone(hdev->sent_cmd, GFP_KERNEL);
> +               if (!hdev->req_skb) {
> +                       hci_dev_clear_flag(hdev, HCI_CMD_PENDING);
> +                       hci_cmd_sync_cancel_sync(hdev, ENOMEM);
> +                       err = -ENOMEM;
> +               }

Sashiko flagged a problem with this change:

https://sashiko.dev/#/patchset/20260817205017.56524-1-johannes.goede%40oss.qualcomm.com

Im with the opinion that this shouldn't cancel the request, because at
this point it may already been sent to the hardware so maybe we should
flag it somehow that req_skb wasn't able to be allocated, or something
like that, but the command is still outstanding.

>         }
>
>         return err;
> --
> 2.55.0
>


-- 
Luiz Augusto von Dentz