Issue with AIO + async skcipher + IV chaining

[WOnder93]

On a machine with Intel QAT accelerator, which provides an async implementation of cbc(aes), the following libkcapi tests fail:

[FAILED: open - 5.6.13-7190cc7.cki] Symmetric asynchronous cipher one shot multiple test
(./libkcapi/test/../bin/kcapi  -d 2 -x 9  -e -c cbc(aes) -k 8d7dd9b0170ce0b5f2f8e1aa768e01e91da8bfc67fd486d081b28254c99eb423 -i 7fbc02ebf5b93322329df9bfccb635af -p 48981da18e4bb9ef7e2e3162d16b1910)
Exp 8b19050f66582cb7f7e4b6c873819b7108afa0eaa7de29bac7d903576b674c32
Got 8b19050f66582cb7f7e4b6c873819b718b19050f66582cb7f7e4b6c873819b71
[FAILED: open - 5.6.13-7190cc7.cki] Symmetric asynchronous cipher stream multiple test
(./libkcapi/test/../bin/kcapi -s -d 2 -x 9  -e -c cbc(aes) -k 8d7dd9b0170ce0b5f2f8e1aa768e01e91da8bfc67fd486d081b28254c99eb423 -i 7fbc02ebf5b93322329df9bfccb635af -p 48981da18e4bb9ef7e2e3162d16b1910)
Exp 8b19050f66582cb7f7e4b6c873819b7108afa0eaa7de29bac7d903576b674c32
Got 8b19050f66582cb7f7e4b6c873819b718b19050f66582cb7f7e4b6c873819b71
[FAILED: open - 5.6.13-7190cc7.cki] Symmetric asynchronous cipher vmsplice multiple test
(./libkcapi/test/../bin/kcapi -v -d 2 -x 9  -e -c cbc(aes) -k 8d7dd9b0170ce0b5f2f8e1aa768e01e91da8bfc67fd486d081b28254c99eb423 -i 7fbc02ebf5b93322329df9bfccb635af -p 48981da18e4bb9ef7e2e3162d16b1910)
Exp 8b19050f66582cb7f7e4b6c873819b7108afa0eaa7de29bac7d903576b674c32
Got 8b19050f66582cb7f7e4b6c873819b718b19050f66582cb7f7e4b6c873819b71

It seems that libkcapi sends two parallel AIO requests, yet expects them to be executed serially (i.e. chain the IV properly). And this is indeed what happens when the implementation is synchronous. However, when it is asynchronous (i.e. it actually returns -EINPROGRESS), the second request can start before the first one completes, so it is processed with the same IV.

This is the crypto driver's entry from /proc/crypto:

name         : cbc(aes)
driver       : qat_aes_cbc
module       : intel_qat
priority     : 4001
refcnt       : 1
selftest     : passed
internal     : no
type         : skcipher
async        : yes
blocksize    : 16
min keysize  : 16
max keysize  : 32
ivsize       : 16
chunksize    : 16
walksize     : 16

I think it should be possible to reproduce this also with pcrypt(...), but I didn't manage to get it working...

I'm not sure it is correct to execute multiple parallel requests in _kcapi_cipher_crypt_aio() - it seems to me that it causes a logical race condition, unless the cipher is ecb(...). However, I don't have a deep understanding of this AIO stuff, so I could be mistaken...

[smuellerDD]

You are absolutely correct, parallel AIO requests on the same cipher handle are undefined because the driver may choose to either serialize the request or perform parallel operations with separate states. Only for non-AIO drivers (e.g. the C implementations or AESNI) that are invoked via the AIO interface, the requests will be serialized.

Due to this undefined behavior of AIO, I disabled the test for parallel AIO.

Thanks.

[smuellerDD]

Ondrej, what do you think about the following: it is clear that the parallel execution of AIO requests leads to undefined behaivor. Yet, there are only two possible solutions: either the driver serializes the requests and thus performs an implicit chaining of the requests or the driver handles both requests totally independent.

That said, I only see that either result https://github.com/smuellerDD/libkcapi/blob/master/test/test.sh#L1260 or result https://github.com/smuellerDD/libkcapi/blob/master/test/test.sh#L1267 (without the CR in the middle) is possible.

Thus, what about reenabling the test and allowing both results as valid? The goal of the test is not to verify that the cipher operation is correct but that the entire code path is able to handle such undefined yet possible requests.

[smuellerDD]

I have reenabled the test with the two results as mentioned above. Please let me know if it works. Thanks.

[WOnder93]

I don't know, the libkcapi AIO encrypt/decrypt functions seem badly designed to me... The behavior is undefined for pretty much any skcipher but ecb(...) if you pass more han one iov, and the fact that the AEAD variant doe separate op for each iov is also strange (what if I want to do a signle op with 2+ iovs?). I have a better implementation of _kcapi_cipher_crypt_aio in mind, but it wouldn't be compatible with the current behavior for AEAD (and possibly other cipher types).

So I'd say testing for the undefined behavior is slightly better than disabling the tests entirely, but in the long term I think the libkcapi's approach to AIO needs some overhaul...