Re: [PATCH v6] staging: rtl8723bs: fix heap buffer overflow in cfg80211_rtw_add_key()

[Greg KH]

On Mon, May 04, 2026 at 03:48:30PM +0000, Feng Ning wrote:
> On Mon, May 04, 2026 at 04:12:44PM +0200, Greg KH wrote:
> > What about these review comments:
> >         https://sashiko.dev/#/patchset/20260427111738.33069-1-feng@xxxxxxxxx
> >
> > Are they incorrect?
> >
> > And was this tested on real hardware?
> 
> Hi Greg,
> 
> Thank you for the pointer to the Sashiko review.
> 
> Regarding the review comment (Medium): Sashiko suggests returning -EINVAL
> when params->seq_len exceeds sizeof(param->u.crypt.seq), rather than
> silently truncating with min_t().
> 
> The comment raises a valid point.  I chose min_t() for two reasons:
> 
>   1. The upstream cfg80211 framework does not enforce an upper bound on
>      seq_len before reaching the driver, so a strict -EINVAL could
>      break any existing userspace that happens to pass seq_len > 8
>      (even if no standard cipher requires more than 6 bytes).
> 
>   2. Staging drivers historically favour silent clamping over hard
>      rejections for parameters that are out of the ordinary but
>      otherwise harmless -- the primary goal was to close the overflow,
>      not to police the caller.

Let's fix this in a way that the code can be moved out of staging
someday please.

> That said, I can see the argument for -EINVAL: it makes the contract
> explicit and avoids installing a key with a truncated sequence counter
> that could produce unexpected crypto behaviour.

Yes, that is better.

> Regarding hardware testing: I do not currently have a physical
> rtl8723bs device.  My verification was based on code review of the
> cfg80211 key installation path and static analysis confirming that
> ieee_param.crypt.seq is an 8-byte fixed buffer while params->seq_len
> is fully userspace-controlled via NL80211_CMD_NEW_KEY.
> 
> I understand this is a limitation.  If hardware testing is required
> before merge I can source a RTL8723BU/BS USB dongle (approximately
> 1-2 weeks), or alternatively a community member with the hardware
> could confirm the fix.  Please advise on your preference.

Ideally someone can test this on the real hardware.  I'm loath to take
real patches for this driver without that happening.

thanks,

greg k-h