The kernel crypto API requires the caller to set an IV in the request data
structure. That request data structure shall define one particular cipher
operation. During the cipher operation, the IV is read by the cipher
implementation and eventually the potentially updated IV (e.g. in case of
CBC) is written back to the memory location the request data structure
points to.
AF_ALG allows setting the IV with a sendmsg request, where the IV is stored
in the AF_ALG context that is unique to one particular AF_ALG socket. Note
the analogy: an AF_ALG socket is like a TFM where one recvmsg operation
uses one request with the TFM from the socket.
AF_ALG these days supports AIO operations with multiple IOCBs. I.e. with
one recvmsg call, multiple IOVECs can be specified. Each individual IOCB
(derived from one IOVEC) implies that one request data structure is
created with the data to be processed by the cipher implementation. The
IV that was set with the sendmsg call is registered with the request data
structure before the cipher operation.
In case of an AIO operation, the cipher operation invocation returns
immediately, queuing the request to the hardware. While the AIO request is
processed by the hardware, recvmsg processes the next IOVEC for which
another request is created. Again, the IV buffer from the AF_ALG socket
context is registered with the new request and the cipher operation is
invoked.
You may now see that there is a potential race condition regarding the IV
handling, because there is *no* separate IV buffer for the different
requests. This is nicely demonstrated with libkcapi using the following
command which creates an AIO request with two IOCBs each encrypting one
AES block in CBC mode:
kcapi -d 2 -x 9 -e -c "cbc(aes)" -k
8d7dd9b0170ce0b5f2f8e1aa768e01e91da8bfc67fd486d081b28254c99eb423 -i
7fbc02ebf5b93322329df9bfccb635af -p 48981da18e4bb9ef7e2e3162d16b1910
When the first AIO request finishes before the 2nd AIO request is
processed, the returned value is:
8b19050f66582cb7f7e4b6c873819b7108afa0eaa7de29bac7d903576b674c32
I.e. two blocks where the IV output from the first request is the IV input
to the 2nd block.
In case the first AIO request is not completed before the 2nd request
commences, the result is two identical AES blocks (i.e. both use the same
IV):
8b19050f66582cb7f7e4b6c873819b718b19050f66582cb7f7e4b6c873819b71
This inconsistent result may even lead to the conclusion that there can be
a memory corruption in the IV buffer if both AIO requests write to the IV
buffer at the same time.
As the AF_ALG interface is used by user space, a mutex provides the
serialization which puts the caller to sleep in case a previous IOCB
processing is not yet finished.
If multiple IOCBs arrive that all are blocked, the mutex' FIFO operation
of processing arriving requests ensures that the blocked IOCBs are
unblocked in the right order.
CC: <sta...@vger.kernel.org> #4.14
Fixes: e870456d8e7c8 ("crypto: algif_skcipher - overhaul memory management")
Fixes: d887c52d6ae43 ("crypto: algif_aead - overhaul memory management")
Signed-off-by: Stephan Mueller <smuel...@chronox.de>
---
crypto/af_alg.c | 31 +++++++++++++++++++++++++++++++
crypto/algif_aead.c | 20 +++++++++++++-------
crypto/algif_skcipher.c | 12 +++++++++---
include/crypto/if_alg.h | 5 +++++
4 files changed, 58 insertions(+), 10 deletions(-)
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 5231f421ad00..e7887621aa44 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -1051,6 +1051,8 @@ void af_alg_async_cb(struct crypto_async_request *_req,
int err)
struct kiocb *iocb = areq->iocb;
unsigned int resultlen;
+ af_alg_put_iv(sk);
+
/* Buffer size written by crypto operation. */
resultlen = areq->outlen;
@@ -1175,6 +1177,35 @@ int af_alg_get_rsgl(struct sock *sk, struct msghdr *msg,
int flags,
}
EXPORT_SYMBOL_GPL(af_alg_get_rsgl);
+/**
+ * af_alg_get_iv
+ *
+ * @sk [in] AF_ALG socket
+ * @return 0 on success, < 0 on error
+ */
+int af_alg_get_iv(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct af_alg_ctx *ctx = ask->private;
+
+ return mutex_lock_interruptible(&ctx->ivlock);
+}
+EXPORT_SYMBOL_GPL(af_alg_get_iv);
+
+/**
+ * af_alg_put_iv - release lock on IV in case CTX IV is used
+ *
+ * @sk [in] AF_ALG socket
+ */
+void af_alg_put_iv(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct af_alg_ctx *ctx = ask->private;
+
+ mutex_unlock(&ctx->ivlock);
+}
+EXPORT_SYMBOL_GPL(af_alg_put_iv);
+
static int __init af_alg_init(void)
{
int err = proto_register(&alg_proto, 0);
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 4b07edd5a9ff..402de50d4a58 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -159,10 +159,14 @@ static int _aead_recvmsg(struct socket *sock, struct
msghdr *msg,
if (IS_ERR(areq))
return PTR_ERR(areq);
+ err = af_alg_get_iv(sk);
+ if (err)
+ goto free;
+
/* convert iovecs of output buffers into RX SGL */
err = af_alg_get_rsgl(sk, msg, flags, areq, outlen, &usedpages);
if (err)
- goto free;
+ goto unlock;
/*
* Ensure output buffer is sufficiently large. If the caller provides
@@ -176,7 +180,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr
*msg,
if (used < less) {
err = -EINVAL;
- goto free;
+ goto unlock;
}
used -= less;
outlen -= less;
@@ -197,7 +201,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr
*msg,
}
if (processed && !tsgl_src) {
err = -EFAULT;
- goto free;
+ goto unlock;
}
/*
@@ -230,7 +234,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr
*msg,
err = crypto_aead_copy_sgl(null_tfm, tsgl_src,
areq->first_rsgl.sgl.sg, processed);
if (err)
- goto free;
+ goto unlock;
af_alg_pull_tsgl(sk, processed, NULL, 0);
} else {
/*
@@ -248,7 +252,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr
*msg,
err = crypto_aead_copy_sgl(null_tfm, tsgl_src,
areq->first_rsgl.sgl.sg, outlen);
if (err)
- goto free;
+ goto unlock;
/* Create TX SGL for tag and chain it to RX SGL. */
areq->tsgl_entries = af_alg_count_tsgl(sk, processed,
@@ -260,7 +264,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr
*msg,
GFP_KERNEL);
if (!areq->tsgl) {
err = -ENOMEM;
- goto free;
+ goto unlock;
}
sg_init_table(areq->tsgl, areq->tsgl_entries);
@@ -316,7 +320,8 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr
*msg,
&ctx->wait);
}
-
+unlock:
+ af_alg_put_iv(sk);
free:
af_alg_free_resources(areq);
@@ -562,6 +567,7 @@ static int aead_accept_parent_nokey(void *private, struct
sock *sk)
return -ENOMEM;
}
memset(ctx->iv, 0, ivlen);
+ mutex_init(&ctx->ivlock);
INIT_LIST_HEAD(&ctx->tsgl_list);
ctx->len = len;
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index c88e5e4cd6a6..b65b9a89d6bb 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -77,10 +77,14 @@ static int _skcipher_recvmsg(struct socket *sock, struct
msghdr *msg,
if (IS_ERR(areq))
return PTR_ERR(areq);
+ err = af_alg_get_iv(sk);
+ if (err)
+ goto free;
+
/* convert iovecs of output buffers into RX SGL */
err = af_alg_get_rsgl(sk, msg, flags, areq, -1, &len);
if (err)
- goto free;
+ goto unlock;
/* Process only as much RX buffers for which we have TX data */
if (len > ctx->used)
@@ -104,7 +108,7 @@ static int _skcipher_recvmsg(struct socket *sock, struct
msghdr *msg,
GFP_KERNEL);
if (!areq->tsgl) {
err = -ENOMEM;
- goto free;
+ goto unlock;
}
sg_init_table(areq->tsgl, areq->tsgl_entries);
af_alg_pull_tsgl(sk, len, areq->tsgl, 0);
@@ -146,7 +150,8 @@ static int _skcipher_recvmsg(struct socket *sock, struct
msghdr *msg,
&ctx->wait);
}
-
+unlock:
+ af_alg_put_iv(sk);
free:
af_alg_free_resources(areq);
@@ -353,6 +358,7 @@ static int skcipher_accept_parent_nokey(void *private,
struct sock *sk)
}
memset(ctx->iv, 0, crypto_skcipher_ivsize(tfm));
+ mutex_init(&ctx->ivlock);
INIT_LIST_HEAD(&ctx->tsgl_list);
ctx->len = len;
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index f38227a78eae..c48eff27e5a9 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -19,6 +19,7 @@
#include <linux/scatterlist.h>
#include <linux/types.h>
#include <linux/atomic.h>
+#include <linux/mutex.h>
#include <net/sock.h>
#include <crypto/aead.h>
@@ -127,6 +128,7 @@ struct af_alg_async_req {
*
* @tsgl_list: Link to TX SGL
* @iv: IV for cipher operation
+ * @ivlock: Mutex to serialize access to the IV
* @aead_assoclen: Length of AAD for AEAD cipher operations
* @completion: Work queue for synchronous operation
* @used: TX bytes sent to kernel. This variable is used to
@@ -146,6 +148,7 @@ struct af_alg_ctx {
struct list_head tsgl_list;
void *iv;
+ struct mutex ivlock;
size_t aead_assoclen;
struct crypto_wait wait;
@@ -252,5 +255,7 @@ struct af_alg_async_req *af_alg_alloc_areq(struct sock *sk,
int af_alg_get_rsgl(struct sock *sk, struct msghdr *msg, int flags,
struct af_alg_async_req *areq, size_t maxsize,
size_t *outlen);
+int af_alg_get_iv(struct sock *sk);
+void af_alg_put_iv(struct sock *sk);
#endif /* _CRYPTO_IF_ALG_H */
--
2.14.3
