RE: [PATCH] wifi: rtw88: allow key removal when the CAM table is full

[Ping-Ke Shih]

Luka Gejak <luka.gejak@xxxxxxxxx> wrote:
> Hi Ping-Ke,
> 
> October 20, 2026 at 4:07 AM, "Ping-Ke Shih" <pkshih@xxxxxxxxxxx> wrote:
> 
> >
> > Hungyu Lin <dennylin0707@xxxxxxxxx> 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