mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Tianchu Chen" <tianchu.chen@linux.dev>
To: "Greg KH" <gregkh@linuxfoundation.org>
Cc: hansg@kernel.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] staging: rtl8723bs: fix protected RX frame validation in decrypt path
Date: Thu, 10 Sep 2026 08:41:17 +0000	[thread overview]
Message-ID: <7d58756ac12f7b324c2d8693b03287b786020fd6@linux.dev> (raw)
In-Reply-To: <2026090919-stammer-outclass-5c6f@gregkh>

September 9, 2026 at 3:44 PM, "Greg KH" <gregkh@linuxfoundation.org mailto:gregkh@linuxfoundation.org?to=%22Greg%20KH%22%20%3Cgregkh%40linuxfoundation.org%3E > wrote:


> 
> On Tue, Sep 08, 2026 at 04:02:15PM +0000, Tianchu Chen wrote:
> 
> > 
> > September 8, 2026 at 12:13 AM, "Greg KH" <gregkh@linuxfoundation.org mailto:gregkh@linuxfoundation.org?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
>


Hi Greg,

V3 has been sent: https://lore.kernel.org/all/8ee3bdd1c45034200cb1aa9a7e9b575b9584643a@linux.dev/

Best regards,

Tianchu

      reply	other threads:[~2026-09-10  8:41 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-03  9:19 Tianchu Chen
2026-09-07 16:13 ` Greg KH
2026-09-08 16:02   ` Tianchu Chen
2026-09-09  7:44     ` Greg KH
2026-09-10  8:41       ` Tianchu Chen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7d58756ac12f7b324c2d8693b03287b786020fd6@linux.dev \
    --to=tianchu.chen@linux.dev \
    --cc=gregkh@linuxfoundation.org \
    --cc=hansg@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®