Thread (19 messages) 19 messages, 5 authors, 2018-09-06

[PATCH 2/2] crypto: skcipher: Remove VLA usage for SKCIPHER_REQUEST_ON_STACK

From: gilad@benyossef.com (Gilad Ben-Yossef)
Date: 2018-09-06 08:25:07
Also in: linux-crypto, lkml

On Thu, Sep 6, 2018 at 10:21 AM, Ard Biesheuvel
[off-list ref] wrote:
quoted
quoted
The skcipher implementations based on crypto IP blocks are typically
asynchronous, and I wouldn't be surprised if a fair number of
SKCIPHER_REQUEST_ON_STACK() users are limited to synchronous
skciphers.
According to Herbert, SKCIPHER_REQUEST_ON_STACK() may only be used
for invoking synchronous ciphers.

In fact, due to the way the crypto API is built, if you try using it
with any transformation that uses DMA
you would most probably end up trying to DMA to/from the stack which
as we all know is not a great idea.
Ah yes, I found [0] which contains that quote.

So that means that Kees can disregard the occurrences that are async
only, but it still implies that we cannot limit the reqsize like he
proposes unless we take the sync/async nature into account.
It also means we should probably BUG() or WARN() in
SKCIPHER_REQUEST_ON_STACK() when used with an async algo.
quoted
quoted
So we could formalize this and limit SKCIPHER_REQUEST_ON_STACK() to
synchronous skciphers, which implies that the reqsize limit only has
to apply synchronous skciphers as well. But before we can do this, we
have to identify the remaining occurrences that allow asynchronous
skciphers to be used, and replace them with heap allocations.
Any such occurrences are almost for sure broken already due to the DMA
issue I've mentioned.
I am not convinced of this. The skcipher request struct does not
contain any payload buffers, and whether the algo specific ctx struct
is used for DMA is completely up to the driver. So I am quite sure
there are plenty of async algos that work fine with
SKCIPHER_REQUEST_ON_STACK() and vmapped stacks.

You are right that it is up to the driver but the cost is an extra
memory allocation and release
*per request* for any per request data that needs to be DMAable beyond
the actual plain
and cipher text buffers such as the IV, so driver writers have an
incentive against doing that :-)

Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

values of ? will give rise to dom!
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help