linux-kernel - Re: [PATCH v4] staging: rtl8723bs: fix unchecked return value of skb_copy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <2026012102-thinness-neurotic-6e1b@gregkh>
Date: Wed, 21 Jan 2026 06:05:22 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Minu Jin <s9430939@...er.com>
Cc: andriy.shevchenko@...ux.intel.com, abrahamadekunle50@...il.com,
	milospuric856@...il.com, zxcv2569763104@...il.com,
	dan.carpenter@...aro.org, linux-staging@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] staging: rtl8723bs: fix unchecked return value of
 skb_copy_bits

On Wed, Jan 21, 2026 at 11:33:12AM +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 (including set_qos) to check the return value
>        and handle errors appropriately.
> 
> Signed-off-by: Minu Jin <s9430939@...er.com>
> ---
> Changes in v4:
>     - Modify _rtw_pktfile_read() to return -EINVAL if the remaining data
>       is less than the requested length (suggested by Greg KH).

You do a lot more than just that in that function change.

This needs to be a patch series, each only doing one logical thing.
Please break it up into multiple changes, because as-is, this is hard to
review and verify it is correct.

Also, have you tested this on real hardware?

> -uint _rtw_pktfile_read(struct pkt_file *pfile, u8 *rmem, uint rlen)
> +int _rtw_pktfile_read(struct pkt_file *pfile, u8 *rmem, unsigned int rlen)
>  {
> -	uint	len = 0;
> +	int ret;
>  
> -	len =  rtw_remainder_len(pfile);
> -	len = (rlen > len) ? len : rlen;
> +	if (rtw_remainder_len(pfile) < rlen)
> +		return -EINVAL;

Why change the logic here?  Are you sure it is correct?  Why not
document that in the changelog?

thanks,

greg k-h