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

[Feng Ning <feng@innora.ai>]

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@innora.ai
>
> 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.

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.

I am happy to send v7 with -EINVAL if you prefer that approach.
Alternatively, if min_t() is acceptable as-is, I can add a brief
comment in the code explaining why truncation is intentional.

Please let me know which direction you prefer and I will follow up
promptly.

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.

Thanks,
Feng Ning