RE: [PATCH] wifi: rtw88: Add NULL check for chip->edcca_th

[Ping-Ke Shih <pkshih@realtek.com>]

+ Bitterblue

Panagiotis Petrakopoulos <npetrakopoulos2003@gmail.com> wrote:
> It was recently reported that rtw_fw_adaptivity_result
> in fw.c dereferences rtwdev->chip->edcca_th without
> a null check. The issue appears to be that devices
> with the 8821CE chip don't define edcca_th in their
> chip info. As a result, when rtw_fw_adaptivity_result
> tries to dereference it, the kernel triggers an oops.
> 
> Add a NULL check for edcca_th before dereferencing
> it in rtw_fw_adaptivity_result() in fw.c and
> rtw_phy_set_edcca_th() in phy.c.
> 
> Tested on a 8822CE chip which defines edcca_th, so
> this issue is not present on it, but it still uses
> this driver and I can verify there are no regressions.
> 
> Reported-by: Oleksandr Havrylov <goainwo@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221286
> Link:
> https://lore.kernel.org/linux-wireless/CALdGYqQriS7mP0vj_rm_xvisfzFVh0hbpy+---48r6bodZO7tg@mail.gm
> ail.com/
> Signed-off-by: Panagiotis Petrakopoulos <npetrakopoulos2003@gmail.com>
> ---
>  drivers/net/wireless/realtek/rtw88/fw.c  | 3 +++
>  drivers/net/wireless/realtek/rtw88/phy.c | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
> index 48207052e3f8..c4819ef6d54d 100644
> --- a/drivers/net/wireless/realtek/rtw88/fw.c
> +++ b/drivers/net/wireless/realtek/rtw88/fw.c
> @@ -284,6 +284,9 @@ static void rtw_fw_adaptivity_result(struct rtw_dev *rtwdev, u8 *payload,
>                 result->density, result->igi, result->l2h_th_init, result->l2h,
>                 result->h2l, result->option);
> 
> +       if (!edcca_th)
> +               return;
> +

As Bitterblue analysis, this might be a garbage, so I'd return at the entry
of this function.

>         rtw_dbg(rtwdev, RTW_DBG_ADAPTIVITY, "Reg Setting: L2H %x H2L %x\n",
>                 rtw_read32_mask(rtwdev, edcca_th[EDCCA_TH_L2H_IDX].hw_reg.addr,
>                                 edcca_th[EDCCA_TH_L2H_IDX].hw_reg.mask),
> diff --git a/drivers/net/wireless/realtek/rtw88/phy.c b/drivers/net/wireless/realtek/rtw88/phy.c
> index e2ac5c6fd500..c10eb28e54ad 100644
> --- a/drivers/net/wireless/realtek/rtw88/phy.c
> +++ b/drivers/net/wireless/realtek/rtw88/phy.c
> @@ -161,6 +161,9 @@ void rtw_phy_set_edcca_th(struct rtw_dev *rtwdev, u8 l2h, u8 h2l)
>  {
>         const struct rtw_hw_reg_offset *edcca_th = rtwdev->chip->edcca_th;
> 
> +       if (!edcca_th)
> +               return;
> +

The callers of rtw_phy_set_edcca_th() are RTL8822B and RTL8822C, which both
define rtwdev->chip->edcca_th. How can edcca_th be NULL?

>         rtw_write32_mask(rtwdev,
>                          edcca_th[EDCCA_TH_L2H_IDX].hw_reg.addr,
>                          edcca_th[EDCCA_TH_L2H_IDX].hw_reg.mask,
> --
> 2.53.0