Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Soenke Huster <soenke.huster@eknoes.de>
To:     Marcel Holtmann <marcel@holtmann.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     linux-bluetooth@vger.kernel.org,
        Soenke Huster <soenke.huster@eknoes.de>
Subject: [PATCH] Bluetooth: Fix null pointer deref on unexpected status event
Date:   Fri, 22 Jul 2022 13:53:07 +0200
Message-Id: <20220722115307.243056-1-soenke.huster@eknoes.de>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

__hci_cmd_sync returns NULL if the controller responds with a status
event. This is unexpected for the commands sent here, but on
occurrence leads to null pointer dereferences and thus must be
handled.

Signed-off-by: Soenke Huster <soenke.huster@eknoes.de>
---
 net/bluetooth/aosp.c | 15 ++++++++++++---
 net/bluetooth/msft.c | 15 ++++++++++++---
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/aosp.c b/net/bluetooth/aosp.c
index 432ae3aac9e3..1d67836e95e1 100644
--- a/net/bluetooth/aosp.c
+++ b/net/bluetooth/aosp.c
@@ -54,7 +54,10 @@ void aosp_do_open(struct hci_dev *hdev)
 	/* LE Get Vendor Capabilities Command */
 	skb = __hci_cmd_sync(hdev, hci_opcode_pack(0x3f, 0x153), 0, NULL,
 			     HCI_CMD_TIMEOUT);
-	if (IS_ERR(skb)) {
+	if (IS_ERR_OR_NULL(skb)) {
+		if (!skb)
+			skb = ERR_PTR(-EIO);
+
 		bt_dev_err(hdev, "AOSP get vendor capabilities (%ld)",
 			   PTR_ERR(skb));
 		return;
@@ -152,7 +155,10 @@ static int enable_quality_report(struct hci_dev *hdev)
 
 	skb = __hci_cmd_sync(hdev, BQR_OPCODE, sizeof(cp), &cp,
 			     HCI_CMD_TIMEOUT);
-	if (IS_ERR(skb)) {
+	if (IS_ERR_OR_NULL(skb)) {
+		if (!skb)
+			skb = ERR_PTR(-EIO);
+
 		bt_dev_err(hdev, "Enabling Android BQR failed (%ld)",
 			   PTR_ERR(skb));
 		return PTR_ERR(skb);
@@ -171,7 +177,10 @@ static int disable_quality_report(struct hci_dev *hdev)
 
 	skb = __hci_cmd_sync(hdev, BQR_OPCODE, sizeof(cp), &cp,
 			     HCI_CMD_TIMEOUT);
-	if (IS_ERR(skb)) {
+	if (IS_ERR_OR_NULL(skb)) {
+		if (!skb)
+			skb = ERR_PTR(-EIO);
+
 		bt_dev_err(hdev, "Disabling Android BQR failed (%ld)",
 			   PTR_ERR(skb));
 		return PTR_ERR(skb);
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index 14975769f678..bee6a4c656be 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -120,7 +120,10 @@ static bool read_supported_features(struct hci_dev *hdev,
 
 	skb = __hci_cmd_sync(hdev, hdev->msft_opcode, sizeof(cp), &cp,
 			     HCI_CMD_TIMEOUT);
-	if (IS_ERR(skb)) {
+	if (IS_ERR_OR_NULL(skb)) {
+		if (!skb)
+			skb = ERR_PTR(-EIO);
+
 		bt_dev_err(hdev, "Failed to read MSFT supported features (%ld)",
 			   PTR_ERR(skb));
 		return false;
@@ -319,8 +322,11 @@ static int msft_remove_monitor_sync(struct hci_dev *hdev,
 
 	skb = __hci_cmd_sync(hdev, hdev->msft_opcode, sizeof(cp), &cp,
 			     HCI_CMD_TIMEOUT);
-	if (IS_ERR(skb))
+	if (IS_ERR_OR_NULL(skb)) {
+		if (!skb)
+			return -EIO;
 		return PTR_ERR(skb);
+	}
 
 	return msft_le_cancel_monitor_advertisement_cb(hdev, hdev->msft_opcode,
 						       monitor, skb);
@@ -432,8 +438,11 @@ static int msft_add_monitor_sync(struct hci_dev *hdev,
 			     HCI_CMD_TIMEOUT);
 	kfree(cp);
 
-	if (IS_ERR(skb))
+	if (IS_ERR_OR_NULL(skb)) {
+		if (!skb)
+			return -EIO;
 		return PTR_ERR(skb);
+	}
 
 	return msft_le_monitor_advertisement_cb(hdev, hdev->msft_opcode,
 						monitor, skb);
-- 
2.37.1