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

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

[Feng Ning]

The cfg80211 framework allows key sequence counters (NL80211_KEY_SEQ)
up to 16 bytes, 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 adjacent fields key_len and key[].

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

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Signed-off-by: Feng Ning <feng@innora.ai>
---
Changes v3 -> v4:
  - Re-send as plain text without PGP signature and public key
    attachment (per Luka Gejak feedback)
  - No code changes from v3

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

Changes v1 -> v2:
  - Initial public submission to linux-staging mailing list

 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 1ff763c10..4a984c071 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -893,8 +893,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 v4] staging: rtl8723bs: fix heap buffer overflow in cfg80211_rtw_add_key()

[Luka Gejak]

On April 9, 2026 12:46:13 AM GMT+02:00, Feng Ning <feng@innora.ai> wrote:
>The cfg80211 framework allows key sequence counters (NL80211_KEY_SEQ)
>up to 16 bytes, 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 adjacent fields key_len and key[].
>
>Cap the copy length at the buffer size using min_t().
>
>Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
>Signed-off-by: Feng Ning <feng@innora.ai>
>---
>Changes v3 -> v4:
>  - Re-send as plain text without PGP signature and public key
>    attachment (per Luka Gejak feedback)
>  - No code changes from v3
>
>Changes v2 -> v3:
>  - Added changelog below the cut line (per gregkh patch-bot)
>  - No code changes from v2
>
>Changes v1 -> v2:
>  - Initial public submission to linux-staging mailing list
>
> 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 1ff763c10..4a984c071 100644
>--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
>+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
>@@ -893,8 +893,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;

Looks great. Thank you for dropping the PGP attachments and formatting
the changelog correctly for the mailing list. The fix cleanly 
addresses the bounds checking issue and safely mitigates the heap 
buffer overflow.
Reviewed-by: Luka Gejak <luka.gejak@linux.dev

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

[Luka Gejak]

On April 9, 2026 12:46:13 AM GMT+02:00, Feng Ning <feng@innora.ai> wrote:
>The cfg80211 framework allows key sequence counters (NL80211_KEY_SEQ)
>up to 16 bytes, 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 adjacent fields key_len and key[].
>
>Cap the copy length at the buffer size using min_t().
>
>Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
>Signed-off-by: Feng Ning <feng@innora.ai>
>---
>Changes v3 -> v4:
>  - Re-send as plain text without PGP signature and public key
>    attachment (per Luka Gejak feedback)
>  - No code changes from v3
>
>Changes v2 -> v3:
>  - Added changelog below the cut line (per gregkh patch-bot)
>  - No code changes from v2
>
>Changes v1 -> v2:
>  - Initial public submission to linux-staging mailing list
>
> 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 1ff763c10..4a984c071 100644
>--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
>+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
>@@ -893,8 +893,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;

Looks great. Thank you for dropping the PGP attachments and formatting
the changelog correctly for the mailing list. The fix cleanly 
addresses the bounds checking issue and safely mitigates the heap 
buffer overflow.
Edit: I forgot to close <> around email address in Reviewed-by tag
Reviewed-by: Luka Gejak <luka.gejak@linux.dev>