| Message ID | 1486224522-26392-1-git-send-email-jouni@qca.qualcomm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
Hi, > The skcipher could have been of the async variant which may return > from skcipher_encrypt() with -EINPROGRESS after having queued the > request. > The FILS AEAD implementation here does not have code for dealing with > that possibility, so allocate a sync cipher explicitly to avoid > potential issues with hardware accelerators. > - tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0); > + tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, > CRYPTO_ALG_ASYNC); I'll apply this, after having found some code elsewhere that does something similar, but I'll note that this is super confusing, since the only documentation mentioning this flag says: The mask flag restricts the type of cipher. The only allowed flag is CRYPTO_ALG_ASYNC to restrict the cipher lookup function to asynchronous ciphers. Usually, a caller provides a 0 for the mask flag. (I have a vague feeling the first sentence was intended to be documentation for the algorithm *implementation* specifying the flag, and the second for a caller doing a lookup, or something strange?) johannes
On Mon, Feb 06, 2017 at 07:54:37AM +0100, Johannes Berg wrote: > Hi, > > > The skcipher could have been of the async variant which may return > > from skcipher_encrypt() with -EINPROGRESS after having queued the > > request. > > The FILS AEAD implementation here does not have code for dealing with > > that possibility, so allocate a sync cipher explicitly to avoid > > potential issues with hardware accelerators. > > > - tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0); > > + tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, > > CRYPTO_ALG_ASYNC); > > I'll apply this, after having found some code elsewhere that does > something similar, but I'll note that this is super confusing, since > the only documentation mentioning this flag says: > > The mask flag restricts the type of cipher. The only allowed flag is > CRYPTO_ALG_ASYNC to restrict the cipher lookup function to > asynchronous ciphers. Usually, a caller provides a 0 for the mask flag. The type and mask are used as follows when checking an algorithm: alg->type & mask == type & mask So to request a synchronous algorithm (that is, one with the CRYPTO_ALG_ASYNC bit set to zero), you would set type to 0 and mask to CRYPTO_ALG_ASYNC. Cheers,
> The type and mask are used as follows when checking an algorithm: > > alg->type & mask == type & mask > > So to request a synchronous algorithm (that is, one with the > CRYPTO_ALG_ASYNC bit set to zero), you would set type to 0 and > mask to CRYPTO_ALG_ASYNC. Ah. Ok, that makes sense, thanks for the explanation. johannes
diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c index e3bbe24..912f3e2c 100644 --- a/net/mac80211/fils_aead.c +++ b/net/mac80211/fils_aead.c @@ -192,7 +192,7 @@ static int aes_siv_encrypt(const u8 *key, size_t key_len, /* CTR */ - tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0); + tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC); if (IS_ERR(tfm2)) { kfree(tmp); return PTR_ERR(tfm2); @@ -251,7 +251,7 @@ static int aes_siv_decrypt(const u8 *key, size_t key_len, /* CTR */ - tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0); + tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC); if (IS_ERR(tfm2)) return PTR_ERR(tfm2); /* K2 for CTR */
The skcipher could have been of the async variant which may return from skcipher_encrypt() with -EINPROGRESS after having queued the request. The FILS AEAD implementation here does not have code for dealing with that possibility, so allocate a sync cipher explicitly to avoid potential issues with hardware accelerators. This is based on the patch sent out by Ard. Fixes: 39404feee691 ("mac80211: FILS AEAD protection for station mode association frames") Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com> --- net/mac80211/fils_aead.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)