[PATCH v5 3/3] staging: rtl8723bs: prevent partial reads in _rtw_pktfile_read

Minu Jin posted 3 patches 2 weeks, 4 days ago
[PATCH v5 3/3] staging: rtl8723bs: prevent partial reads in _rtw_pktfile_read
Posted by Minu Jin 2 weeks, 4 days ago
The current implementation of _rtw_pktfile_read() allows reading less
data than requested if there isn't enough data remaining.

This is problematic because callers usually request a fixed size (like
a header size) and expect that full amount. Reading only part of the
data means the caller gets incomplete information, which can lead to
errors in packet processing.

To fix this, update the function to:
1. Return -EINVAL if the remaining data is smaller than the requested
   length.
2. Check the return value of skb_copy_bits() and propagate errors.
3. Only update the internal pointers (cur_addr, pkt_len) if the read
   is fully successful.

Callers have already been updated in previous patches to handle these
negative error codes.

Signed-off-by: Minu Jin <s9430939@naver.com>
---
 drivers/staging/rtl8723bs/os_dep/xmit_linux.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
index ea54a573e025..72cf8cd5f7c6 100644
--- a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
@@ -23,21 +23,20 @@ void _rtw_open_pktfile(struct sk_buff *pktptr, struct pkt_file *pfile)
 
 int _rtw_pktfile_read(struct pkt_file *pfile, u8 *rmem, unsigned int rlen)
 {
-	unsigned int len;
 	int ret;
 
-	len =  rtw_remainder_len(pfile);
- 	len = (rlen > len) ? len : rlen;
+	if (rtw_remainder_len(pfile) < rlen)
+		return -EINVAL;
 
 	if (rmem) {
-		ret = skb_copy_bits(pfile->pkt, pfile->buf_len - pfile->pkt_len, rmem, len);
+		ret = skb_copy_bits(pfile->pkt, pfile->buf_len - pfile->pkt_len, rmem, rlen);
 		if (ret < 0)
 			return ret;
 	}
 
-	pfile->cur_addr += len;
-	pfile->pkt_len -= len;
-	return len;
+	pfile->cur_addr += rlen;
+	pfile->pkt_len -= rlen;
+	return rlen;
 }
 
 signed int rtw_endofpktfile(struct pkt_file *pfile)
-- 
2.43.0
Re: [PATCH v5 3/3] staging: rtl8723bs: prevent partial reads in _rtw_pktfile_read
Posted by Dan Carpenter 2 weeks, 4 days ago
On Thu, Jan 22, 2026 at 01:14:50PM +0900, Minu Jin wrote:
> The current implementation of _rtw_pktfile_read() allows reading less
> data than requested if there isn't enough data remaining.
> 
> This is problematic because callers usually request a fixed size (like
> a header size) and expect that full amount. Reading only part of the
> data means the caller gets incomplete information, which can lead to
> errors in packet processing.
> 
> To fix this, update the function to:
> 1. Return -EINVAL if the remaining data is smaller than the requested
>    length.
> 2. Check the return value of skb_copy_bits() and propagate errors.
> 3. Only update the internal pointers (cur_addr, pkt_len) if the read
>    is fully successful.
> 
> Callers have already been updated in previous patches to handle these
> negative error codes.
> 
> Signed-off-by: Minu Jin <s9430939@naver.com>
> ---
>  drivers/staging/rtl8723bs/os_dep/xmit_linux.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> index ea54a573e025..72cf8cd5f7c6 100644
> --- a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> +++ b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> @@ -23,21 +23,20 @@ void _rtw_open_pktfile(struct sk_buff *pktptr, struct pkt_file *pfile)
>  
>  int _rtw_pktfile_read(struct pkt_file *pfile, u8 *rmem, unsigned int rlen)
>  {
> -	unsigned int len;
>  	int ret;
>  
> -	len =  rtw_remainder_len(pfile);
> - 	len = (rlen > len) ? len : rlen;

The rest of this looks okay.  But just drop patch 1 since you
delete the len variable in the end anyway.  Leave the type alone and
leave the whitespace alone.  Your subjects are too vague.

patch 1: update _rtw_pktfile_read() to return error codes
patch 2: clean up _rtw_pktfile_read()

regards,
dan carpenter