Re: [PATCH v2] staging: rtl8723bs: fix unchecked return value of skb_copy_bits
From: Andy Shevchenko
Date: Tue Jan 20 2026 - 08:43:54 EST
On Tue, Jan 20, 2026 at 10:22:41PM +0900, Minu Jin wrote:
> The function _rtw_pktfile_read() incorrectly updated the file pointer
> even when skb_copy_bits() failed.
>
> This patch fixes the issue by:
>
> 1. Propagating the negative error code from skb_copy_bits() if it fails,
> preventing internal pointer updates.
>
> 2. Updating all callers to check the return value and handle errors
> appropriately.
>
> Signed-off-by: Minu Jin <s9430939@xxxxxxxxx>
> ---
Ick, and what has been changed? Where is the changelog?
...
> struct qos_priv *pqospriv = &pmlmepriv->qospriv;
> signed int res = _SUCCESS;
>
> + signed int ret;
No way. This should be attached to the definition block, id est no blank lines
should be in between.
> s32 rtw_xmitframe_coalesce(struct adapter *padapter, struct sk_buff *pkt, struct
> s32 bmcst = is_multicast_ether_addr(pattrib->ra);
> s32 res = _SUCCESS;
>
> + signed int ret;
Ditto.
...
> extern uint rtw_remainder_len(struct pkt_file *pfile);
> extern void _rtw_open_pktfile(struct sk_buff *pkt, struct pkt_file *pfile);
> -extern uint _rtw_pktfile_read(struct pkt_file *pfile, u8 *rmem, uint rlen);
> +extern int _rtw_pktfile_read(struct pkt_file *pfile, u8 *rmem, uint rlen);
Drop 'extern' at the same time. Or even better, submit a followup patch to drop
all of them at once.
...
Also you can submit the patches to convert types:
uint --> unsigned int
signed int --> int // this one is optional, signed int longer, but still well
// known standard. etc...
...
> -uint _rtw_pktfile_read(struct pkt_file *pfile, u8 *rmem, uint rlen)
> +int _rtw_pktfile_read(struct pkt_file *pfile, u8 *rmem, uint rlen)
> {
> - uint len = 0;
> + int ret;
> + int len = 0;
Reversed xmas tree ordering?
--
With Best Regards,
Andy Shevchenko