[PATCH 5/5] Bluetooth: hci_qca: validate controller memdump frames

[Li Qiang]

The QCA controller memdump worker reads frame and dump-size headers
before validating the received skb length. The event classifier likewise
accesses its fixed header without ensuring that all accessed bytes are
present. Malformed controller data can therefore cause out-of-bounds
reads.

Validate every header and the initial dump size before changing the dump
state. Reset the local dump state when discarding an unstarted
collection. Use subtraction-based capacity checks to prevent overflow
while accounting for received and synthesized dump data.

Signed-off-by: Li Qiang <liqiang01@xxxxxxxxxx>
---
 drivers/bluetooth/hci_qca.c | 58 ++++++++++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index b2d1ee3a3d11..b5d4b8e0e376 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1057,6 +1057,34 @@ static void qca_controller_memdump(struct work_struct *work)
 			return;
 		}
 
+		if (skb->len < sizeof(*cmd_hdr)) {
+			bt_dev_err(hu->hdev, "Rx short memdump header");
+			kfree_skb(skb);
+			mutex_unlock(&qca->hci_memdump_lock);
+			continue;
+		}
+
+		cmd_hdr = (void *) skb->data;
+		seq_no = __le16_to_cpu(cmd_hdr->seq_no);
+		skb_pull(skb, sizeof(struct qca_memdump_event_hdr));
+
+		if (!seq_no) {
+			if (skb->len < sizeof(*dump)) {
+				bt_dev_err(hu->hdev, "Rx short memdump size");
+				kfree_skb(skb);
+				mutex_unlock(&qca->hci_memdump_lock);
+				continue;
+			}
+
+			dump = (void *)skb->data;
+			if (!__le32_to_cpu(dump->dump_size)) {
+				bt_dev_err(hu->hdev, "Rx invalid memdump size");
+				kfree_skb(skb);
+				mutex_unlock(&qca->hci_memdump_lock);
+				continue;
+			}
+		}
+
 		if (!qca_memdump) {
 			qca_memdump = kzalloc_obj(*qca_memdump, GFP_ATOMIC);
 			if (!qca_memdump) {
@@ -1068,9 +1096,6 @@ static void qca_controller_memdump(struct work_struct *work)
 		}
 
 		qca->memdump_state = QCA_MEMDUMP_COLLECTING;
-		cmd_hdr = (void *) skb->data;
-		seq_no = __le16_to_cpu(cmd_hdr->seq_no);
-		skb_pull(skb, sizeof(struct qca_memdump_event_hdr));
 
 		if (!seq_no) {
 
@@ -1082,15 +1107,7 @@ static void qca_controller_memdump(struct work_struct *work)
 			 */
 			set_bit(QCA_IBS_DISABLED, &qca->flags);
 			set_bit(QCA_MEMDUMP_COLLECTION, &qca->flags);
-			dump = (void *) skb->data;
 			qca_memdump->ram_dump_size = __le32_to_cpu(dump->dump_size);
-			if (!(qca_memdump->ram_dump_size)) {
-				bt_dev_err(hu->hdev, "Rx invalid memdump size");
-				kfree(qca_memdump);
-				kfree_skb(skb);
-				mutex_unlock(&qca->hci_memdump_lock);
-				return;
-			}
 
 			queue_delayed_work(qca->workqueue,
 					   &qca->ctrl_memdump_timeout,
@@ -1123,6 +1140,8 @@ static void qca_controller_memdump(struct work_struct *work)
 		if (!test_bit(QCA_MEMDUMP_COLLECTION, &qca->flags)) {
 			bt_dev_err(hu->hdev, "QCA: Discarding other packets");
 			kfree(qca_memdump);
+			qca->qca_memdump = NULL;
+			qca->memdump_state = QCA_MEMDUMP_IDLE;
 			kfree_skb(skb);
 			mutex_unlock(&qca->hci_memdump_lock);
 			return;
@@ -1140,9 +1159,11 @@ static void qca_controller_memdump(struct work_struct *work)
 			seq_no != QCA_LAST_SEQUENCE_NUM) {
 			bt_dev_err(hu->hdev, "QCA controller missed packet:%d",
 				   qca_memdump->current_seq_no);
-			rx_size = qca_memdump->received_dump;
-			rx_size += QCA_DUMP_PACKET_SIZE;
-			if (rx_size > qca_memdump->ram_dump_size) {
+			if (qca_memdump->received_dump >
+				    qca_memdump->ram_dump_size ||
+			    QCA_DUMP_PACKET_SIZE >
+				    qca_memdump->ram_dump_size -
+				    qca_memdump->received_dump) {
 				bt_dev_err(hu->hdev,
 					   "QCA memdump received %d, no space for missed packet",
 					   qca_memdump->received_dump);
@@ -1154,8 +1175,10 @@ static void qca_controller_memdump(struct work_struct *work)
 			qca_memdump->current_seq_no++;
 		}
 
-		rx_size = qca_memdump->received_dump  + skb->len;
-		if (rx_size <= qca_memdump->ram_dump_size) {
+		if (qca_memdump->received_dump <= qca_memdump->ram_dump_size &&
+		    skb->len <= qca_memdump->ram_dump_size -
+				qca_memdump->received_dump) {
+			rx_size = qca_memdump->received_dump + skb->len;
 			if ((seq_no != QCA_LAST_SEQUENCE_NUM) &&
 			    (seq_no != qca_memdump->current_seq_no)) {
 				bt_dev_err(hu->hdev,
@@ -1235,7 +1258,8 @@ static int qca_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
 	 * received we store dump into a file before closing hci. This
 	 * dump will help in triaging the issues.
 	 */
-	if ((skb->data[0] == HCI_VENDOR_PKT) &&
+	if (skb->len >= offsetof(struct qca_memdump_event_hdr, seq_no) &&
+	    (skb->data[0] == HCI_VENDOR_PKT) &&
 	    (get_unaligned_be16(skb->data + 2) == QCA_SSR_DUMP_HANDLE))
 		return qca_controller_memdump_event(hdev, skb);
 
-- 
2.43.0