[PATCH] wifi: rtlwifi: add array bounds check in rtl92d_dm_rxgain_tracking_thermalmeter()

[PATCH] wifi: rtlwifi: add array bounds check in rtl92d_dm_rxgain_tracking_thermalmeter()

[Dmitry Antipov]

In 'rtl92d_dm_rxgain_tracking_thermalmeter()', add an extra 'index_mapping'
array bounds check and use the convenient 'array_index_nospec()' to protect
against spectre. Compile tested only.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.c
index 20373ce998bf..44f1d3b40d22 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.c
@@ -139,6 +139,9 @@ static void rtl92d_dm_rxgain_tracking_thermalmeter(struct ieee80211_hw *hw)
 	u32 u4tmp;
 
 	idx = rtlpriv->efuse.eeprom_thermalmeter - rtlpriv->dm.thermalvalue_rxgain;
+	if (WARN_ON_ONCE(idx < 0 || idx >= ARRAY_SIZE(index_mapping)))
+		return;
+	idx = array_index_nospec(idx, ARRAY_SIZE(index_mapping));
 	u4tmp = index_mapping[idx] << 12;
 
 	rtl_dbg(rtlpriv, COMP_POWER_TRACKING, DBG_LOUD,
-- 
2.49.0

RE: [PATCH] wifi: rtlwifi: add array bounds check in rtl92d_dm_rxgain_tracking_thermalmeter()

[Ping-Ke Shih]

Dmitry Antipov <dmantipov@yandex.ru> wrote:
> In 'rtl92d_dm_rxgain_tracking_thermalmeter()', add an extra 'index_mapping'
> array bounds check and use the convenient 'array_index_nospec()' to protect
> against spectre. Compile tested only.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.c
> b/drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.c
> index 20373ce998bf..44f1d3b40d22 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.c
> @@ -139,6 +139,9 @@ static void rtl92d_dm_rxgain_tracking_thermalmeter(struct ieee80211_hw *hw)
>         u32 u4tmp;
> 
>         idx = rtlpriv->efuse.eeprom_thermalmeter - rtlpriv->dm.thermalvalue_rxgain;
> +       if (WARN_ON_ONCE(idx < 0 || idx >= ARRAY_SIZE(index_mapping)))
> +               return;

By original flow, we can ensure rtlpriv->dm.thermalvalue_rxgain must be smaller
or equal to rtlpriv->efuse.eeprom_thermalmeter, so 'idx < 0' is not possible. 

Maybe we can clamp the range within [0, RX_INDEX_MAPPING_NUM - 1] when
assigning value to rtlpriv->dm.thermalvalue_rxgain. 

> +       idx = array_index_nospec(idx, ARRAY_SIZE(index_mapping));

Does this overkill? I'm not very clear when we should use this to prevent CPU
speculates, but if this is really needed, the statements will be added
everywhere. 

>         u4tmp = index_mapping[idx] << 12;
> 
>         rtl_dbg(rtlpriv, COMP_POWER_TRACKING, DBG_LOUD,
> --
> 2.49.0
>