linux-kernel - [PATCH 3.16 079/305] iwlwifi: mvm: check return value of rs_rate_from_ucode

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:   Sun, 03 Feb 2019 14:45:08 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, Denis Kirjanov <kda@...ux-powerpc.org>,
        "Luca Coelho" <luciano.coelho@...el.com>,
        "Kalle Valo" <kvalo@...eaurora.org>
Subject: [PATCH 3.16 079/305] iwlwifi: mvm: check return value of
 rs_rate_from_ucode_rate()

3.16.63-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Luca Coelho <luciano.coelho@...el.com>

commit 3d71c3f1f50cf309bd20659422af549bc784bfff upstream.

The rs_rate_from_ucode_rate() function may return -EINVAL if the rate
is invalid, but none of the callsites check for the error, potentially
making us access arrays with index IWL_RATE_INVALID, which is larger
than the arrays, causing an out-of-bounds access.  This will trigger
KASAN warnings, such as the one reported in the bugzilla issue
mentioned below.

This fixes https://bugzilla.kernel.org/show_bug.cgi?id=200659

Signed-off-by: Luca Coelho <luciano.coelho@...el.com>
Signed-off-by: Kalle Valo <kvalo@...eaurora.org>
[bwh: Backported to 3.16:
 - Fix up one additional caller
 - Adjust filename, context
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- a/drivers/net/wireless/iwlwifi/mvm/rs.c
+++ b/drivers/net/wireless/iwlwifi/mvm/rs.c
@@ -1057,7 +1057,10 @@ static void rs_tx_status(void *mvm_r, st
 	 */
 	table = &lq_sta->lq;
 	ucode_rate = le32_to_cpu(table->rs_table[0]);
-	rs_rate_from_ucode_rate(ucode_rate, info->band, &rate);
+	if (rs_rate_from_ucode_rate(ucode_rate, info->band, &rate)) {
+		WARN_ON_ONCE(1);
+		return;
+	}
 	if (info->band == IEEE80211_BAND_5GHZ)
 		rate.index -= IWL_FIRST_OFDM_RATE;
 	mac_flags = info->status.rates[0].flags;
@@ -1161,7 +1164,10 @@ static void rs_tx_status(void *mvm_r, st
 	 */
 	if (info->flags & IEEE80211_TX_STAT_AMPDU) {
 		ucode_rate = le32_to_cpu(table->rs_table[0]);
-		rs_rate_from_ucode_rate(ucode_rate, info->band, &rate);
+		if (rs_rate_from_ucode_rate(ucode_rate, info->band, &rate)) {
+			WARN_ON_ONCE(1);
+			return;
+		}
 		rs_collect_tx_data(lq_sta, curr_tbl, rate.index,
 				   info->status.ampdu_len,
 				   info->status.ampdu_ack_len,
@@ -1186,7 +1192,12 @@ static void rs_tx_status(void *mvm_r, st
 		/* Collect data for each rate used during failed TX attempts */
 		for (i = 0; i <= retries; ++i) {
 			ucode_rate = le32_to_cpu(table->rs_table[i]);
-			rs_rate_from_ucode_rate(ucode_rate, info->band, &rate);
+			if (rs_rate_from_ucode_rate(ucode_rate, info->band,
+						    &rate)) {
+				WARN_ON_ONCE(1);
+				return;
+			}
+
 			/*
 			 * Only collect stats if retried rate is in the same RS
 			 * table as active/search.
@@ -2677,7 +2688,10 @@ static void rs_build_rates_table_from_fi
 	for (i = 0; i < num_rates; i++)
 		lq_cmd->rs_table[i] = ucode_rate_le32;
 
-	rs_rate_from_ucode_rate(ucode_rate, band, &rate);
+	if (rs_rate_from_ucode_rate(ucode_rate, band, &rate)) {
+		WARN_ON_ONCE(1);
+		return;
+	}
 
 	if (is_mimo(&rate))
 		lq_cmd->mimo_delim = num_rates - 1;
@@ -2928,8 +2942,11 @@ static void rs_program_fix_rate(struct i
 
 	if (lq_sta->dbg_fixed_rate) {
 		struct rs_rate rate;
-		rs_rate_from_ucode_rate(lq_sta->dbg_fixed_rate,
-					lq_sta->band, &rate);
+		if (rs_rate_from_ucode_rate(lq_sta->dbg_fixed_rate,
+					    lq_sta->band, &rate)) {
+			WARN_ON_ONCE(1);
+			return;
+		}
 		rs_fill_lq_cmd(mvm, NULL, lq_sta, &rate);
 		iwl_mvm_send_lq_cmd(lq_sta->drv, &lq_sta->lq, false);
 	}