[PATCH] wifi: mac80211: Fix cryptographic MAC comparison to be constant-time

[PATCH] wifi: mac80211: Fix cryptographic MAC comparison to be constant-time

[Eric Biggers]

To prevent timing attacks, the comparison of cryptographic message
authentication codes (MACs) needs to have data-independent timing.
Replace the memcmp() with the correct function, crypto_memneq().

Fixes: 39404feee691 ("mac80211: FILS AEAD protection for station mode association frames")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 net/mac80211/fils_aead.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c
index d2f4a17eab99..00b91e6ed994 100644
--- a/net/mac80211/fils_aead.c
+++ b/net/mac80211/fils_aead.c
@@ -195,7 +195,7 @@ static int aes_siv_decrypt(const u8 *key, size_t key_len,
 	res = aes_s2v(key /* K1 */, key_len, num_elem, addr, len, check);
 	if (res)
 		return res;
-	if (memcmp(check, frame_iv, AES_BLOCK_SIZE) != 0)
+	if (crypto_memneq(check, frame_iv, AES_BLOCK_SIZE))
 		return -EINVAL;
 	return 0;
 }

base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
-- 
2.55.0

Re: [PATCH] wifi: mac80211: Fix cryptographic MAC comparison to be constant-time

[Johannes Berg]

On Wed, 2026-07-08 at 22:44 -0400, Eric Biggers wrote:
> To prevent timing attacks, the comparison of cryptographic message
> authentication codes (MACs) needs to have data-independent timing.
> Replace the memcmp() with the correct function, crypto_memneq().
> 
> Fixes: 39404feee691 ("mac80211: FILS AEAD protection for station mode association frames")
> Cc: stable@vger.kernel.org

I guess I'll apply (a variant of) this patch for -next, but that commit
log really makes it sound like something is actually broken and needs
fixing, and I don't think that's true in this specific context.

What happens is that the frame is validated and then we associate
successfully (upon success) or drop the frame (upon failure). Only the
failure case is relevant for the timing issue, but in that case we
simply drop the frame and there isn't really an observable signal -
nothing else happens, at least not immediately, we may retry the request
later after a timer.

So sure, it looks better to have a crypto_memneq() in AES-SIV related
code, but in practice I don't see how it would make a difference now,
and it's even unlikely this code will ever matter for anything else in
the future, given that things are moving more and more towards full
frame encryption, including association request/response now.

I saw you originally had this in the "use libraries" patch [1], I'm also
good with you just keeping the change there. This might even be better
if you're planning to have this in -next soon, where it would otherwise
conflict if I keep this to -next.

[1] https://lore.kernel.org/linux-crypto/20260707053503.209874-24-ebiggers@kernel.org/

(The whole feature is also fairly much unused anyway in practice as far
as I can tell.)

johannes

Re: [PATCH] wifi: mac80211: Fix cryptographic MAC comparison to be constant-time

[Eric Biggers]

On Thu, Jul 09, 2026 at 08:45:35AM +0200, Johannes Berg wrote:
> On Wed, 2026-07-08 at 22:44 -0400, Eric Biggers wrote:
> > To prevent timing attacks, the comparison of cryptographic message
> > authentication codes (MACs) needs to have data-independent timing.
> > Replace the memcmp() with the correct function, crypto_memneq().
> > 
> > Fixes: 39404feee691 ("mac80211: FILS AEAD protection for station mode association frames")
> > Cc: stable@vger.kernel.org
> 
> I guess I'll apply (a variant of) this patch for -next, but that commit
> log really makes it sound like something is actually broken and needs
> fixing, and I don't think that's true in this specific context.
> 
> What happens is that the frame is validated and then we associate
> successfully (upon success) or drop the frame (upon failure). Only the
> failure case is relevant for the timing issue, but in that case we
> simply drop the frame and there isn't really an observable signal -
> nothing else happens, at least not immediately, we may retry the request
> later after a timer.
> 
> So sure, it looks better to have a crypto_memneq() in AES-SIV related
> code, but in practice I don't see how it would make a difference now,
> and it's even unlikely this code will ever matter for anything else in
> the future, given that things are moving more and more towards full
> frame encryption, including association request/response now.

Surely the other end of the connection could observe a difference in
timing at some point?  Even if only a very small amount and not right
away?  A timing attack doesn't take much.

It's safest to assume that this is a bug fix.

We've gone through similar crypto_memneq() conversions in other
subsystems, where maintainers try to come up with some heuristic
argument that timing attacks don't apply.  Those arguments tend to have
holes.  I'm not sure why people are so eager to risk it.

> I saw you originally had this in the "use libraries" patch [1], I'm also
> good with you just keeping the change there. This might even be better
> if you're planning to have this in -next soon, where it would otherwise
> conflict if I keep this to -next.
> 
> [1] https://lore.kernel.org/linux-crypto/20260707053503.209874-24-ebiggers@kernel.org/

I just figured this is a bug fix worth splitting out.

Only patches 1-13 of that series are intended for 7.3, so this would be
a good time to get any preliminary fixes in to code that is going to get
converted to use the library later.

- Eric