UbiquitousOS
/
OpenCloudOS-Kernel
mirror of https://gitee.com/OpenCloudOS/OpenCloudOS-Kernel.git
11
0
Fork 0
Code Issues Projects Releases Wiki Activity

mwifiex: drop most magic numbers from mwifiex_process_tdls_action_frame() 

Before commit 1e58252e33 ("mwifiex: Fix heap overflow in
mmwifiex_process_tdls_action_frame()"),
mwifiex_process_tdls_action_frame() already had too many magic numbers.
But this commit just added a ton more, in the name of checking for
buffer overflows. That seems like a really bad idea.

Let's make these magic numbers a little less magic, by
(a) factoring out 'pos[1]' as 'ie_len'
(b) using 'sizeof' on the appropriate source or destination fields where
    possible, instead of bare numbers
(c) dropping redundant checks, per below.

Regarding redundant checks: the beginning of the loop has this:

                if (pos + 2 + pos[1] > end)
                        break;

but then individual 'case's include stuff like this:

 			if (pos > end - 3)
 				return;
 			if (pos[1] != 1)
				return;

Note that the second 'return' (validating the length, pos[1]) combined
with the above condition (ensuring 'pos + 2 + length' doesn't exceed
'end'), makes the first 'return' (whose 'if' can be reworded as 'pos >
end - pos[1] - 2') redundant. Rather than unwind the magic numbers
there, just drop those conditions.

Fixes: 1e58252e33 ("mwifiex: Fix heap overflow in mmwifiex_process_tdls_action_frame()")
Signed-off-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
This commit is contained in:
Brian Norris 2019-12-06 11:45:35 -08:00 committed by Kalle Valo
parent fafa7424ba
commit 70e5b8f445
1 changed files with 28 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
drivers/net/wireless/marvell/mwifiex/tdls.c
View File

@ -894,7 +894,7 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv,
u8 *peer, *pos, *end;
u8 i, action, basic;
u16 cap = 0;
int ie_len = 0;
int ies_len = 0;
if (len < (sizeof(struct ethhdr) + 3))
return;
@ -916,7 +916,7 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv,
pos = buf + sizeof(struct ethhdr) + 4;
/* payload 1+ category 1 + action 1 + dialog 1 */
cap = get_unaligned_le16(pos);
ie_len = len - sizeof(struct ethhdr) - TDLS_REQ_FIX_LEN;
ies_len = len - sizeof(struct ethhdr) - TDLS_REQ_FIX_LEN;
pos += 2;
break;
@ -926,7 +926,7 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv,
/* payload 1+ category 1 + action 1 + dialog 1 + status code 2*/
pos = buf + sizeof(struct ethhdr) + 6;
cap = get_unaligned_le16(pos);
ie_len = len - sizeof(struct ethhdr) - TDLS_RESP_FIX_LEN;
ies_len = len - sizeof(struct ethhdr) - TDLS_RESP_FIX_LEN;
pos += 2;
break;
@ -934,7 +934,7 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv,
if (len < (sizeof(struct ethhdr) + TDLS_CONFIRM_FIX_LEN))
return;
pos = buf + sizeof(struct ethhdr) + TDLS_CONFIRM_FIX_LEN;
ie_len = len - sizeof(struct ethhdr) - TDLS_CONFIRM_FIX_LEN;
ies_len = len - sizeof(struct ethhdr) - TDLS_CONFIRM_FIX_LEN;
break;
default:
mwifiex_dbg(priv->adapter, ERROR, "Unknown TDLS frame type.\n");
@ -947,33 +947,33 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv,
sta_ptr->tdls_cap.capab = cpu_to_le16(cap);
for (end = pos + ie_len; pos + 1 < end; pos += 2 + pos[1]) {
if (pos + 2 + pos[1] > end)
for (end = pos + ies_len; pos + 1 < end; pos += 2 + pos[1]) {
u8 ie_len = pos[1];
if (pos + 2 + ie_len > end)
break;
switch (*pos) {
case WLAN_EID_SUPP_RATES:
if (pos[1] > 32)
if (ie_len > sizeof(sta_ptr->tdls_cap.rates))
return;
sta_ptr->tdls_cap.rates_len = pos[1];
for (i = 0; i < pos[1]; i++)
sta_ptr->tdls_cap.rates_len = ie_len;
for (i = 0; i < ie_len; i++)
sta_ptr->tdls_cap.rates[i] = pos[i + 2];
break;
case WLAN_EID_EXT_SUPP_RATES:
if (pos[1] > 32)
if (ie_len > sizeof(sta_ptr->tdls_cap.rates))
return;
basic = sta_ptr->tdls_cap.rates_len;
if (pos[1] > 32 - basic)
if (ie_len > sizeof(sta_ptr->tdls_cap.rates) - basic)
return;
for (i = 0; i < pos[1]; i++)
for (i = 0; i < ie_len; i++)
sta_ptr->tdls_cap.rates[basic + i] = pos[i + 2];
sta_ptr->tdls_cap.rates_len += pos[1];
sta_ptr->tdls_cap.rates_len += ie_len;
break;
case WLAN_EID_HT_CAPABILITY:
if (pos > end - sizeof(struct ieee80211_ht_cap) - 2)
return;
if (pos[1] != sizeof(struct ieee80211_ht_cap))
if (ie_len != sizeof(struct ieee80211_ht_cap))
return;
/* copy the ie's value into ht_capb*/
memcpy((u8 *)&sta_ptr->tdls_cap.ht_capb, pos + 2,
@ -981,59 +981,45 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv,
sta_ptr->is_11n_enabled = 1;
break;
case WLAN_EID_HT_OPERATION:
if (pos > end -
sizeof(struct ieee80211_ht_operation) - 2)
return;
if (pos[1] != sizeof(struct ieee80211_ht_operation))
if (ie_len != sizeof(struct ieee80211_ht_operation))
return;
/* copy the ie's value into ht_oper*/
memcpy(&sta_ptr->tdls_cap.ht_oper, pos + 2,
sizeof(struct ieee80211_ht_operation));
break;
case WLAN_EID_BSS_COEX_2040:
if (pos > end - 3)
return;
if (pos[1] != 1)
if (ie_len != sizeof(pos[2]))
return;
sta_ptr->tdls_cap.coex_2040 = pos[2];
break;
case WLAN_EID_EXT_CAPABILITY:
if (pos > end - sizeof(struct ieee_types_header))
if (ie_len < sizeof(struct ieee_types_header))
return;
if (pos[1] < sizeof(struct ieee_types_header))
return;
if (pos[1] > 8)
if (ie_len > 8)
return;
memcpy((u8 *)&sta_ptr->tdls_cap.extcap, pos,
sizeof(struct ieee_types_header) +
min_t(u8, pos[1], 8));
min_t(u8, ie_len, 8));
break;
case WLAN_EID_RSN:
if (pos > end - sizeof(struct ieee_types_header))
if (ie_len < sizeof(struct ieee_types_header))
return;
if (pos[1] < sizeof(struct ieee_types_header))
return;
if (pos[1] > IEEE_MAX_IE_SIZE -
if (ie_len > IEEE_MAX_IE_SIZE -
sizeof(struct ieee_types_header))
return;
memcpy((u8 *)&sta_ptr->tdls_cap.rsn_ie, pos,
sizeof(struct ieee_types_header) +
min_t(u8, pos[1], IEEE_MAX_IE_SIZE -
min_t(u8, ie_len, IEEE_MAX_IE_SIZE -
sizeof(struct ieee_types_header)));
break;
case WLAN_EID_QOS_CAPA:
if (pos > end - 3)
return;
if (pos[1] != 1)
if (ie_len != sizeof(pos[2]))
return;
sta_ptr->tdls_cap.qos_info = pos[2];
break;
case WLAN_EID_VHT_OPERATION:
if (priv->adapter->is_hw_11ac_capable) {
if (pos > end -
sizeof(struct ieee80211_vht_operation) - 2)
return;
if (pos[1] !=
if (ie_len !=
sizeof(struct ieee80211_vht_operation))
return;
/* copy the ie's value into vhtoper*/
@ -1043,10 +1029,7 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv,
break;
case WLAN_EID_VHT_CAPABILITY:
if (priv->adapter->is_hw_11ac_capable) {
if (pos > end -
sizeof(struct ieee80211_vht_cap) - 2)
return;
if (pos[1] != sizeof(struct ieee80211_vht_cap))
if (ie_len != sizeof(struct ieee80211_vht_cap))
return;
/* copy the ie's value into vhtcap*/
memcpy((u8 *)&sta_ptr->tdls_cap.vhtcap, pos + 2,
@ -1056,9 +1039,7 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv,
break;
case WLAN_EID_AID:
if (priv->adapter->is_hw_11ac_capable) {
if (pos > end - 4)
return;
if (pos[1] != 2)
if (ie_len != sizeof(u16))
return;
sta_ptr->tdls_cap.aid =
get_unaligned_le16((pos + 2));