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

[Tianchu Chen]

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. 

It is the head of the whole post-handle pipeline
(decryptor -> chk_defrag -> portctrl -> indicate, rtw_recv.c:2082+),
so one check covers not just the three decrypt routines but every
later consumer of hdrlen + iv_len offsets. For TKIP the invariant
has to hold before both stages of the WPA model - decryption and
the Michael MIC verification in recvframe_chkmic(), which subtracts
8 more; the +8 in min_len exists for that second stage, and
decryptor() is the only point preceding both.

This matches the WPA model in mac80211: both
ieee80211_crypto_tkip_decrypt() and ieee80211_crypto_ccmp_decrypt()
reject frames too short for header + IV + ICV/MIC at their entry
(net/mac80211/wpa.c).

Since I have the real hardware, I can also verify the execution flow
at runtime with a build that adds a temporary printk in this path if
that would help.

Please let me know if a v3 is needed, or if there is any further
verification you would like me to do on my side.

Best regards,

Tianchu