linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] rtw89: add Realtek 802.11ax driver
@ 2021-11-12  7:51 Dan Carpenter
  2021-11-15  7:33 ` Pkshih
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2021-11-12  7:51 UTC (permalink / raw)
  To: pkshih; +Cc: linux-wireless

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  }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [bug report] rtw89: add Realtek 802.11ax driver
  2021-11-12  7:51 [bug report] rtw89: add Realtek 802.11ax driver Dan Carpenter
@ 2021-11-15  7:33 ` Pkshih
  2021-11-19  6:01   ` Pkshih
  0 siblings, 1 reply; 10+ messages in thread
From: Pkshih @ 2021-11-15  7:33 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-wireless


> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Sent: Friday, November 12, 2021 3:52 PM
> To: Pkshih <pkshih@realtek.com>
> Cc: linux-wireless@vger.kernel.org
> 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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [bug report] rtw89: add Realtek 802.11ax driver
  2021-11-15  7:33 ` Pkshih
@ 2021-11-19  6:01   ` Pkshih
  0 siblings, 0 replies; 10+ messages in thread
From: Pkshih @ 2021-11-19  6:01 UTC (permalink / raw)
  To: Pkshih, Dan Carpenter; +Cc: linux-wireless


> -----Original Message-----
> From: Pkshih <pkshih@realtek.com>
> Sent: Monday, November 15, 2021 3:34 PM
> To: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: linux-wireless@vger.kernel.org
> Subject: RE: [bug report] rtw89: add Realtek 802.11ax driver
> 
> > -----Original Message-----
> > From: Dan Carpenter <dan.carpenter@oracle.com>
> > Sent: Friday, November 12, 2021 3:52 PM
> > To: Pkshih <pkshih@realtek.com>
> > Cc: linux-wireless@vger.kernel.org
> > 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@realtek.com/T/#u

--
Ping-Ke


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [bug report] rtw89: add Realtek 802.11ax driver
  2022-12-13  0:46 ` Ping-Ke Shih
@ 2022-12-13  4:21   ` Dan Carpenter
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2022-12-13  4:21 UTC (permalink / raw)
  To: Ping-Ke Shih; +Cc: linux-wireless

On Tue, Dec 13, 2022 at 12:46:22AM +0000, Ping-Ke Shih wrote:
> 
> 
> > -----Original Message-----
> > From: Dan Carpenter <error27@gmail.com>
> > Sent: Monday, December 12, 2022 11:34 PM
> > To: Ping-Ke Shih <pkshih@realtek.com>
> > Cc: linux-wireless@vger.kernel.org
> > 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 potential issue (just from reading
> > the code):
> > 
> > drivers/net/wireless/realtek/rtw89/core.h
> >     3878 static inline u32
> >     3879 rtw89_read32_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask)
> >     3880 {
> >     3881         u32 shift = __ffs(mask);
> >     3882         u32 orig;
> >     3883         u32 ret;
> >     3884
> >     3885         orig = rtw89_read32(rtwdev, addr);
> > --> 3886         ret = (orig & mask) >> shift;
> > 
> > I think this line should be:
> > 
> > 	ret = (orig & mask) >> (shift - 1);
> > 
> > A typical mask here is 0xff so __ffs() is 1 because the first bit is
> > set.  This code will do: ret = (orig & 0xff) >> 1;  I think it should be
> > ret = (orig & 0xff) >> 0;
> > 
> > If the mask was 0xff00 I would expect >> 8 instead of >> 9 etc.
> 
> ffs(0xff)=1 and __ffs(0xff)=0, so I think original is correct.

Oh, wow.  You're right.  I hadn't realized it worked like that.

Sorry!

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [bug report] rtw89: add Realtek 802.11ax driver
  2022-12-12 15:33 Dan Carpenter
@ 2022-12-13  0:46 ` Ping-Ke Shih
  2022-12-13  4:21   ` Dan Carpenter
  0 siblings, 1 reply; 10+ messages in thread
From: Ping-Ke Shih @ 2022-12-13  0:46 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-wireless



> -----Original Message-----
> From: Dan Carpenter <error27@gmail.com>
> Sent: Monday, December 12, 2022 11:34 PM
> To: Ping-Ke Shih <pkshih@realtek.com>
> Cc: linux-wireless@vger.kernel.org
> 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 potential issue (just from reading
> the code):
> 
> drivers/net/wireless/realtek/rtw89/core.h
>     3878 static inline u32
>     3879 rtw89_read32_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask)
>     3880 {
>     3881         u32 shift = __ffs(mask);
>     3882         u32 orig;
>     3883         u32 ret;
>     3884
>     3885         orig = rtw89_read32(rtwdev, addr);
> --> 3886         ret = (orig & mask) >> shift;
> 
> I think this line should be:
> 
> 	ret = (orig & mask) >> (shift - 1);
> 
> A typical mask here is 0xff so __ffs() is 1 because the first bit is
> set.  This code will do: ret = (orig & 0xff) >> 1;  I think it should be
> ret = (orig & 0xff) >> 0;
> 
> If the mask was 0xff00 I would expect >> 8 instead of >> 9 etc.

ffs(0xff)=1 and __ffs(0xff)=0, so I think original is correct.

Ping-Ke


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug report] rtw89: add Realtek 802.11ax driver
@ 2022-12-12 15:33 Dan Carpenter
  2022-12-13  0:46 ` Ping-Ke Shih
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2022-12-12 15:33 UTC (permalink / raw)
  To: pkshih; +Cc: linux-wireless

Hello Ping-Ke Shih,

The patch e3ec7017f6a2: "rtw89: add Realtek 802.11ax driver" from Oct
11, 2021, leads to the following potential issue (just from reading
the code):

drivers/net/wireless/realtek/rtw89/core.h
    3878 static inline u32
    3879 rtw89_read32_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask)
    3880 {
    3881         u32 shift = __ffs(mask);
    3882         u32 orig;
    3883         u32 ret;
    3884 
    3885         orig = rtw89_read32(rtwdev, addr);
--> 3886         ret = (orig & mask) >> shift;

I think this line should be:

	ret = (orig & mask) >> (shift - 1);

A typical mask here is 0xff so __ffs() is 1 because the first bit is
set.  This code will do: ret = (orig & 0xff) >> 1;  I think it should be
ret = (orig & 0xff) >> 0;

If the mask was 0xff00 I would expect >> 8 instead of >> 9 etc.

    3887 
    3888         return ret;
    3889 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [bug report] rtw89: add Realtek 802.11ax driver
  2021-11-30 10:42 Dan Carpenter
@ 2021-12-01  1:57 ` Pkshih
  0 siblings, 0 replies; 10+ messages in thread
From: Pkshih @ 2021-12-01  1:57 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-wireless


> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Sent: Tuesday, November 30, 2021 6:43 PM
> To: Pkshih <pkshih@realtek.com>
> Cc: linux-wireless@vger.kernel.org
> 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/mac80211.c:31 rtw89_ops_tx()
> 	error: uninitialized symbol 'qsel'.
> 
> drivers/net/wireless/realtek/rtw89/mac80211.c
>     16 static void rtw89_ops_tx(struct ieee80211_hw *hw,
>     17                          struct ieee80211_tx_control *control,
>     18                          struct sk_buff *skb)
>     19 {
>     20         struct rtw89_dev *rtwdev = hw->priv;
>     21         struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
>     22         struct ieee80211_vif *vif = info->control.vif;
>     23         struct ieee80211_sta *sta = control->sta;
>     24         int ret, qsel;
>     25
>     26         ret = rtw89_core_tx_write(rtwdev, vif, sta, skb, &qsel);
>     27         if (ret) {
>     28                 rtw89_err(rtwdev, "failed to transmit skb: %d\n", ret);
>     29                 ieee80211_free_txskb(hw, skb);
> 
> Missing return; here?

Yes. Thanks for the finding.
I will send a patch to fix it.

--
Ping-Ke


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug report] rtw89: add Realtek 802.11ax driver
@ 2021-11-30 10:42 Dan Carpenter
  2021-12-01  1:57 ` Pkshih
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2021-11-30 10:42 UTC (permalink / raw)
  To: pkshih; +Cc: linux-wireless

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/mac80211.c:31 rtw89_ops_tx()
	error: uninitialized symbol 'qsel'.

drivers/net/wireless/realtek/rtw89/mac80211.c
    16 static void rtw89_ops_tx(struct ieee80211_hw *hw,
    17                          struct ieee80211_tx_control *control,
    18                          struct sk_buff *skb)
    19 {
    20         struct rtw89_dev *rtwdev = hw->priv;
    21         struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
    22         struct ieee80211_vif *vif = info->control.vif;
    23         struct ieee80211_sta *sta = control->sta;
    24         int ret, qsel;
    25 
    26         ret = rtw89_core_tx_write(rtwdev, vif, sta, skb, &qsel);
    27         if (ret) {
    28                 rtw89_err(rtwdev, "failed to transmit skb: %d\n", ret);
    29                 ieee80211_free_txskb(hw, skb);

Missing return; here?

    30         }
--> 31         rtw89_core_tx_kick_off(rtwdev, qsel);
    32 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [bug report] rtw89: add Realtek 802.11ax driver
  2021-11-08 14:47 Dan Carpenter
@ 2021-11-09  1:04 ` Pkshih
  0 siblings, 0 replies; 10+ messages in thread
From: Pkshih @ 2021-11-09  1:04 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-wireless


> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Sent: Monday, November 8, 2021 10:47 PM
> To: Pkshih <pkshih@realtek.com>
> Cc: linux-wireless@vger.kernel.org
> 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug report] rtw89: add Realtek 802.11ax driver
@ 2021-11-08 14:47 Dan Carpenter
  2021-11-09  1:04 ` Pkshih
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2021-11-08 14:47 UTC (permalink / raw)
  To: pkshih; +Cc: linux-wireless

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.

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?  Also it's puzzling that the last element is
FWCMDQ instead of H2C.  So maybe that's the reason?

    589         rtw89_write32(rtwdev, R_AX_ACH0_PAGE_CTRL + ch * 4, val);
    590 
    591         return 0;
    592 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-12-13  4:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-12  7:51 [bug report] rtw89: add Realtek 802.11ax driver Dan Carpenter
2021-11-15  7:33 ` Pkshih
2021-11-19  6:01   ` Pkshih
  -- strict thread matches above, loose matches on Subject: below --
2022-12-12 15:33 Dan Carpenter
2022-12-13  0:46 ` Ping-Ke Shih
2022-12-13  4:21   ` Dan Carpenter
2021-11-30 10:42 Dan Carpenter
2021-12-01  1:57 ` Pkshih
2021-11-08 14:47 Dan Carpenter
2021-11-09  1:04 ` Pkshih

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).