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@xxxxxxxxxxxxxxx
> 
> 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@xxxxxxxxxx/

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