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
prev parent 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®