Rejecting duplicate cipher suites in wiphy_register()

First posted 2026-04-13 · merged into mainline 2026-04-28 · wifi, cfg80211, WEXT

When a wireless driver registers with cfg80211, it describes its hardware in a struct wiphy. Among dozens of fields — supported bands, interface modes, bitrate tables — there is a small list of cipher suites the hardware can do: TKIP, CCMP, the two WEP variants, and a handful of others. This post walks through a bug where cfg80211 blindly accepted duplicate entries in that list, how that breaks the ancient WEXT compatibility layer with an out-of-bounds write, and why the fix I sent upstream rejects the malformed description at registration time rather than patching the one consumer that happened to blow up.

The bug

The cipher suite list is conceptually a set: a cipher either is or isn't supported by the device. Listing WLAN_CIPHER_SUITE_WEP104 twice does not mean "this device supports WEP104 twice as much". It means the driver's description is malformed — usually a copy-paste slip in a static array, or two feature blocks that both appended the same cipher.

cfg80211, however, performed no such check. A driver could register with any number of duplicates and the kernel happily carried the list around, leaving every consumer to interpret it on its own. Most consumers treat the list as a set and are unaffected. At least one treats it as a sequence and appends state per entry — and that one had a fixed-size destination buffer.

The commit message states the problem compactly:

Duplicate entries in wiphy->cipher_suites do not describe any
additional capability, but cfg80211 currently accepts them and leaves
individual consumers to deal with them.

One such consumer is the WEXT compatibility code, which appends a WEP
key length for each WEP cipher entry it sees. Repeated WEP entries can
therefore overflow the fixed iw_range::encoding_size array returned by
SIOCGIWRANGE.

So the bug has two halves: (1) cfg80211 accepts a description that carries no valid meaning, and (2) at least one consumer of that description writes past a fixed-size array when the duplicates are WEP entries. The second half is the exploitable symptom; the first half is the root cause.

The consumer that couldn't cope

Wireless Extensions (WEXT) is the pre-nl80211 userspace API. Modern tools talk netlink, but the SIOCGIWRANGE ioctl still works — cfg80211 keeps a compatibility shim in net/wireless/wext-compat.c so that old binaries keep functioning. The handler is cfg80211_wext_giwrange(), hooked into the WEXT handler table at the bottom of the file:

IW_HANDLER(SIOCGIWRANGE,	cfg80211_wext_giwrange),

Any process can reach it: open a socket, issue ioctl(fd, SIOCGIWRANGE, &wrq) on a wireless interface. No special privilege is needed for a "get range" query. The handler fills a struct iw_range in kernel memory and copies it back to userspace.

The fixed-size array

struct iw_range is a uapi structure, frozen in include/uapi/linux/wireless.h since the 1990s. The "encoder stuff" section looks like this:

#define IW_MAX_ENCODING_SIZES	8

/* Encoder stuff */
__u16	encoding_size[IW_MAX_ENCODING_SIZES];	/* Different token sizes */
__u8	num_encoding_sizes;	/* Number of entry in the list */
__u8	max_encoding_tokens;	/* Max number of tokens */

That is eight __u16 slots — sixteen bytes — with the count sitting immediately after the array, followed by the rest of the structure. There is no capacity field; the layout is ABI and cannot grow.

The append loop

Inside cfg80211_wext_giwrange(), the cipher suite list is iterated once, and each entry maps to WEXT flags. TKIP and CCMP just OR bits into enc_capa — idempotent, harmless under duplication. The WEP cases are different: they append:

for (i = 0; i < wdev->wiphy->n_cipher_suites; i++) {
	switch (wdev->wiphy->cipher_suites[i]) {
	case WLAN_CIPHER_SUITE_TKIP:
		range->enc_capa |= (IW_ENC_CAPA_CIPHER_TKIP |
				    IW_ENC_CAPA_WPA);
		break;

	case WLAN_CIPHER_SUITE_CCMP:
		range->enc_capa |= (IW_ENC_CAPA_CIPHER_CCMP |
				    IW_ENC_CAPA_WPA2);
		break;

	case WLAN_CIPHER_SUITE_WEP40:
		range->encoding_size[range->num_encoding_sizes++] =
			WLAN_KEY_LEN_WEP40;
		break;

	case WLAN_CIPHER_SUITE_WEP104:
		range->encoding_size[range->num_encoding_sizes++] =
			WLAN_KEY_LEN_WEP104;
		break;
	}
}

There is no bounds check against IW_MAX_ENCODING_SIZES. The code assumes — reasonably, before this bug class existed — that the wiphy description contains each cipher at most once, so the loop can write at most two entries (WEP40 and WEP104) into an array of eight.

Now connect the two halves. If wiphy->cipher_suites contains nine WLAN_CIPHER_SUITE_WEP104 entries — which cfg80211 accepted without complaint — the ninth iteration writes encoding_size[8], one slot past the end, and increments num_encoding_sizes past the true count. The overflow target is the very next field in the struct (num_encoding_sizes, max_encoding_tokens, encoding_login_index...), and with enough duplicates the writes march through the remainder of struct iw_range. The range buffer lives inside the ioctl handling path; what exactly gets corrupted depends on the call site's allocation, but "uncontrolled count of __u16 writes past a uapi struct, reachable by an unprivileged ioctl, parameterized by a driver-supplied list" is a bad sentence in any dialect.

Two things make this worse than it first looks. First, the duplicated list does not have to come from a malicious actor at runtime — it is a static description authored by the driver (or firmware abstraction layer) and registered once, so a single buggy or compromised driver registration permanently arms the overflow for every later SIOCGIWRANGE call on that device. Second, the trigger and the mistake are far apart: the overflow fires in a legacy ioctl handler, while the actual error was accepting the malformed list in wiphy_register(), potentially much earlier.

Why not patch WEXT

The obvious one-liner would be to bound the loop:

if (range->num_encoding_sizes < IW_MAX_ENCODING_SIZES)
	range->encoding_size[range->num_encoding_sizes++] = ...;

That fixes this overflow, and nothing else. It leaves the invariant broken: cfg80211 would still accept a description containing data that means nothing, and every other consumer — present and future — still has to decide for itself what duplicates mean. Consider what else reads the list:

That spread is the point. The semantics of the list are "set of supported ciphers", but the representation is "array the driver handed us, whatever's in it". Every consumer has to re-derive the set semantics, and as WEXT demonstrates, getting it wrong is a memory-safety bug, not just a cosmetic one. Deduplicating in WEXT would also leave a subtler question unanswered: should WEXT dedup? Should nl80211 dedup before copying to userspace? Should each of them silently paper over driver bugs differently?

The cleaner invariant is to make the malformed input illegal at the boundary where it enters the subsystem. Then "no duplicates" stops being an assumption scattered across consumers and becomes a guaranteed property of any registered wiphy, provable by pointing at one check in one function. This is the same reason wiphy_register() already rejects empty channel lists, bad band/rate combinations, and inconsistent interface mode masks — the registration path is exactly where cfg80211 enforces that driver descriptions are sane.

The fix

The patch adds one helper and one call site in net/wireless/core.c. The helper, placed right after wiphy_verify_combinations() — the existing "validate the driver's description" function — is:

static bool wiphy_cipher_suites_valid(const struct wiphy *wiphy)
{
	int i, j;

	if (wiphy->n_cipher_suites && !wiphy->cipher_suites)
		return false;

	for (i = 0; i < wiphy->n_cipher_suites; i++) {
		for (j = 0; j < i; j++) {
			if (wiphy->cipher_suites[i] ==
			    wiphy->cipher_suites[j])
				return false;
		}
	}

	return true;
}

And the call site, inside wiphy_register() immediately after wiphy_verify_combinations() succeeds:

	res = wiphy_verify_combinations(wiphy);
	if (res)
		return res;

	if (!wiphy_cipher_suites_valid(wiphy))
		return -EINVAL;

	/* sanity check supported bands/channels */

The NULL-vs-count check

The first condition handles an inconsistency that has nothing to do with duplicates: n_cipher_suites != 0 with a NULL cipher_suites pointer. That combination is how you get a NULL dereference in every consumer shown above — each loop indexes cipher_suites[i] up to the count. Catching it here converts an oops at first use into a clean -EINVAL at registration. (The reverse — non-NULL pointer with zero count — is harmless: every loop body runs zero times, so the patch leaves it alone. Minimal checks for real problems.)

The O(n²) duplicate loop

The nested loop is a straightforward all-pairs comparison: for each element, compare against every earlier element and fail on the first match. One could ask why not sort a copy and compare neighbors, or use a bitmap keyed by cipher ID. The answer is that this list is tiny — real drivers advertise somewhere between two and a dozen suites — and wiphy_register() runs once per device, at probe time, never in a hot path. An O(n²) scan over eight integers is a few dozen cycles of predictable, obviously-correct code. Sorting would add allocation and comparison machinery; a bitmap would need a bound on valid cipher IDs. Both buy performance nobody needs at the cost of code somebody has to review. This is the right kind of dumb.

Failing early with -EINVAL

The call site placement matters. It sits before the band/channel sanity checks and long before any state is committed — before the wiphy is visible to userspace, before rfkill registration, before any netdev exists. A driver with a malformed list simply fails to probe, with -EINVAL returned to its wiphy_register() call. The failure is loud, immediate, and attributed to the right place: the driver's own description, not a crash in an unrelated ioctl handler weeks later. Driver authors testing against a patched cfg80211 find their copy-paste error on the first insmod.

How it was investigated

Credit where due: the issue was reported by Yifan Wu and Juefei Pu, and the fix direction — reject at registration rather than patch the consumer — was suggested by Xin Liu. My part was tracing the data flow to confirm the report, checking whether other consumers had the same shape, and writing the patch. Concretely, that meant a few hours with grep.

First, find every writer and reader of the encoding-size array to confirm the overflow and see if it was duplicated anywhere else:

$ grep -rn "encoding_size" net/wireless/
net/wireless/wext-compat.c:174:			range->encoding_size[range->num_encoding_sizes++] =
net/wireless/wext-compat.c:179:			range->encoding_size[range->num_encoding_sizes++] =

Only one consumer appends — good, the blast radius is one function. Then confirm the array really is fixed-size and uapi-frozen:

$ grep -n "IW_MAX_ENCODING_SIZES" include/uapi/linux/wireless.h
482:#define IW_MAX_ENCODING_SIZES	8

Next, walk the other readers of cipher_suites to see whether the WEXT pattern — append per entry into a bounded destination — exists elsewhere:

$ grep -rn "cipher_suites" net/wireless/ | grep -v wext-compat
net/wireless/nl80211.c:3138:			    sizeof(u32) * rdev->wiphy.n_cipher_suites,
net/wireless/nl80211.c:3139:			    rdev->wiphy.cipher_suites))
net/wireless/nl80211.c:12832:		for (i = 0; i < rdev->wiphy.n_cipher_suites; i++) {
net/wireless/nl80211.c:12833:			if (key.p.cipher == rdev->wiphy.cipher_suites[i]) {
net/wireless/util.c:238:	for (i = 0; i < wiphy->n_cipher_suites; i++)
net/wireless/util.c:239:		if (cipher == wiphy->cipher_suites[i])

The nl80211 site at 3138 copies the raw list into a netlink attribute sized by the same count — internally consistent, no overflow, but it does leak the duplicates to userspace, which strengthened the case for fixing the input rather than each reader. The other sites are membership scans, safe under duplication. So WEXT was the only memory-corrupting consumer, but not the only affected one.

Finally, look at how drivers actually build the list, to sanity-check that rejecting duplicates wouldn't break legitimate hardware. A typical example, from drivers/net/wireless/ath/wcn36xx/main.c:

static const u32 cipher_suites[] = {
	WLAN_CIPHER_SUITE_WEP40,
	WLAN_CIPHER_SUITE_WEP104,
	WLAN_CIPHER_SUITE_TKIP,
	WLAN_CIPHER_SUITE_CCMP,
};
...
wcn->hw->wiphy->cipher_suites = cipher_suites;
wcn->hw->wiphy->n_cipher_suites = ARRAY_SIZE(cipher_suites);

A short static array, hand-written per driver. Duplicates in such a table are never intentional — there is no encoding of "capability" that requires repetition — so a hard rejection breaks no valid use case. With that confirmed, the patch wrote itself: one helper, one call site, and a commit message pointing at the WEXT overflow as the motivating consumer.

Upstream

The patch was first posted on 2026-04-13 and applied by Johannes Berg, landing in mainline on 2026-04-28 as commit 7187d145d904 ("wifi: cfg80211: reject duplicate wiphy cipher suite entries"). It was a fast, quiet review — the fix is small and the reasoning is hard to argue with.

The full credits from the commit message:

Takeaway

Validate invariants where data enters the subsystem. cipher_suites is a set; the moment cfg80211 started enforcing that at wiphy_register(), every consumer — the WEXT shim, the nl80211 dump, and all the code that will be written against this API in the future — got the guarantee for free. One check at the boundary beats N defensive checks scattered across consumers, especially when one of those consumers is a legacy compatibility layer that nobody wants to touch and everybody forgets to audit.