Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)

[Johannes Berg]

> ÂÂÂ1. revert that patch (doing so would need some major adjustments now,
> ÂÂÂÂÂÂsince it's pretty old and a number of new things were added in the
> ÂÂÂÂÂÂmeantime)

This it will have to be, I guess.

> ÂÂÂ2. allocate a per-CPU buffer for all the things that we put on the
> ÂÂÂÂÂÂstack and use in SG lists, those are:
> ÂÂÂÂÂÂÂ* CCM/GCM: AAD (32B), B_0/J_0 (16B)
> ÂÂÂÂÂÂÂ* GMAC: AAD (20B), zero (16B)
> ÂÂÂÂÂÂÂ* (not sure why CMAC isn't using this API, but it would be likeÂGMAC)

This doesn't work - I tried to move the mac80211 buffers, but because
we also put theÂstruct aead_request on the stack, and crypto_ccm has
the "odata" in there, and we can't separate the odata from that struct,
we'd have to also put that into a per-CPU buffer, but it's very big -
456 bytes for CCM, didn't measure the others but I'd expect them to be
larger, if different.

I don't think we can allocate half a kb for each CPU just to be able to
possibly use the acceleration here. We can't even make that conditional
on not having hardware crypto in the wifi NIC because drivers are
always allowed to pass undecrypted frames, regardless of whether or not
HW crypto was attempted, so we don't know upfront if we'll have to
decrypt anything in software...

Given that, I think we have had a bug in here basically since Ard's
patch, we never should've put these structs on the stack. Herbert, you
also touched this later and converted the API usage, did you see the
way the stack is used here and think it should be OK, or did you simply
not realize that?

Ard, are you able to help out working on a revert of your patch? That
would require also reverting a number of other patches (various fixes,
API adjustments, etc. to the AEAD usage), but the more complicated part
is that in the meantime Jouni introduced GCMP and CCMP-256, both of
which we of course need to retain.

johannes