linux-kernel - [PATCH 06/13] staging: wfx: improve protection against malformed HIF messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed,  1 Jul 2020 17:07:00 +0200
From:   Jerome Pouiller <Jerome.Pouiller@...abs.com>
To:     devel@...verdev.osuosl.org, linux-wireless@...r.kernel.org
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jérôme Pouiller 
        <jerome.pouiller@...abs.com>
Subject: [PATCH 06/13] staging: wfx: improve protection against malformed HIF messages

From: Jérôme Pouiller <jerome.pouiller@...abs.com>

As discussed here[1], if a message was smaller than the size of the
message header, it could be incorrectly processed.

[1] https://lore.kernel.org/driverdev-devel/2302785.6C7ODC2LYm@pc-42/

Signed-off-by: Jérôme Pouiller <jerome.pouiller@...abs.com>
---
 drivers/staging/wfx/bh.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 1cbaf8bb4fa38..53ae0b5abcdd8 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -57,7 +57,6 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 	int release_count;
 	int piggyback = 0;
 
-	WARN(read_len < 4, "corrupted read");
 	WARN(read_len > round_down(0xFFF, 2) * sizeof(u16),
 	     "%s: request exceed WFx capability", __func__);
 
@@ -76,7 +75,27 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 	hif = (struct hif_msg *)skb->data;
 	WARN(hif->encrypted & 0x1, "unsupported encryption type");
 	if (hif->encrypted == 0x2) {
-		if (wfx_sl_decode(wdev, (void *)hif)) {
+		if (WARN(read_len < sizeof(struct hif_sl_msg), "corrupted read"))
+			goto err;
+		computed_len = le16_to_cpu(((struct hif_sl_msg *)hif)->len);
+		computed_len = round_up(computed_len - sizeof(u16), 16);
+		computed_len += sizeof(struct hif_sl_msg);
+		computed_len += sizeof(struct hif_sl_tag);
+	} else {
+		if (WARN(read_len < sizeof(struct hif_msg), "corrupted read"))
+			goto err;
+		computed_len = le16_to_cpu(hif->len);
+		computed_len = round_up(computed_len, 2);
+	}
+	if (computed_len != read_len) {
+		dev_err(wdev->dev, "inconsistent message length: %zu != %zu\n",
+			computed_len, read_len);
+		print_hex_dump(KERN_INFO, "hif: ", DUMP_PREFIX_OFFSET, 16, 1,
+			       hif, read_len, true);
+		goto err;
+	}
+	if (hif->encrypted == 0x2) {
+		if (wfx_sl_decode(wdev, (struct hif_sl_msg *)hif)) {
 			dev_kfree_skb(skb);
 			// If frame was a confirmation, expect trouble in next
 			// exchange. However, it is harmless to fail to decode
@@ -84,19 +103,6 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 			// piggyback is probably correct.
 			return piggyback;
 		}
-		computed_len =
-			round_up(le16_to_cpu(hif->len) - sizeof(hif->len), 16) +
-			sizeof(struct hif_sl_msg) +
-			sizeof(struct hif_sl_tag);
-	} else {
-		computed_len = round_up(le16_to_cpu(hif->len), 2);
-	}
-	if (computed_len != read_len) {
-		dev_err(wdev->dev, "inconsistent message length: %zu != %zu\n",
-			computed_len, read_len);
-		print_hex_dump(KERN_INFO, "hif: ", DUMP_PREFIX_OFFSET, 16, 1,
-			       hif, read_len, true);
-		goto err;
 	}
 
 	if (!(hif->id & HIF_ID_IS_INDICATION)) {
-- 
2.27.0