> -----Original Message-----
> From: Pkshih <pkshih@xxxxxxxxxxx>
> Sent: Monday, November 15, 2021 3:34 PM
> To: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Cc: linux-wireless@xxxxxxxxxxxxxxx
> Subject: RE: [bug report] rtw89: add Realtek 802.11ax driver
>
> > -----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.
>
I have sent a patch [1] to fix this.
[1] https://lore.kernel.org/linux-wireless/20211119055729.12826-1-pkshih@xxxxxxxxxxx/T/#u
--
Ping-Ke
