| Message ID | 20181013064608.31952-1-luca@coelho.fi (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3d71c3f1f50cf309bd20659422af549bc784bfff |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | iwlwifi: mvm: check return value of rs_rate_from_ucode_rate() | expand |
On Sat, 2018-10-13 at 09:46 +0300, Luca Coelho wrote: > From: Luca Coelho <luciano.coelho@intel.com> > > 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: stable@vger.kernel.org > Signed-off-by: Luca Coelho <luciano.coelho@intel.com> > --- 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.
Luca Coelho <luca@coelho.fi> wrote: > From: Luca Coelho <luciano.coelho@intel.com> > > 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: stable@vger.kernel.org > Signed-off-by: Luca Coelho <luciano.coelho@intel.com> Patch applied to wireless-drivers-next.git, thanks. 3d71c3f1f50c iwlwifi: mvm: check return value of rs_rate_from_ucode_rate()
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;