[PATCH] Bluetooth: btintel_pcie: Fix array bounds check bugs

[PATCH] Bluetooth: btintel_pcie: Fix array bounds check bugs

[ZhaoJinming]

Fix four array bounds issues in the Intel BT PCIe driver:

1. btintel_pcie_send_sync(): bounds check for tfd_index uses '>' instead
   of '>='.  When tfd_index == txq->count (32), the check passes and
   btintel_pcie_prepare_tx() writes past the end of txq->tfds[] and
   txq->bufs[].

2. btintel_pcie_submit_rx(): same off-by-one on frbd_index.  When
   frbd_index == rxq->count (64), the check passes and
   btintel_pcie_prepare_rx() writes past the end of rxq->frbds[] and
   rxq->bufs[].

3. btintel_pcie_msix_tx_handle(): cr_tia (device-controlled, from
   shared DMA memory) is used to index txq->urbd0s[] before any bounds
   check, and the urbd0->tfd_index check uses '>' instead of '>='.

4. btintel_pcie_msix_rx_handle(): cr_tia (device-controlled) indexes
   rxq->urbd1s[] with no bounds check.  urbd1->frbd_tag is a 16-bit
   device-controlled field (0-65535) used directly as an index into
   rxq->bufs[] (64 elements).

Fix all four by correcting the comparison operators and adding explicit
bounds checks on device-controlled indices before array access.

Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe transport")
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/bluetooth/btintel_pcie.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 2b7231be5973..32cfa0f5af1c 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -401,7 +401,7 @@ static int btintel_pcie_send_sync(struct btintel_pcie_data *data,
 
 	tfd_index = data->ia.tr_hia[BTINTEL_PCIE_TXQ_NUM];
 
-	if (tfd_index > txq->count)
+	if (tfd_index >= txq->count)
 		return -ERANGE;
 
 	/* Firmware raises alive interrupt on HCI_OP_RESET or
@@ -502,7 +502,7 @@ static int btintel_pcie_submit_rx(struct btintel_pcie_data *data)
 
 	frbd_index = data->ia.tr_hia[BTINTEL_PCIE_RXQ_NUM];
 
-	if (frbd_index > rxq->count)
+	if (frbd_index >= rxq->count)
 		return -ERANGE;
 
 	/* Prepare for RX submit. It updates the FRBD with the address of DMA
@@ -1094,12 +1094,15 @@ static void btintel_pcie_msix_tx_handle(struct btintel_pcie_data *data)
 	txq = &data->txq;
 
 	while (cr_tia != cr_hia) {
+		if (cr_tia >= txq->count)
+			return;
+
 		data->tx_wait_done = true;
 		wake_up(&data->tx_wait_q);
 
 		urbd0 = &txq->urbd0s[cr_tia];
 
-		if (urbd0->tfd_index > txq->count)
+		if (urbd0->tfd_index >= txq->count)
 			return;
 
 		cr_tia = (cr_tia + 1) % txq->count;
@@ -1584,9 +1587,21 @@ static void btintel_pcie_msix_rx_handle(struct btintel_pcie_data *data)
 	 * process all received CDs in this interrupt.
 	 */
 	while (cr_tia != cr_hia) {
+		if (cr_tia >= rxq->count) {
+			bt_dev_err(hdev, "RXQ: invalid cr_tia %u (count %u)",
+				   cr_tia, rxq->count);
+			return;
+		}
+
 		urbd1 = &rxq->urbd1s[cr_tia];
 		ipc_print_urbd1(data->hdev, urbd1, cr_tia);
 
+		if (urbd1->frbd_tag >= rxq->count) {
+			bt_dev_err(hdev, "RXQ: invalid frbd_tag %u (count %u)",
+				   urbd1->frbd_tag, rxq->count);
+			return;
+		}
+
 		buf = &rxq->bufs[urbd1->frbd_tag];
 		if (!buf) {
 			bt_dev_err(hdev, "RXQ: failed to get the DMA buffer for %d",

base-commit: 3d6d817622b0a9721e3cc404df3469171582be13
-- 
2.51.0

RE: Bluetooth: btintel_pcie: Fix array bounds check bugs

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1181 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=1145362

---Test result---

Test Summary:
CheckPatch                    PASS      0.58 seconds
VerifyFixes                   PASS      0.07 seconds
VerifySignedoff               PASS      0.07 seconds
GitLint                       PASS      0.33 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      27.07 seconds
CheckAllWarning               PASS      30.27 seconds
CheckSparse                   PASS      28.26 seconds
BuildKernel32                 PASS      26.37 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      499.32 seconds
IncrementalBuild              PASS      26.11 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


https://github.com/bluez/bluetooth-next/pull/576

---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: btintel_pcie: Fix array bounds check bugs

[Paul Menzel]

Dear Zhao,


Thank you for your patch. For the summary I’d use:

Bluetooth: btintel_pcie: Check array bounds of dev controlled indices

Am 13.08.26 um 12:37 schrieb ZhaoJinming:
> Fix four array bounds issues in the Intel BT PCIe driver:
> 
> 1. btintel_pcie_send_sync(): bounds check for tfd_index uses '>' instead
>     of '>='.  When tfd_index == txq->count (32), the check passes and
>     btintel_pcie_prepare_tx() writes past the end of txq->tfds[] and
>     txq->bufs[].
> 
> 2. btintel_pcie_submit_rx(): same off-by-one on frbd_index.  When
>     frbd_index == rxq->count (64), the check passes and
>     btintel_pcie_prepare_rx() writes past the end of rxq->frbds[] and
>     rxq->bufs[].
> 
> 3. btintel_pcie_msix_tx_handle(): cr_tia (device-controlled, from
>     shared DMA memory) is used to index txq->urbd0s[] before any bounds
>     check, and the urbd0->tfd_index check uses '>' instead of '>='.
> 
> 4. btintel_pcie_msix_rx_handle(): cr_tia (device-controlled) indexes
>     rxq->urbd1s[] with no bounds check.  urbd1->frbd_tag is a 16-bit
>     device-controlled field (0-65535) used directly as an index into
>     rxq->bufs[] (64 elements).

Enumerating things in the commit message, is a good indicator to split 
the commit into smaller ones.

> Fix all four by correcting the comparison operators and adding explicit
> bounds checks on device-controlled indices before array access.
> 
> Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe transport")
> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
> ---
>   drivers/bluetooth/btintel_pcie.c | 21 ++++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 2b7231be5973..32cfa0f5af1c 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -401,7 +401,7 @@ static int btintel_pcie_send_sync(struct btintel_pcie_data *data,
>   
>   	tfd_index = data->ia.tr_hia[BTINTEL_PCIE_TXQ_NUM];
>   
> -	if (tfd_index > txq->count)
> +	if (tfd_index >= txq->count)
>   		return -ERANGE;
>   
>   	/* Firmware raises alive interrupt on HCI_OP_RESET or
> @@ -502,7 +502,7 @@ static int btintel_pcie_submit_rx(struct btintel_pcie_data *data)
>   
>   	frbd_index = data->ia.tr_hia[BTINTEL_PCIE_RXQ_NUM];
>   
> -	if (frbd_index > rxq->count)
> +	if (frbd_index >= rxq->count)
>   		return -ERANGE;
>   
>   	/* Prepare for RX submit. It updates the FRBD with the address of DMA
> @@ -1094,12 +1094,15 @@ static void btintel_pcie_msix_tx_handle(struct btintel_pcie_data *data)
>   	txq = &data->txq;
>   
>   	while (cr_tia != cr_hia) {
> +		if (cr_tia >= txq->count)
> +			return;
> +
>   		data->tx_wait_done = true;
>   		wake_up(&data->tx_wait_q);
>   
>   		urbd0 = &txq->urbd0s[cr_tia];
>   
> -		if (urbd0->tfd_index > txq->count)
> +		if (urbd0->tfd_index >= txq->count)
>   			return;

gemini/gemini-3.1-pro-preview comments [1]:

> If urbd0->tfd_index is invalid and we return early here, doesn't this permanently
> stall the TX completion queue?
> By returning early, cr_tia is never advanced. The next interrupt will process
> the exact same corrupted descriptor and return early again.



>   
>   		cr_tia = (cr_tia + 1) % txq->count;
> @@ -1584,9 +1587,21 @@ static void btintel_pcie_msix_rx_handle(struct btintel_pcie_data *data)
>   	 * process all received CDs in this interrupt.
>   	 */
>   	while (cr_tia != cr_hia) {
> +		if (cr_tia >= rxq->count) {
> +			bt_dev_err(hdev, "RXQ: invalid cr_tia %u (count %u)",

Maybe: RXQ: invalid cr_tia %u >= %u, contact device vendor

> +				   cr_tia, rxq->count);
> +			return;
> +		}
> +
>   		urbd1 = &rxq->urbd1s[cr_tia];
>   		ipc_print_urbd1(data->hdev, urbd1, cr_tia);
>   
> +		if (urbd1->frbd_tag >= rxq->count) {
> +			bt_dev_err(hdev, "RXQ: invalid frbd_tag %u (count %u)",

Ditto regarding the log message.

> +				   urbd1->frbd_tag, rxq->count);
> +			return;
> +		}
> +

gemini/gemini-3.1-pro-preview comments [1]:

> By returning early when urbd1->frbd_tag is invalid, we fail to advance cr_tia.
> Will this permanently stall the RX completion queue since the next interrupt
> will process the same corrupted descriptor?

>   		buf = &rxq->bufs[urbd1->frbd_tag];

gemini/gemini-3.1-pro-preview comments [1]:

> Does this introduce a Time-of-Check to Time-of-Use (TOCTOU) vulnerability?
> urbd1->frbd_tag is a bitfield inside a DMA-coherent structure. The code accesses
> it once for the bounds check (above) and again here to index the array.
> Can the compiler emit two separate memory reads, allowing the device to alter
> the value after the check passes but before the array access?

>   		if (!buf) {
>   			bt_dev_err(hdev, "RXQ: failed to get the DMA buffer for %d",


Kind regards,

Paul


[1]: 
https://sashiko.dev/#/patchset/460316663D6D34ED%2B20260813103734.222955-1-zhaojinming%40uniontech.com

Re: [PATCH] Bluetooth: btintel_pcie: Fix array bounds check bugs

[赵金明]

Thank you for your review.

I split the fixes into 3 patches in a new thread as suggested.
Changes in v1:
- Fixed off-by-one comparisons in the synchronous paths
- Added bounds checks for cr_tia and descriptor fields in TX/RX handlers
- Used READ_ONCE() for DMA-coherent bitfield reads to avoid TOCTOU
- Changed return to break to avoid repeated processing of corrupted descriptors

https://lore.kernel.org/all/20260820-btintel_pcie_bounds_fixes-v1-0-c9dcd1ac8bf6@uniontech.com/

Thanks,
Zhao Jinming



>Dear Zhao,



>



>



>Thank you for your patch. For the summary I’d use:



>



>Bluetooth: btintel_pcie: Check array bounds of dev controlled indices



>



>Am 13.08.26 um 12:37 schrieb ZhaoJinming:



>> Fix four array bounds issues in the Intel BT PCIe driver:



>> 



>> 1. btintel_pcie_send_sync(): bounds check for tfd_index uses '>' instead



>>???? of '>='.? When tfd_index == txq->count (32), the check passes and



>>???? btintel_pcie_prepare_tx() writes past the end of txq->tfds[] and



>>???? txq->bufs[].



>> 



>> 2. btintel_pcie_submit_rx(): same off-by-one on frbd_index.? When



>>???? frbd_index == rxq->count (64), the check passes and



>>???? btintel_pcie_prepare_rx() writes past the end of rxq->frbds[] and



>>???? rxq->bufs[].



>> 



>> 3. btintel_pcie_msix_tx_handle(): cr_tia (device-controlled, from



>>???? shared DMA memory) is used to index txq->urbd0s[] before any bounds



>>???? check, and the urbd0->tfd_index check uses '>' instead of '>='.



>> 



>> 4. btintel_pcie_msix_rx_handle(): cr_tia (device-controlled) indexes



>>???? rxq->urbd1s[] with no bounds check.? urbd1->frbd_tag is a 16-bit



>>???? device-controlled field (0-65535) used directly as an index into



>>???? rxq->bufs[] (64 elements).



>



>Enumerating things in the commit message, is a good indicator to split 



>the commit into smaller ones.



>



>> Fix all four by correcting the comparison operators and adding explicit



>> bounds checks on device-controlled indices before array access.



>> 



>> Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe transport")



>> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>



>> ---



>>?? drivers/bluetooth/btintel_pcie.c | 21 ++++++++++++++++++---



>>?? 1 file changed, 18 insertions(+), 3 deletions(-)



>> 



>> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c



>> index 2b7231be5973..32cfa0f5af1c 100644



>> --- a/drivers/bluetooth/btintel_pcie.c



>> +++ b/drivers/bluetooth/btintel_pcie.c



>> @@ -401,7 +401,7 @@ static int btintel_pcie_send_sync(struct btintel_pcie_data *data,



>>?? 



>>?? 	tfd_index = data->ia.tr_hia[BTINTEL_PCIE_TXQ_NUM];



>>?? 



>> -	if (tfd_index > txq->count)



>> +	if (tfd_index >= txq->count)



>>?? 		return -ERANGE;



>>?? 



>>?? 	/* Firmware raises alive interrupt on HCI_OP_RESET or



>> @@ -502,7 +502,7 @@ static int btintel_pcie_submit_rx(struct btintel_pcie_data *data)



>>?? 



>>?? 	frbd_index = data->ia.tr_hia[BTINTEL_PCIE_RXQ_NUM];



>>?? 



>> -	if (frbd_index > rxq->count)



>> +	if (frbd_index >= rxq->count)



>>?? 		return -ERANGE;



>>?? 



>>?? 	/* Prepare for RX submit. It updates the FRBD with the address of DMA



>> @@ -1094,12 +1094,15 @@ static void btintel_pcie_msix_tx_handle(struct btintel_pcie_data *data)



>>?? 	txq = &data->txq;



>>?? 



>>?? 	while (cr_tia != cr_hia) {



>> +		if (cr_tia >= txq->count)



>> +			return;



>> +



>>?? 		data->tx_wait_done = true;



>>?? 		wake_up(&data->tx_wait_q);



>>?? 



>>?? 		urbd0 = &txq->urbd0s[cr_tia];



>>?? 



>> -		if (urbd0->tfd_index > txq->count)



>> +		if (urbd0->tfd_index >= txq->count)



>>?? 			return;



>



>gemini/gemini-3.1-pro-preview comments [1]:



>



>> If urbd0->tfd_index is invalid and we return early here, doesn't this permanently



>> stall the TX completion queue?



>> By returning early, cr_tia is never advanced. The next interrupt will process



>> the exact same corrupted descriptor and return early again.



>



>



>



>>?? 



>>?? 		cr_tia = (cr_tia + 1) % txq->count;



>> @@ -1584,9 +1587,21 @@ static void btintel_pcie_msix_rx_handle(struct btintel_pcie_data *data)



>>?? 	 * process all received CDs in this interrupt.



>>?? 	 */



>>?? 	while (cr_tia != cr_hia) {



>> +		if (cr_tia >= rxq->count) {



>> +			bt_dev_err(hdev, "RXQ: invalid cr_tia %u (count %u)",



>



>Maybe: RXQ: invalid cr_tia %u >= %u, contact device vendor



>



>> +				?? cr_tia, rxq->count);



>> +			return;



>> +		}



>> +



>>?? 		urbd1 = &rxq->urbd1s[cr_tia];



>>?? 		ipc_print_urbd1(data->hdev, urbd1, cr_tia);



>>?? 



>> +		if (urbd1->frbd_tag >= rxq->count) {



>> +			bt_dev_err(hdev, "RXQ: invalid frbd_tag %u (count %u)",



>



>Ditto regarding the log message.



>



>> +				?? urbd1->frbd_tag, rxq->count);



>> +			return;



>> +		}



>> +



>



>gemini/gemini-3.1-pro-preview comments [1]:



>



>> By returning early when urbd1->frbd_tag is invalid, we fail to advance cr_tia.



>> Will this permanently stall the RX completion queue since the next interrupt



>> will process the same corrupted descriptor?



>



>>?? 		buf = &rxq->bufs[urbd1->frbd_tag];



>



>gemini/gemini-3.1-pro-preview comments [1]:



>



>> Does this introduce a Time-of-Check to Time-of-Use (TOCTOU) vulnerability?



>> urbd1->frbd_tag is a bitfield inside a DMA-coherent structure. The code accesses



>> it once for the bounds check (above) and again here to index the array.



>> Can the compiler emit two separate memory reads, allowing the device to alter



>> the value after the check passes but before the array access?



>



>>?? 		if (!buf) {



>>?? 			bt_dev_err(hdev, "RXQ: failed to get the DMA buffer for %d",



>



>



>Kind regards,



>



>Paul



>



>



>[1]: 



>https://sashiko.dev/#/patchset/460316663D6D34ED%2B20260813103734.222955-1-zhaojinming%40uniontech.com



>