> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Sent: Friday, November 12, 2021 3:52 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/fw.c:1383 rtw89_fw_h2c_rf_reg()
> error: buffer overflow 'info->rtw89_phy_config_rf_h2c' 3 <= 3
>
> drivers/net/wireless/realtek/rtw89/phy.c
> 662 static int rtw89_phy_config_rf_reg_fw(struct rtw89_dev *rtwdev,
> 663 struct rtw89_fw_h2c_rf_reg_info *info)
> 664 {
> 665 u16 page = info->curr_idx / RTW89_H2C_RF_PAGE_SIZE;
> 666 u16 len = (info->curr_idx % RTW89_H2C_RF_PAGE_SIZE) * 4;
> 667 u8 i;
> 668 int ret = 0;
> 669
> 670 if (page > RTW89_H2C_RF_PAGE_NUM) {
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Lets assume "page == RTW89_H2C_RF_PAGE_NUM.
>
> 671 rtw89_warn(rtwdev,
> 672 "rf reg h2c total page num %d larger than %d (RTW89_H2C_RF_PAGE_NUM)\n",
> 673 page, RTW89_H2C_RF_PAGE_NUM);
> 674 return -EINVAL;
> 675 }
> 676
> 677 for (i = 0; i < page; i++) {
> ^^^^^^^^^
>
> 678 ret = rtw89_fw_h2c_rf_reg(rtwdev, info,
> 679 RTW89_H2C_RF_PAGE_SIZE * 4, i);
> 680 if (ret)
> 681 return ret;
> 682 }
> 683 ret = rtw89_fw_h2c_rf_reg(rtwdev, info, len, i);
> ^
> So "i" is now RTW89_H2C_RF_PAGE_NUM and it leads to off by one out of
> bounds error.
>
> 684 if (ret)
> 685 return ret;
> 686 info->curr_idx = 0;
> 687
> 688 return 0;
> 689 }
>
When the info->curr_idx is between RTW89_H2C_RF_PAGE_SIZE * RTW89_H2C_RF_PAGE_NUM and
RTW89_H2C_RF_PAGE_SIZE * (RTW89_H2C_RF_PAGE_NUM + 1), it falls into the case
you mentioned. Fortunately, the input is predictable if we don't change
RF parameters, so it doesn't lead wrong situation for now.
Anyway, I will refine this function to handle all cases. Thanks for
your finding.
--
Ping-Ke
