[PATCH] wifi: nl80211: require admin perm on SET_PMK / DEL_PMK

[PATCH] wifi: nl80211: require admin perm on SET_PMK / DEL_PMK

[Michael Bommarito]

NL80211_CMD_SET_PMK and NL80211_CMD_DEL_PMK manage the offloaded
4-way-handshake PMK state used by drivers advertising
NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_1X.  The only in-tree
driver that wires up both ->set_pmk / ->del_pmk and advertises
the feature today is brcmfmac, so the practical reach of this
patch is narrow.

Both ops were introduced without a .flags gate, so the generic
netlink layer dispatches them to an unprivileged caller instead
of rejecting with -EPERM at the permission check.  Every other
connection-state op in the adjacent block (CONNECT, ASSOCIATE,
AUTHENTICATE, SET_KEY, ...) carries GENL_UNS_ADMIN_PERM; SET_PMK
/ DEL_PMK were introduced without the flag in 2017 and left
unchanged by later refactors.  Johannes checked the original
Intel submission history and confirmed there is no admin check
in any prior revision either, so this seems likely to be a
simple oversight rather than an intentional carve-out.

Require GENL_UNS_ADMIN_PERM so the genl layer performs the same
capable(CAP_NET_ADMIN) check as its siblings.  wpa_supplicant
already needs CAP_NET_ADMIN for every other nl80211 op it issues,
so supplicant operation is unaffected.  The worst case the missing
gate enables today is an unprivileged local process on a
multi-user system invalidating the offloaded PMK state of another
user's 4-way-handshake session, forcing a full EAP re-auth on the
next reconnect.

Verified in UML: an unprivileged probe (uid=1000) sees
SET_MULTICAST_TO_UNICAST (sibling op with GENL_UNS_ADMIN_PERM)
return -EPERM on both pre- and post-fix kernels, while SET_PMK /
DEL_PMK return -ENODEV from nl80211_pre_doit()'s wdev lookup pre-
fix (proving dispatch crossed the genl permission check) and
-EPERM post-fix (rejected at the genl layer as intended).

Suggested-by: Johannes Berg <johannes@sipsolutions.net>
Fixes: 3a00df5707b6 ("cfg80211: support 4-way handshake offloading for 802.1X")
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-7
---
 net/wireless/nl80211.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b94231c8441c..1f5124cb284d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -19016,6 +19016,7 @@ static const struct genl_small_ops nl80211_small_ops[] = {
 		.cmd = NL80211_CMD_SET_PMK,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = nl80211_set_pmk,
+		.flags = GENL_UNS_ADMIN_PERM,
 		.internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP |
 					 NL80211_FLAG_CLEAR_SKB),
 	},
@@ -19023,6 +19024,7 @@ static const struct genl_small_ops nl80211_small_ops[] = {
 		.cmd = NL80211_CMD_DEL_PMK,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = nl80211_del_pmk,
+		.flags = GENL_UNS_ADMIN_PERM,
 		.internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP),
 	},
 	{
-- 
2.53.0

Re: [PATCH] wifi: nl80211: require admin perm on SET_PMK / DEL_PMK

[Johannes Berg]

On Tue, 2026-04-21 at 18:45 -0400, Michael Bommarito wrote:
> 
> Both ops were introduced without a .flags gate, so the generic
> netlink layer dispatches them to an unprivileged caller instead
> of rejecting with -EPERM at the permission check.  Every other
> connection-state op in the adjacent block (CONNECT, ASSOCIATE,
> AUTHENTICATE, SET_KEY, ...) carries GENL_UNS_ADMIN_PERM; SET_PMK
> / DEL_PMK were introduced without the flag in 2017 and left
> unchanged by later refactors.  Johannes checked the original
> Intel submission history and confirmed there is no admin check
> in any prior revision either, so this seems likely to be a
> simple oversight rather than an intentional carve-out.

FWIW, this submission did originally come from Avi, but we no longer
have a driver using it (it was never upstream anyway), so that now the
only affected driver is brcmfmac, AFAICT (other non-upstream drivers I
wouldn't know, of course.)

Arend, it does seem like the right thing to do here, but I wanted to
confirm with you and thus asked Michael to CC you, what do you think?

johannes

Re: [PATCH] wifi: nl80211: require admin perm on SET_PMK / DEL_PMK

[Arend van Spriel]

On 22/04/2026 00:45, Michael Bommarito wrote:
> NL80211_CMD_SET_PMK and NL80211_CMD_DEL_PMK manage the offloaded
> 4-way-handshake PMK state used by drivers advertising
> NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_1X.  The only in-tree
> driver that wires up both ->set_pmk / ->del_pmk and advertises
> the feature today is brcmfmac, so the practical reach of this
> patch is narrow.
> 
> Both ops were introduced without a .flags gate, so the generic
> netlink layer dispatches them to an unprivileged caller instead
> of rejecting with -EPERM at the permission check.  Every other
> connection-state op in the adjacent block (CONNECT, ASSOCIATE,
> AUTHENTICATE, SET_KEY, ...) carries GENL_UNS_ADMIN_PERM; SET_PMK
> / DEL_PMK were introduced without the flag in 2017 and left
> unchanged by later refactors.  Johannes checked the original
> Intel submission history and confirmed there is no admin check
> in any prior revision either, so this seems likely to be a
> simple oversight rather than an intentional carve-out.
> 
> Require GENL_UNS_ADMIN_PERM so the genl layer performs the same
> capable(CAP_NET_ADMIN) check as its siblings.  wpa_supplicant
> already needs CAP_NET_ADMIN for every other nl80211 op it issues,
> so supplicant operation is unaffected.  The worst case the missing
> gate enables today is an unprivileged local process on a
> multi-user system invalidating the offloaded PMK state of another
> user's 4-way-handshake session, forcing a full EAP re-auth on the
> next reconnect.
> 
> Verified in UML: an unprivileged probe (uid=1000) sees
> SET_MULTICAST_TO_UNICAST (sibling op with GENL_UNS_ADMIN_PERM)
> return -EPERM on both pre- and post-fix kernels, while SET_PMK /
> DEL_PMK return -ENODEV from nl80211_pre_doit()'s wdev lookup pre-
> fix (proving dispatch crossed the genl permission check) and
> -EPERM post-fix (rejected at the genl layer as intended).
> 
> Suggested-by: Johannes Berg <johannes@sipsolutions.net>
> Fixes: 3a00df5707b6 ("cfg80211: support 4-way handshake offloading for 802.1X")

Good catch. Seems like good thing to do.

Acked-by: Arend van Spriel <arend.vanspriel@broadcom>

> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> Assisted-by: Claude:claude-opus-4-7
> ---
>   net/wireless/nl80211.c | 2 ++
>   1 file changed, 2 insertions(+)

Re: [PATCH] wifi: nl80211: require admin perm on SET_PMK / DEL_PMK

[Arend van Spriel]

On 22/04/2026 08:23, Johannes Berg wrote:
> On Tue, 2026-04-21 at 18:45 -0400, Michael Bommarito wrote:
>>
>> Both ops were introduced without a .flags gate, so the generic
>> netlink layer dispatches them to an unprivileged caller instead
>> of rejecting with -EPERM at the permission check.  Every other
>> connection-state op in the adjacent block (CONNECT, ASSOCIATE,
>> AUTHENTICATE, SET_KEY, ...) carries GENL_UNS_ADMIN_PERM; SET_PMK
>> / DEL_PMK were introduced without the flag in 2017 and left
>> unchanged by later refactors.  Johannes checked the original
>> Intel submission history and confirmed there is no admin check
>> in any prior revision either, so this seems likely to be a
>> simple oversight rather than an intentional carve-out.
> 
> FWIW, this submission did originally come from Avi, but we no longer
> have a driver using it (it was never upstream anyway), so that now the
> only affected driver is brcmfmac, AFAICT (other non-upstream drivers I
> wouldn't know, of course.)
> 
> Arend, it does seem like the right thing to do here, but I wanted to
> confirm with you and thus asked Michael to CC you, what do you think?

I agree. I saw the patch earlier this morning and acked the patch just now.

Regards,
Arend