Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH] wifi: rtw88: allow key removal when the CAM table is full
@ 2026-10-19 16:51 Hungyu Lin
  2026-10-19 19:01 ` Luka Gejak
  2026-10-20  2:07 ` Ping-Ke Shih
  0 siblings, 2 replies; 6+ messages in thread
From: Hungyu Lin @ 2026-10-19 16:51 UTC (permalink / raw)
  To: pkshih; +Cc: linux-wireless, linux-kernel, reza1234khan1234, Hungyu Lin

rtw_ops_set_key() looks for a free CAM entry before checking the
command. When all pairwise entries are in use, DISABLE_KEY returns
-ENOSPC without clearing the existing entry.

Move the allocation and bounds check into SET_KEY. DISABLE_KEY uses
the existing key->hw_key_idx.

Fixes: 10d58823cb44 ("wifi: rtw88: fix out-of-bounds CAM write when key table is full")
Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 drivers/net/wireless/realtek/rtw88/mac80211.c | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index 2a9b09fa76e7..5e40cf06a9e3 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -598,20 +598,20 @@ static int rtw_ops_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 
 	rtw_leave_lps_deep(rtwdev);
 
-	if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
-		hw_key_idx = rtw_sec_get_free_cam(sec);
-	} else {
-		/* multiple interfaces? */
-		hw_key_idx = key->keyidx;
-	}
-
-	if (hw_key_idx >= sec->total_cam_num) {
-		ret = -ENOSPC;
-		goto out;
-	}
-
 	switch (cmd) {
 	case SET_KEY:
+		if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
+			hw_key_idx = rtw_sec_get_free_cam(sec);
+		} else {
+			/* multiple interfaces? */
+			hw_key_idx = key->keyidx;
+		}
+
+		if (hw_key_idx >= sec->total_cam_num) {
+			ret = -ENOSPC;
+			goto out;
+		}
+
 		/* need sw generated IV */
 		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
 		key->hw_key_idx = hw_key_idx;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] wifi: rtw88: allow key removal when the CAM table is full
  2026-10-19 16:51 [PATCH] wifi: rtw88: allow key removal when the CAM table is full Hungyu Lin
@ 2026-10-19 19:01 ` Luka Gejak
  2026-10-20  2:07 ` Ping-Ke Shih
  1 sibling, 0 replies; 6+ messages in thread
From: Luka Gejak @ 2026-10-19 19:01 UTC (permalink / raw)
  To: Hungyu Lin, pkshih
  Cc: linux-wireless, linux-kernel, reza1234khan1234, Hungyu Lin

October 19, 2026 at 6:51 PM, "Hungyu Lin" <dennylin0707@gmail.com> wrote:


> 
> rtw_ops_set_key() looks for a free CAM entry before checking the
> command. When all pairwise entries are in use, DISABLE_KEY returns
> -ENOSPC without clearing the existing entry.
> 
> Move the allocation and bounds check into SET_KEY. DISABLE_KEY uses
> the existing key->hw_key_idx.
> 
> Fixes: 10d58823cb44 ("wifi: rtw88: fix out-of-bounds CAM write when key table is full")
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> ---

Looking good to me.

Reviewed-by: Luka Gejak <luka.gejak@linux.dev>

Best regards,
Luka Gejak

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] wifi: rtw88: allow key removal when the CAM table is full
  2026-10-19 16:51 [PATCH] wifi: rtw88: allow key removal when the CAM table is full Hungyu Lin
  2026-10-19 19:01 ` Luka Gejak
@ 2026-10-20  2:07 ` Ping-Ke Shih
  2026-10-20  4:43   ` Luka Gejak
  1 sibling, 1 reply; 6+ messages in thread
From: Ping-Ke Shih @ 2026-10-20  2:07 UTC (permalink / raw)
  To: Hungyu Lin
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	reza1234khan1234@gmail.com


Hungyu Lin <dennylin0707@gmail.com> wrote:
> rtw_ops_set_key() looks for a free CAM entry before checking the
> command. When all pairwise entries are in use, DISABLE_KEY returns
> -ENOSPC without clearing the existing entry.

Is this the actual case you met?

> 
> Move the allocation and bounds check into SET_KEY. DISABLE_KEY uses
> the existing key->hw_key_idx.
> 
> Fixes: 10d58823cb44 ("wifi: rtw88: fix out-of-bounds CAM write when key table is full")

Should it be ?

Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")

> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>

Then, 

Acked-by: Ping-Ke Shih <pkshih@realtek.com>




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] wifi: rtw88: allow key removal when the CAM table is full
  2026-10-20  2:07 ` Ping-Ke Shih
@ 2026-10-20  4:43   ` Luka Gejak
  2026-10-20  5:29     ` Ping-Ke Shih
  0 siblings, 1 reply; 6+ messages in thread
From: Luka Gejak @ 2026-10-20  4:43 UTC (permalink / raw)
  To: Ping-Ke Shih, Hungyu Lin; +Cc: linux-wireless, linux-kernel, reza1234khan1234

Hi Ping-Ke,

October 20, 2026 at 4:07 AM, "Ping-Ke Shih" <pkshih@realtek.com> wrote:

> 
> Hungyu Lin <dennylin0707@gmail.com> wrote:
> 
> > 
> > rtw_ops_set_key() looks for a free CAM entry before checking the
> >  command. When all pairwise entries are in use, DISABLE_KEY returns
> >  -ENOSPC without clearing the existing entry.
> > 
> Is this the actual case you met?
> 

From my investigation it seems reachable.

As far as I undersrand, it needs all 28 pairwise entries (4-31) in
use at once, so AP mode with 28 secured stations (31 mac_ids are
left after the AP vif), or IBSS/TDLS with as many peers. Not
reachable as a plain client. Once it is hit its pairwise
DISABLE_KEY fails, mac80211 frees the key anyway after logging
"failed to remove key", the cam_map bit is never cleared, and the
table stays full until the driver is reloaded.

I think it can be tested with two clients by temporarily setting
sec.total_cam_num = 6.

> > 
> > Move the allocation and bounds check into SET_KEY. DISABLE_KEY uses
> >  the existing key->hw_key_idx.
> >  
> >  Fixes: 10d58823cb44 ("wifi: rtw88: fix out-of-bounds CAM write when key table is full")
> > 
> Should it be ?
> 
> Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
> 

I think 10d58823cb44 is the right tag. In e3037485c68e the check was

      if (hw_key_idx > sec->total_cam_num) {

and rtw_sec_get_free_cam() returns at most RTW_MAX_SEC_CAM_NUM (32),
which equals total_cam_num, so the check never fired and DISABLE_KEY
always reached rtw_sec_clear_cam(). The bug there was the
out-of-bounds SET_KEY, which 10d58823cb44 fixed. DISABLE_KEY only
started failing once the check became ">=". This patch also does not
apply without 10d58823cb44, since its context has the ">=". Since
10d58823cb44 is tagged for stable, I think this one should carry
Cc: stable@vger.kernel.org too.

Best regards,
Luka Gejak

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] wifi: rtw88: allow key removal when the CAM table is full
  2026-10-20  4:43   ` Luka Gejak
@ 2026-10-20  5:29     ` Ping-Ke Shih
  2026-10-20  8:28       ` Hungyu Lin
  0 siblings, 1 reply; 6+ messages in thread
From: Ping-Ke Shih @ 2026-10-20  5:29 UTC (permalink / raw)
  To: Luka Gejak, Hungyu Lin
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	reza1234khan1234@gmail.com

Luka Gejak <luka.gejak@linux.dev> wrote:
> Hi Ping-Ke,
> 
> October 20, 2026 at 4:07 AM, "Ping-Ke Shih" <pkshih@realtek.com> wrote:
> 
> >
> > Hungyu Lin <dennylin0707@gmail.com> wrote:
> >
> > >
> > > rtw_ops_set_key() looks for a free CAM entry before checking the
> > >  command. When all pairwise entries are in use, DISABLE_KEY returns
> > >  -ENOSPC without clearing the existing entry.
> > >
> > Is this the actual case you met?
> >
> 
> From my investigation it seems reachable.

I knew. I just want to know if people encountered the problem, or 
found and fixed this by reviewing. 

> 
> As far as I undersrand, it needs all 28 pairwise entries (4-31) in
> use at once, so AP mode with 28 secured stations (31 mac_ids are
> left after the AP vif), or IBSS/TDLS with as many peers. Not
> reachable as a plain client. Once it is hit its pairwise
> DISABLE_KEY fails, mac80211 frees the key anyway after logging
> "failed to remove key", the cam_map bit is never cleared, and the
> table stays full until the driver is reloaded.
> 
> I think it can be tested with two clients by temporarily setting
> sec.total_cam_num = 6.
> 
> > >
> > > Move the allocation and bounds check into SET_KEY. DISABLE_KEY uses
> > >  the existing key->hw_key_idx.
> > >
> > >  Fixes: 10d58823cb44 ("wifi: rtw88: fix out-of-bounds CAM write when key table is full")
> > >
> > Should it be ?
> >
> > Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
> >
> 
> I think 10d58823cb44 is the right tag. In e3037485c68e the check was
> 
>       if (hw_key_idx > sec->total_cam_num) {
> 
> and rtw_sec_get_free_cam() returns at most RTW_MAX_SEC_CAM_NUM (32),
> which equals total_cam_num, so the check never fired and DISABLE_KEY
> always reached rtw_sec_clear_cam(). The bug there was the
> out-of-bounds SET_KEY, which 10d58823cb44 fixed. DISABLE_KEY only
> started failing once the check became ">=". This patch also does not
> apply without 10d58823cb44, since its context has the ">=".

I feel the initial commit e3037485c68e containing two bugs. One is 
missing to check proper boundary, and the other is this one. 

Without e3037485c68e, it will out-of-bounds SET_KEY, and then
it is possible to DISABLE_KEY fail (at wrong boundary), no?

Ping-Ke



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] wifi: rtw88: allow key removal when the CAM table is full
  2026-10-20  5:29     ` Ping-Ke Shih
@ 2026-10-20  8:28       ` Hungyu Lin
  0 siblings, 0 replies; 6+ messages in thread
From: Hungyu Lin @ 2026-10-20  8:28 UTC (permalink / raw)
  To: Ping-Ke Shih
  Cc: Luka Gejak, linux-wireless@vger.kernel.org,
	linux-kernel@vger.kernel.org, reza1234khan1234@gmail.com

> I knew. I just want to know if people encountered the problem, or
> found and fixed this by reviewing.

Found by code inspection. I don't have rtw88 hardware.

Thanks,
Hungyu

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-20  8:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-19 16:51 [PATCH] wifi: rtw88: allow key removal when the CAM table is full Hungyu Lin
2026-10-19 19:01 ` Luka Gejak
2026-10-20  2:07 ` Ping-Ke Shih
2026-10-20  4:43   ` Luka Gejak
2026-10-20  5:29     ` Ping-Ke Shih
2026-10-20  8:28       ` Hungyu Lin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox