2018-10-13 06:46:19

by Luca Coelho

[permalink] [raw]
Subject: [PATCH] iwlwifi: mvm: check return value of rs_rate_from_ucode_rate()

From: Luca Coelho <[email protected]>

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

Cc: [email protected]
Signed-off-by: Luca Coelho <[email protected]>
---
drivers/net/wireless/intel/iwlwifi/mvm/rs.c | 24 ++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
index 2c75f51a04e4..089972280daa 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
@@ -1239,7 +1239,11 @@ void iwl_mvm_rs_tx_status(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
!(info->flags & IEEE80211_TX_STAT_AMPDU))
return;

- rs_rate_from_ucode_rate(tx_resp_hwrate, info->band, &tx_resp_rate);
+ if (rs_rate_from_ucode_rate(tx_resp_hwrate, info->band,
+ &tx_resp_rate)) {
+ WARN_ON_ONCE(1);
+ return;
+ }

#ifdef CONFIG_MAC80211_DEBUGFS
/* Disable last tx check if we are debugging with fixed rate but
@@ -1290,7 +1294,10 @@ void iwl_mvm_rs_tx_status(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
*/
table = &lq_sta->lq;
lq_hwrate = le32_to_cpu(table->rs_table[0]);
- rs_rate_from_ucode_rate(lq_hwrate, info->band, &lq_rate);
+ if (rs_rate_from_ucode_rate(lq_hwrate, info->band, &lq_rate)) {
+ WARN_ON_ONCE(1);
+ return;
+ }

/* Here we actually compare this rate to the latest LQ command */
if (lq_color != LQ_FLAG_COLOR_GET(table->flags)) {
@@ -1392,8 +1399,12 @@ void iwl_mvm_rs_tx_status(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
/* Collect data for each rate used during failed TX attempts */
for (i = 0; i <= retries; ++i) {
lq_hwrate = le32_to_cpu(table->rs_table[i]);
- rs_rate_from_ucode_rate(lq_hwrate, info->band,
- &lq_rate);
+ if (rs_rate_from_ucode_rate(lq_hwrate, info->band,
+ &lq_rate)) {
+ WARN_ON_ONCE(1);
+ return;
+ }
+
/*
* Only collect stats if retried rate is in the same RS
* table as active/search.
@@ -3260,7 +3271,10 @@ static void rs_build_rates_table_from_fixed(struct iwl_mvm *mvm,
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;
--
2.19.1



2018-10-13 06:48:37

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH] iwlwifi: mvm: check return value of rs_rate_from_ucode_rate()

On Sat, 2018-10-13 at 09:46 +0300, Luca Coelho wrote:
> From: Luca Coelho <[email protected]>
>
> 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
>
> Cc: [email protected]
> Signed-off-by: Luca Coelho <[email protected]>
> ---

Kalle,

Just for the record, as we discussed on IRC, please take this patch
directly to wireless-drivers-next for 4.20 so I don't need to send a
pull-req just for it.

--
Cheers,
Luca.


2018-10-13 11:49:36

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] iwlwifi: mvm: check return value of rs_rate_from_ucode_rate()

Luca Coelho <[email protected]> wrote:

> From: Luca Coelho <[email protected]>
>
> 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
>
> Cc: [email protected]
> Signed-off-by: Luca Coelho <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

3d71c3f1f50c iwlwifi: mvm: check return value of rs_rate_from_ucode_rate()

--
https://patchwork.kernel.org/patch/10639957/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches