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

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

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 is 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. 

I'd point out function name in subject as 
"wifi: rtw88: Add NULL check for chip->edcca_th in rtw_fw_adaptivity_result()"

> Placing
> the check at the function entry avoids logging any
> garbage values.
> 
> This change does not address the root cause for
> this behavior, but it prevents the NULL dereference
> and the resulting oops while a more permanent solution
> is developed.
> 
> 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.
> 
> Suggested-by: Ping-Ke Shih <pkshih@realtek.com>
> Reported-by: Oleksandr Havrylov <goainwo@gmail.com>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=221286

I'd change Link tag to Closes.

> Link:
> https://lore.kernel.org/linux-wireless/CALdGYqQriS7mP0vj_rm_xvisfzFVh0hbpy+---48r6bodZO7tg@mail.gm
> ail.com/

I can preserve this Link, but actually the thread contains more than one
things.

> Signed-off-by: Panagiotis Petrakopoulos <npetrakopoulos2003@gmail.com>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>

> ---
> v2:
>   - Move NULL check to the entry of rtw_fw_adaptivity_result() (Ping-Ke Shih)
>   - Drop rtw_phy_set_edcca_th() hunk; all callers define edcca_th (Ping-Ke Shih)
>   - Change bugzilla reference from Closes: to Link: since this does not
>     address the root cause

Even this is a workaround, I think we can still use Closes and mark the
bugzilla resolved. 

> 
>  drivers/net/wireless/realtek/rtw88/fw.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
> index 48207052e3f8..945fedcd375b 100644
> --- a/drivers/net/wireless/realtek/rtw88/fw.c
> +++ b/drivers/net/wireless/realtek/rtw88/fw.c
> @@ -279,6 +279,9 @@ static void rtw_fw_adaptivity_result(struct rtw_dev *rtwdev, u8 *payload,
>         const struct rtw_hw_reg_offset *edcca_th = rtwdev->chip->edcca_th;
>         struct rtw_c2h_adaptivity *result = (struct rtw_c2h_adaptivity *)payload;
> 
> +       if (!edcca_th)
> +               return;
> +
>         rtw_dbg(rtwdev, RTW_DBG_ADAPTIVITY,
>                 "Adaptivity: density %x igi %x l2h_th_init %x l2h %x h2l %x option %x\n",
>                 result->density, result->igi, result->l2h_th_init, result->l2h,
> --
> 2.53.0