| Message ID | 20220608171614.28891-1-aleksei.kodanev@bell-sw.com |
|---|---|
| State | New |
| Headers | show |
| Series | iwlegacy: 4965: fix potential off-by-one overflow in il4965_rs_fill_link_cmd() | expand |
Alexey Kodanev <aleksei.kodanev@bell-sw.com> wrote: > As a result of the execution of the inner while loop, the value > of 'idx' can be equal to LINK_QUAL_MAX_RETRY_NUM. However, this > is not checked after the loop and 'idx' is used to write the > LINK_QUAL_MAX_RETRY_NUM size array 'lq_cmd->rs_table[idx]' below > in the outer loop. > > The fix is to check the new value of 'idx' inside the nested loop, > and break both loops if index equals the size. Checking it at the > start is now pointless, so let's remove it. > > Detected using the static analysis tool - Svace. > > Fixes: be663ab67077 ("iwlwifi: split the drivers for agn and legacy devices 3945/4965") > Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com> Patch applied to wireless-next.git, thanks. a8eb8e6f7159 wifi: iwlegacy: 4965: fix potential off-by-one overflow in il4965_rs_fill_link_cmd()
diff --git a/drivers/net/wireless/intel/iwlegacy/4965-rs.c b/drivers/net/wireless/intel/iwlegacy/4965-rs.c index 9dd2d890e35f..c62f299b9e0a 100644 --- a/drivers/net/wireless/intel/iwlegacy/4965-rs.c +++ b/drivers/net/wireless/intel/iwlegacy/4965-rs.c @@ -2403,7 +2403,7 @@ il4965_rs_fill_link_cmd(struct il_priv *il, struct il_lq_sta *lq_sta, /* Repeat initial/next rate. * For legacy IL_NUMBER_TRY == 1, this loop will not execute. * For HT IL_HT_NUMBER_TRY == 3, this executes twice. */ - while (repeat_rate > 0 && idx < LINK_QUAL_MAX_RETRY_NUM) { + while (repeat_rate > 0) { if (is_legacy(tbl_type.lq_type)) { if (ant_toggle_cnt < NUM_TRY_BEFORE_ANT_TOGGLE) ant_toggle_cnt++; @@ -2422,6 +2422,8 @@ il4965_rs_fill_link_cmd(struct il_priv *il, struct il_lq_sta *lq_sta, cpu_to_le32(new_rate); repeat_rate--; idx++; + if (idx >= LINK_QUAL_MAX_RETRY_NUM) + goto out; } il4965_rs_get_tbl_info_from_mcs(new_rate, lq_sta->band, @@ -2466,6 +2468,7 @@ il4965_rs_fill_link_cmd(struct il_priv *il, struct il_lq_sta *lq_sta, repeat_rate--; } +out: lq_cmd->agg_params.agg_frame_cnt_limit = LINK_QUAL_AGG_FRAME_LIMIT_DEF; lq_cmd->agg_params.agg_dis_start_th = LINK_QUAL_AGG_DISABLE_START_DEF;
As a result of the execution of the inner while loop, the value of 'idx' can be equal to LINK_QUAL_MAX_RETRY_NUM. However, this is not checked after the loop and 'idx' is used to write the LINK_QUAL_MAX_RETRY_NUM size array 'lq_cmd->rs_table[idx]' below in the outer loop. The fix is to check the new value of 'idx' inside the nested loop, and break both loops if index equals the size. Checking it at the start is now pointless, so let's remove it. Detected using the static analysis tool - Svace. Fixes: be663ab67077 ("iwlwifi: split the drivers for agn and legacy devices 3945/4965") Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com> --- drivers/net/wireless/intel/iwlegacy/4965-rs.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)