Re: [PATCH v4] mac80211: move extra crypto data off the stack

On 17 October 2016 at 14:16, Johannes Berg [off-list ref] wrote:

On Mon, 2016-10-17 at 14:06 +0100, Ard Biesheuvel wrote:

quoted

Actually, while I think it will be worthwhile going forward to
implement such an 'auxiliary data' feature in a generic way, I still
think we should address the issue at hand without too much
complication.

If we pedal back to the version of 'mac80211: move struct aead_req
off the stack' that uses kzalloc() instead of aead_request_alloc(),
we can simply add some space for aad[] and/or zero[], and get rid of
the kmem cache entirely.

If you're past this point already, i won't bother but otherwise I can
rework 'mac80211: move struct aead_req off the stack' so that the
other patch is no longer required (and IIRC, this is actually
something you proposed yourself a couple of iterations ago?)

Yes, I did consider that.

It makes some sense, and I guess the extra memcpy() would be cheaper
than the extra alloc?

I'd happily use that instead of the combination of my two patches. The
aead_request_alloc() is just a simple inline anyway, so no real problem
not using it.

Indeed. And it keeps the clutter inside the aes_xxx.c files, which
could easily be updated in the future to use some auxdata feature if
it ever materializes.

I think it would help this code, but also the ESP code you pointed
out, to have some kind of 'ordered synchronous' CRYPTO_xxx flag, where
the crypto API could manage the kmem cache and percpu pointers to
allocations. This goes well beyond what we can do as a fix, though, so
we need an intermediate solution in any case.

Shall I propose the patch?

Indeed. And it keeps the clutter inside the aes_xxx.c files, which
could easily be updated in the future to use some auxdata feature if
it ever materializes.

I think it would help this code, but also the ESP code you pointed
out, to have some kind of 'ordered synchronous' CRYPTO_xxx flag,
where the crypto API could manage the kmem cache and percpu pointers
to allocations.

Yeah, could be useful to have that more generally.

 This goes well beyond what we can do as a fix, though, so we need an
intermediate solution in any case.

Shall I propose the patch?

I assume you mean a mac80211 patch - sure, I'll take that instead of
the two I have now.

johannes