[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: Bluetooth: hci_h4: Fix race during initialization

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2833 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1069873

---Test result---

Test Summary:
CheckPatch                    PENDING   0.28 seconds
GitLint                       PENDING   0.22 seconds
SubjectPrefix                 PASS      0.11 seconds
BuildKernel                   PASS      26.60 seconds
CheckAllWarning               PASS      29.28 seconds
CheckSparse                   PASS      27.58 seconds
BuildKernel32                 PASS      25.94 seconds
TestRunnerSetup               PASS      569.53 seconds
TestRunner_l2cap-tester       PASS      28.30 seconds
TestRunner_iso-tester         FAIL      36.61 seconds
TestRunner_bnep-tester        PASS      6.29 seconds
TestRunner_mgmt-tester        FAIL      115.04 seconds
TestRunner_rfcomm-tester      PASS      9.29 seconds
TestRunner_sco-tester         FAIL      14.40 seconds
TestRunner_ioctl-tester       PASS      10.10 seconds
TestRunner_mesh-tester        FAIL      12.55 seconds
TestRunner_smp-tester         PASS      8.65 seconds
TestRunner_userchan-tester    PASS      8.98 seconds
IncrementalBuild              PENDING   0.67 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
BUG: KASAN: slab-use-after-free in le_read_features_complete+0x7e/0x2b0
Total: 141, Passed: 141 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 494, Passed: 489 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.108 seconds
##############################
Test: TestRunner_sco-tester - FAIL
Desc: Run sco-tester with test-runner
Output:
WARNING: possible circular locking dependency detected
BUG: sleeping function called from invalid context at net/core/sock.c:3782
Total: 30, Passed: 30 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    2.740 seconds
Mesh - Send cancel - 2                               Timed out    1.996 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

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