[PATCH] Bluetooth: hci_h4: Fix race during initialization

[PATCH] Bluetooth: hci_h4: Fix race during initialization

[Jonathan Rissanen]

Commit 5df5dafc171b ("Bluetooth: hci_uart: Fix another race during
initialization") fixed a race for hci commands sent during initialization.
However, there is still a race that happens if an hci event from one of
these commands is received before HCI_UART_REGISTERED has been set at
the end of hci_uart_register_dev(). The event will be ignored which
causes the command to fail with a timeout in the log:

"Bluetooth: hci0: command 0x1003 tx timeout"

This is because the hci event receive path (hci_uart_tty_receive ->
h4_recv) requires HCI_UART_REGISTERED to be set in h4_recv(), while the
hci command transmit path (hci_uart_send_frame -> h4_enqueue) only
requires HCI_UART_PROTO_INIT to be set in hci_uart_send_frame().

The check for HCI_UART_REGISTERED was originally added in commit
c2578202919a ("Bluetooth: Fix H4 crash from incoming UART packets")
to fix a crash caused by hu->hdev being null dereferenced. That can no
longer happen: once HCI_UART_PROTO_INIT is set in hci_uart_register_dev()
all pointers (hu, hu->priv and hu->hdev) are valid, and
hci_uart_tty_receive() already calls h4_recv() on HCI_UART_PROTO_INIT
or HCI_UART_PROTO_READY.

Remove the check for HCI_UART_REGISTERED in h4_recv() to fix the race
condition.

Signed-off-by: Jonathan Rissanen <jonathan.rissanen@axis.com>
---
 drivers/bluetooth/hci_h4.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/bluetooth/hci_h4.c b/drivers/bluetooth/hci_h4.c
index ec017df8572c..1e9e2cad9ddf 100644
--- a/drivers/bluetooth/hci_h4.c
+++ b/drivers/bluetooth/hci_h4.c
@@ -109,9 +109,6 @@ static int h4_recv(struct hci_uart *hu, const void *data, int count)
 {
 	struct h4_struct *h4 = hu->priv;
 
-	if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
-		return -EUNATCH;
-
 	h4->rx_skb = h4_recv_buf(hu, h4->rx_skb, data, count,
 				 h4_recv_pkts, ARRAY_SIZE(h4_recv_pkts));
 	if (IS_ERR(h4->rx_skb)) {

---
base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
change-id: 20260303-hci-init-fix-9657128a0104

Best regards,
-- 
Jonathan Rissanen <jonathan.rissanen@axis.com>

Re: [PATCH] Bluetooth: hci_h4: Fix race during initialization

[Luiz Augusto von Dentz]

Hi Jonathan,

On Fri, Mar 20, 2026 at 8:10 AM Jonathan Rissanen
<jonathan.rissanen@axis.com> wrote:
>
> Commit 5df5dafc171b ("Bluetooth: hci_uart: Fix another race during
> initialization") fixed a race for hci commands sent during initialization.
> However, there is still a race that happens if an hci event from one of
> these commands is received before HCI_UART_REGISTERED has been set at
> the end of hci_uart_register_dev(). The event will be ignored which
> causes the command to fail with a timeout in the log:
>
> "Bluetooth: hci0: command 0x1003 tx timeout"
>
> This is because the hci event receive path (hci_uart_tty_receive ->
> h4_recv) requires HCI_UART_REGISTERED to be set in h4_recv(), while the
> hci command transmit path (hci_uart_send_frame -> h4_enqueue) only
> requires HCI_UART_PROTO_INIT to be set in hci_uart_send_frame().
>
> The check for HCI_UART_REGISTERED was originally added in commit
> c2578202919a ("Bluetooth: Fix H4 crash from incoming UART packets")
> to fix a crash caused by hu->hdev being null dereferenced. That can no
> longer happen: once HCI_UART_PROTO_INIT is set in hci_uart_register_dev()
> all pointers (hu, hu->priv and hu->hdev) are valid, and
> hci_uart_tty_receive() already calls h4_recv() on HCI_UART_PROTO_INIT
> or HCI_UART_PROTO_READY.
>
> Remove the check for HCI_UART_REGISTERED in h4_recv() to fix the race
> condition.
>
> Signed-off-by: Jonathan Rissanen <jonathan.rissanen@axis.com>
> ---
>  drivers/bluetooth/hci_h4.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_h4.c b/drivers/bluetooth/hci_h4.c
> index ec017df8572c..1e9e2cad9ddf 100644
> --- a/drivers/bluetooth/hci_h4.c
> +++ b/drivers/bluetooth/hci_h4.c
> @@ -109,9 +109,6 @@ static int h4_recv(struct hci_uart *hu, const void *data, int count)
>  {
>         struct h4_struct *h4 = hu->priv;
>
> -       if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
> -               return -EUNATCH;
> -
>         h4->rx_skb = h4_recv_buf(hu, h4->rx_skb, data, count,
>                                  h4_recv_pkts, ARRAY_SIZE(h4_recv_pkts));
>         if (IS_ERR(h4->rx_skb)) {
>
> ---

There is some interesting comments on:

https://sashiko.dev/#/patchset/20260320-hci-init-fix-v1-1-e1960a41baf2%40axis.com

It seems the issues pointed out there are unrelated to this change,
but I guess it is worth double checking just in case.

-- 
Luiz Augusto von Dentz

Re: [PATCH] Bluetooth: hci_h4: Fix race during initialization

[Jonathan Rissanen]

Hello Luiz,

On 3/20/26 21:04, Luiz Augusto von Dentz wrote:
> [You don't often get email from luiz.dentz@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Hi Jonathan,
> 
> On Fri, Mar 20, 2026 at 8:10 AM Jonathan Rissanen
> <jonathan.rissanen@axis.com> wrote:
>>
>> Commit 5df5dafc171b ("Bluetooth: hci_uart: Fix another race during
>> initialization") fixed a race for hci commands sent during initialization.
>> However, there is still a race that happens if an hci event from one of
>> these commands is received before HCI_UART_REGISTERED has been set at
>> the end of hci_uart_register_dev(). The event will be ignored which
>> causes the command to fail with a timeout in the log:
>>
>> "Bluetooth: hci0: command 0x1003 tx timeout"
>>
>> This is because the hci event receive path (hci_uart_tty_receive ->
>> h4_recv) requires HCI_UART_REGISTERED to be set in h4_recv(), while the
>> hci command transmit path (hci_uart_send_frame -> h4_enqueue) only
>> requires HCI_UART_PROTO_INIT to be set in hci_uart_send_frame().
>>
>> The check for HCI_UART_REGISTERED was originally added in commit
>> c2578202919a ("Bluetooth: Fix H4 crash from incoming UART packets")
>> to fix a crash caused by hu->hdev being null dereferenced. That can no
>> longer happen: once HCI_UART_PROTO_INIT is set in hci_uart_register_dev()
>> all pointers (hu, hu->priv and hu->hdev) are valid, and
>> hci_uart_tty_receive() already calls h4_recv() on HCI_UART_PROTO_INIT
>> or HCI_UART_PROTO_READY.
>>
>> Remove the check for HCI_UART_REGISTERED in h4_recv() to fix the race
>> condition.
>>
>> Signed-off-by: Jonathan Rissanen <jonathan.rissanen@axis.com>
>> ---
>>   drivers/bluetooth/hci_h4.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_h4.c b/drivers/bluetooth/hci_h4.c
>> index ec017df8572c..1e9e2cad9ddf 100644
>> --- a/drivers/bluetooth/hci_h4.c
>> +++ b/drivers/bluetooth/hci_h4.c
>> @@ -109,9 +109,6 @@ static int h4_recv(struct hci_uart *hu, const void *data, int count)
>>   {
>>          struct h4_struct *h4 = hu->priv;
>>
>> -       if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
>> -               return -EUNATCH;
>> -
>>          h4->rx_skb = h4_recv_buf(hu, h4->rx_skb, data, count,
>>                                   h4_recv_pkts, ARRAY_SIZE(h4_recv_pkts));
>>          if (IS_ERR(h4->rx_skb)) {
>>
>> ---
> 
> There is some interesting comments on:
> 
> https://sashiko.dev/#/patchset/20260320-hci-init-fix-v1-1-e1960a41baf2%40axis.com

I see. Yeah there seem to be some valid comments:

> If hci_register_dev() fails, the error path calls hu->proto->close(hu)
> which frees the protocol-private data in hu->priv and sets it to NULL.
> However, the error path fails to clear the HCI_UART_PROTO_INIT flag.

I think regardless of this patch it makes sense to clear 
HCI_UART_PROTO_INIT if hci_register_dev() fails, since we're no longer 
in the initializing state. It becomes more important with this patch 
since it will lead to a null pointer dereference if h4_recv() is called 
after hci_register_dev() fails.

> Additionally, since hci_uart_register_dev() is called without holding the
> write-side of hu->proto_lock, can concurrent incoming UART data during
> the failure window trigger a use-after-free while hu->priv is actively
> being freed by h4_close()?

I believe adding a write lock and clearing the bit would solve these issues:

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 2b28515de92c..5455990ab211 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -692,6 +692,9 @@ static int hci_uart_register_dev(struct hci_uart *hu)

  	if (hci_register_dev(hdev) < 0) {
  		BT_ERR("Can't register HCI device");
+		percpu_down_write(&hu->proto_lock);
+		clear_bit(HCI_UART_PROTO_INIT, &hu->flags);
+		percpu_up_write(&hu->proto_lock);
  		hu->proto->close(hu);
  		hu->hdev = NULL;
  		hci_free_dev(hdev);


> It seems the issues pointed out there are unrelated to this change,
> but I guess it is worth double checking just in case.

I think it's best to fix it before applying this change as it can cause 
null pointer dereference in error path. I can send a new patchset with 
the fix.


-- 
Best regards, Jonathan

Re: [PATCH] Bluetooth: hci_h4: Fix race during initialization

[Luiz Augusto von Dentz]

Hi Jonathan,

On Tue, Mar 24, 2026 at 6:04 AM Jonathan Rissanen
<jonathan.rissanen@axis.com> wrote:
>
> Hello Luiz,
>
> On 3/20/26 21:04, Luiz Augusto von Dentz wrote:
> > [You don't often get email from luiz.dentz@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Hi Jonathan,
> >
> > On Fri, Mar 20, 2026 at 8:10 AM Jonathan Rissanen
> > <jonathan.rissanen@axis.com> wrote:
> >>
> >> Commit 5df5dafc171b ("Bluetooth: hci_uart: Fix another race during
> >> initialization") fixed a race for hci commands sent during initialization.
> >> However, there is still a race that happens if an hci event from one of
> >> these commands is received before HCI_UART_REGISTERED has been set at
> >> the end of hci_uart_register_dev(). The event will be ignored which
> >> causes the command to fail with a timeout in the log:
> >>
> >> "Bluetooth: hci0: command 0x1003 tx timeout"
> >>
> >> This is because the hci event receive path (hci_uart_tty_receive ->
> >> h4_recv) requires HCI_UART_REGISTERED to be set in h4_recv(), while the
> >> hci command transmit path (hci_uart_send_frame -> h4_enqueue) only
> >> requires HCI_UART_PROTO_INIT to be set in hci_uart_send_frame().
> >>
> >> The check for HCI_UART_REGISTERED was originally added in commit
> >> c2578202919a ("Bluetooth: Fix H4 crash from incoming UART packets")
> >> to fix a crash caused by hu->hdev being null dereferenced. That can no
> >> longer happen: once HCI_UART_PROTO_INIT is set in hci_uart_register_dev()
> >> all pointers (hu, hu->priv and hu->hdev) are valid, and
> >> hci_uart_tty_receive() already calls h4_recv() on HCI_UART_PROTO_INIT
> >> or HCI_UART_PROTO_READY.
> >>
> >> Remove the check for HCI_UART_REGISTERED in h4_recv() to fix the race
> >> condition.
> >>
> >> Signed-off-by: Jonathan Rissanen <jonathan.rissanen@axis.com>
> >> ---
> >>   drivers/bluetooth/hci_h4.c | 3 ---
> >>   1 file changed, 3 deletions(-)
> >>
> >> diff --git a/drivers/bluetooth/hci_h4.c b/drivers/bluetooth/hci_h4.c
> >> index ec017df8572c..1e9e2cad9ddf 100644
> >> --- a/drivers/bluetooth/hci_h4.c
> >> +++ b/drivers/bluetooth/hci_h4.c
> >> @@ -109,9 +109,6 @@ static int h4_recv(struct hci_uart *hu, const void *data, int count)
> >>   {
> >>          struct h4_struct *h4 = hu->priv;
> >>
> >> -       if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
> >> -               return -EUNATCH;
> >> -
> >>          h4->rx_skb = h4_recv_buf(hu, h4->rx_skb, data, count,
> >>                                   h4_recv_pkts, ARRAY_SIZE(h4_recv_pkts));
> >>          if (IS_ERR(h4->rx_skb)) {
> >>
> >> ---
> >
> > There is some interesting comments on:
> >
> > https://sashiko.dev/#/patchset/20260320-hci-init-fix-v1-1-e1960a41baf2%40axis.com
>
> I see. Yeah there seem to be some valid comments:
>
> > If hci_register_dev() fails, the error path calls hu->proto->close(hu)
> > which frees the protocol-private data in hu->priv and sets it to NULL.
> > However, the error path fails to clear the HCI_UART_PROTO_INIT flag.
>
> I think regardless of this patch it makes sense to clear
> HCI_UART_PROTO_INIT if hci_register_dev() fails, since we're no longer
> in the initializing state. It becomes more important with this patch
> since it will lead to a null pointer dereference if h4_recv() is called
> after hci_register_dev() fails.
>
> > Additionally, since hci_uart_register_dev() is called without holding the
> > write-side of hu->proto_lock, can concurrent incoming UART data during
> > the failure window trigger a use-after-free while hu->priv is actively
> > being freed by h4_close()?
>
> I believe adding a write lock and clearing the bit would solve these issues:
>
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 2b28515de92c..5455990ab211 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -692,6 +692,9 @@ static int hci_uart_register_dev(struct hci_uart *hu)
>
>         if (hci_register_dev(hdev) < 0) {
>                 BT_ERR("Can't register HCI device");
> +               percpu_down_write(&hu->proto_lock);
> +               clear_bit(HCI_UART_PROTO_INIT, &hu->flags);
> +               percpu_up_write(&hu->proto_lock);
>                 hu->proto->close(hu);
>                 hu->hdev = NULL;
>                 hci_free_dev(hdev);

LGTM

>
>
> > It seems the issues pointed out there are unrelated to this change,
> > but I guess it is worth double checking just in case.
>
> I think it's best to fix it before applying this change as it can cause
> null pointer dereference in error path. I can send a new patchset with
> the fix.

Please do.

>
> --
> Best regards, Jonathan



-- 
Luiz Augusto von Dentz

Re: [PATCH] Bluetooth: hci_h4: Fix race during initialization

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Fri, 20 Mar 2026 13:09:49 +0100 you wrote:
> Commit 5df5dafc171b ("Bluetooth: hci_uart: Fix another race during
> initialization") fixed a race for hci commands sent during initialization.
> However, there is still a race that happens if an hci event from one of
> these commands is received before HCI_UART_REGISTERED has been set at
> the end of hci_uart_register_dev(). The event will be ignored which
> causes the command to fail with a timeout in the log:
> 
> [...]

Here is the summary with links:
  - Bluetooth: hci_h4: Fix race during initialization
    https://git.kernel.org/bluetooth/bluetooth-next/c/42ee54fcd11e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html