Re: [PATCH v2] staging: rtl8723bs: fix protected RX frame validation in decrypt path

[Greg KH]

On Tue, Sep 08, 2026 at 04:02:15PM +0000, Tianchu Chen wrote:
> September 8, 2026 at 12:13 AM, "Greg KH" <gregkh@xxxxxxxxxxxxxxxxxxx mailto:gregkh@xxxxxxxxxxxxxxxxxxx?to=%22Greg%20KH%22%20%3Cgregkh%40linuxfoundation.org%3E > wrote:
> 
> 
> -snip-
> 
> > >  
> > >  drivers/staging/rtl8723bs/core/rtw_recv.c | 21 ++++++++++++++++++---
> > >  1 file changed, 18 insertions(+), 3 deletions(-)
> > >  
> > >  diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
> > >  index 7568fc514d7ce..4756e0fedd46f 100644
> > >  --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
> > >  +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
> > >  @@ -426,8 +426,21 @@ static union recv_frame *decryptor(struct adapter *padapter, union recv_frame *p
> > >  u32 res = _SUCCESS;
> > >  
> > >  if (prxattrib->encrypt > 0) {
> > >  - u8 *iv = precv_frame->u.hdr.rx_data + prxattrib->hdrlen;
> > >  + u8 *iv;
> > >  + u32 min_len = prxattrib->hdrlen + prxattrib->iv_len + prxattrib->icv_len;
> > > 
> > Why will this not overflow?
> 
> The overflow(without this fix) happens inside the per-suite decrypt routines
> when a received frame is shorter than this minimum. For example:
> 
> rtw_aes_decrypt() computes length = len - hdrlen - iv_len, so a
> 30-byte frame with hdrlen = 26 and iv_len = 8 wraps length to ~4GiB,
> and aes_decipher() then iterates num_blocks = (plen - 8) / 16 16-byte
> blocks, reading and writing gigabytes past the skb.
> 
> The WEP and TKIP decryptors start from the same subtraction and
> underflow the same way. 
> 
> min_len is the minimum size of a legitimate protected frame: header +
> IV + ICV, plus the 8-byte Michael MIC for TKIP.
> 
> The min_len computation itself cannot wrap either: all three
> addends are u8 fields, hdrlen is at most 36 and iv_len/icv_len are
> per-suite constants (max 18/16), so the sum stays below 80 even with
> the TKIP +8. A frame shorter than min_len cannot even hold its IV
> and ICV, so only malformed frames are dropped.
> 
> 
> > > 
> > > + /* TKIP appends an 8-byte Michael MIC that icv_len doesn't account for */
> > >  + if (prxattrib->encrypt == _TKIP_)
> > >  + min_len += 8;
> > >  +
> > >  + /* a protected frame must be long enough to hold the IV and ICV/MIC */
> > >  + if (precv_frame->u.hdr.len < min_len) {
> > >  + rtw_free_recvframe(precv_frame,
> > >  + &padapter->recvpriv.free_recv_queue);
> > >  + return NULL;
> > >  + }
> > >  +
> > >  + iv = precv_frame->u.hdr.rx_data + prxattrib->hdrlen;
> > > 
> > What prevents this from overflowing?
> 
> The check above keeps the offset within the frame: it guarantees
> len >= hdrlen + iv_len + icv_len, and iv_len >= 4 for every suite, so
> len >= hdrlen + 4 and both iv and the iv[3] dereference stay within the
> first len bytes ([rx_data, rx_data + len)).
> 
> 
> Those len bytes are in turn inside the allocation: hdr.len is set by
> recvframe_put() only after pkt_exceeds_tail() verified that pkt_len
> bytes were actually copied from the RX FIFO into an skb sized for
> them (rtl8723bs_recv.c), so any offset below len is inside the
> buffer.
> 
> This is also why the iv assignment moved: the iv[3] read that follows
> it would otherwise be an out-of-bounds read when the frame is shorter
> than hdrlen + 4. The IV may only be examined once the frame is known
> to actually contain it.
> 
> 
> > 
> > thanks,
> > 
> > greg k-h
> >
> 
> Regarding the format issue being mentioned earlier, I can send a v3
> patch. Also, I believe decryptor() is also where this check should belongs. 

Please send a v3.

thanks,

greg k-h