> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Sent: Monday, November 8, 2021 10:47 PM
> To: Pkshih <pkshih@xxxxxxxxxxx>
> Cc: linux-wireless@xxxxxxxxxxxxxxx
> Subject: [bug report] rtw89: add Realtek 802.11ax driver
>
> Hello Ping-Ke Shih,
>
> The patch e3ec7017f6a2: "rtw89: add Realtek 802.11ax driver" from Oct
> 11, 2021, leads to the following Smatch static checker warning:
>
> drivers/net/wireless/realtek/rtw89/mac.c:586 hfc_ch_ctrl()
> warn: array off by one? 'cfg[ch]'
>
> drivers/net/wireless/realtek/rtw89/mac.c
> 568 static int hfc_ch_ctrl(struct rtw89_dev *rtwdev, u8 ch)
> 569 {
> 570 struct rtw89_hfc_param *param = &rtwdev->mac.hfc_param;
> 571 const struct rtw89_hfc_ch_cfg *cfg = param->ch_cfg;
> 572 int ret = 0;
> 573 u32 val = 0;
> 574
> 575 ret = rtw89_mac_check_mac_en(rtwdev, RTW89_MAC_0, RTW89_DMAC_SEL);
> 576 if (ret)
> 577 return ret;
> 578
> 579 ret = hfc_ch_cfg_chk(rtwdev, ch);
> 580 if (ret)
> 581 return ret;
> 582
> 583 if (ch > RTW89_DMA_B1HI)
> ^^^^^^^^^^^^^^^^^^^
>
> 584 return -EINVAL;
> 585
> --> 586 val = u32_encode_bits(cfg[ch].min, B_AX_MIN_PG_MASK) |
> 587 u32_encode_bits(cfg[ch].max, B_AX_MAX_PG_MASK) |
> 588 (cfg[ch].grp ? B_AX_GRP : 0);
>
> This is an unpublished Smatch warning which uses an "assume every >
> comparison should be >= wrong until proven otherwise" approach. In
> this case, Smatch can't tell the size of the cfg[] array.
I'm not clear how to fix this warning. Any suggestion?
The caller of this function is
for (ch = RTW89_DMA_ACH0; ch < RTW89_DMA_H2C; ch++) {
ret = hfc_ch_ctrl(rtwdev, ch);
...
}
So, 'ch' is between 0 and 11. Maybe, this checking can be removed?
>
> Manually it looks like cfg can only be rtw8852a_hfc_chcfg_pcie[] which
> has RTW89_DMA_CH_NUM (13) elements. Is there a reason why we don't use
> the last element?
We don't use the last element, because channel 0 to 11 are the regular TX channels
that will transmit packets to air. The channel 12 (H2C) is a special channel we
send firmware commands from driver, so it doesn't need additional configurations.
More, the list of R_AX_ACH0_PAGE_CTRL series don't have one for channel 12 neither.
> Also it's puzzling that the last element is
> FWCMDQ instead of H2C. So maybe that's the reason?
FWCMDQ is a queue or channel that is used to send H2C from driver to firmware.
So, FWCMD and H2C can be seen as synonyms, as well as FWCMDQ and H2CQ.
>
> 589 rtw89_write32(rtwdev, R_AX_ACH0_PAGE_CTRL + ch * 4, val);
> 590
> 591 return 0;
> 592 }
>
--
Ping-Ke
