RE: [PATCH rtw-next v3 2/2] wifi: rtw88: check the C2H payload length before reading it

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

Luka Gejak <luka.gejak@linux.dev> wrote:
> Hi Ping-Ke,
> 
> Just one comment on the struct idea.
> 
> November 8, 2026 at 3:56 AM, "Ping-Ke Shih" <pkshih@realtek.com> wrote:
> 
> >
> > Huihui Huang <hhhuang@smu.edu.sg> wrote:
> >
> >>
> >> +    if (rtw_c2h_payload_len(skb) < 3) {
> >
> > The 3 is because of GET_SCAN_REPORT_RETURN_CODE(). How does people understand
> > what 3 means?
> >
> > Maybe, we should define specific struct for status_report, like
> >
> > struct status_report {
> >     u8 id;
> >     u8 seq;
> >     u8 rsvd[2]
> >     u8 retrun_code;
> > };
> >
> > struct status_report *status_report = get_c2h_from_skb(skb);
> >
> > if (rtw_c2h_payload_len(skb) < sizeof(*status_report))
> 
> Agreed, a struct is much easier to follow than the bare numbers. One
> small thing though. This struct starts at the C2H header, but
> rtw_c2h_payload_len() leaves the two header bytes out. So as far as I can
> see the check would ask for 5 payload bytes while only 3 are read, and a
> valid report with 3 or 4 payload bytes would be treated as too short.
> 
> Maybe the struct could cover only the payload, like struct rtw_c2h_ra_rpt
> and struct rtw_c2h_adaptivity already do:
> 
>         struct rtw_c2h_scan_status_rpt {
>                 u8 rsvd[2];
>                 u8 return_code;
>         } __packed;
> 
> Then it compares with rtw_c2h_payload_len() directly, and the same would
> work for the channel switch report and the two TX report formats.

I just don't want magic number 3, 5 and etc. That's very hard to people
to understand. No strict opinion about the struct.