[PATCH 3/3] Bluetooth: btintel_pcie: Fix bounds checks in RX completion handler

ZhaoJinming <[email protected]>
Newsgroups org.kernel.vger.linux-bluetooth,org.kernel.vger.linux-kernel
Message-ID <20260820-btintel_pcie_bounds_fixes-v1-3-c9dcd1ac8bf6@uniontech.com>
Fix three issues in btintel_pcie_msix_rx_handle():

1. cr_tia is a device-controlled value from shared DMA memory
   (data->ia.cr_tia[]) and is used to index rxq->urbd1s[] without
   a bounds check. An out-of-range value could cause an out-of-bounds
   access when indexing rxq->urbd1s[]. Add a bounds check before the
   array access. When cr_tia is out of range, reset the ring consumer
   pointer (data->ia.cr_tia[]) to cr_hia so the queue can recover on
   the next interrupt.

2. urbd1->frbd_tag is a 16-bit device-controlled field (0-65535)
   used directly as an index into rxq->bufs[] (64 elements). Add a
   bounds check. Read the field via READ_ONCE() to avoid a
   Time-of-Check to Time-of-Use (TOCTOU) race, since the field is
   in DMA-coherent memory and the compiler may emit two separate
   reads.

3. All error paths in the while loop use 'return', which exits the
   handler without advancing cr_tia. This causes the RX completion
   queue to stall, as the next interrupt would process the same
   corrupted descriptor and exit again. Change to 'break' to exit
   the loop without further processing.

Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe transport")
Signed-off-by: ZhaoJinming <[email protected]>
---
 drivers/bluetooth/btintel_pcie.c | 44 ++++++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index c1fd5feb9f81bbd70fabd12b12eb4a1708f6a91f..1f5537df620972d937f65e754701f65566e4655a 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -1586,7 +1586,7 @@ static int btintel_pcie_submit_rx_work(struct btintel_pcie_data *data, u8 status
 /* Handles the MSI-X interrupt for rx queue 1 which is for RX */
 static void btintel_pcie_msix_rx_handle(struct btintel_pcie_data *data)
 {
-	u16 cr_hia, cr_tia;
+	u16 cr_hia, cr_tia, frbd_tag;
 	struct rxq *rxq;
 	struct urbd1 *urbd1;
 	struct data_buf *buf;
@@ -1608,21 +1608,53 @@ 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 >= %u, contact device vendor",
+				   cr_tia, rxq->count);
+			/* Reset consumer pointer so the ring can
+			 * recover on the next interrupt.
+			 */
+			data->ia.cr_tia[BTINTEL_PCIE_RXQ_NUM] = cr_hia;
+			break;
+		}
+
 		urbd1 = &rxq->urbd1s[cr_tia];
 		ipc_print_urbd1(data->hdev, urbd1, cr_tia);
 
-		buf = &rxq->bufs[urbd1->frbd_tag];
+		/* frbd_tag is a bitfield in DMA-coherent memory;
+		 * read the full word once with READ_ONCE to avoid
+		 * TOCTOU race with the device.
+		 */
+		frbd_tag = READ_ONCE(*(const u32 *)urbd1) & 0xffff;
+
+		if (frbd_tag >= rxq->count) {
+			bt_dev_err(hdev, "RXQ: invalid frbd_tag %u >= %u, contact device vendor",
+				   frbd_tag, rxq->count);
+			/* Device provided invalid data. Leave cr_tia
+			 * unchanged so the error remains detectable
+			 * via repeated log messages, aiding debug.
+			 */
+			break;
+		}
+
+		buf = &rxq->bufs[frbd_tag];
 		if (!buf) {
-			bt_dev_err(hdev, "RXQ: failed to get the DMA buffer for %d",
-				   urbd1->frbd_tag);
-			return;
+			bt_dev_err(hdev, "RXQ: failed to get the DMA buffer for %u",
+				   frbd_tag);
+			/* Unexpected NULL pointer; leave cr_tia
+			 * unchanged to keep the error visible.
+			 */
+			break;
 		}
 
 		ret = btintel_pcie_submit_rx_work(data, urbd1->status,
 						  buf->data);
 		if (ret) {
 			bt_dev_err(hdev, "RXQ: failed to submit rx request");
-			return;
+			/* Submission failed; leave cr_tia unchanged
+			 * to keep the error detectable on retry.
+			 */
+			break;
 		}
 
 		cr_tia = (cr_tia + 1) % rxq->count;

-- 
2.51.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.