crypto: af_alg - remove locking in async callback

commit 7d2c3f54e6f646887d019faa45f35d6fe9fe82ce upstream.

The code paths protected by the socket-lock do not use or modify the
socket in a non-atomic fashion. The actions pertaining the socket do not
even need to be handled as an atomic operation. Thus, the socket-lock
can be safely ignored.

This fixes a bug regarding scheduling in atomic as the callback function
may be invoked in interrupt context.

In addition, the sock_hold is moved before the AIO encrypt/decrypt
operation to ensure that the socket is always present. This avoids a
tiny race window where the socket is unprotected and yet used by the AIO
operation.

Finally, the release of resources for a crypto operation is moved into a
common function of af_alg_free_resources.

Fixes: e870456d8e7c8 ("crypto: algif_skcipher - overhaul memory management")
Fixes: d887c52d6ae43 ("crypto: algif_aead - overhaul memory management")
Reported-by: Romain Izard <romain.izard.pro@gmail.com>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
Tested-by: Romain Izard <romain.izard.pro@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 337cf382718ee3cd81ffe3cdc480052a6f634798..a72659f452a5bbb95612601aa374f69a8c72fe01 100644 (file)
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -1047,6 +1047,18 @@ unlock:
 }
 EXPORT_SYMBOL_GPL(af_alg_sendpage);
 
+/**
+ * af_alg_free_resources - release resources required for crypto request
+ */
+void af_alg_free_resources(struct af_alg_async_req *areq)
+{
+       struct sock *sk = areq->sk;
+
+       af_alg_free_areq_sgls(areq);
+       sock_kfree_s(sk, areq, areq->areqlen);
+}
+EXPORT_SYMBOL_GPL(af_alg_free_resources);
+
 /**
  * af_alg_async_cb - AIO callback handler
  *
@@ -1063,18 +1075,13 @@ void af_alg_async_cb(struct crypto_async_request *_req, int err)
        struct kiocb *iocb = areq->iocb;
        unsigned int resultlen;
 
-       lock_sock(sk);
-
        /* Buffer size written by crypto operation. */
        resultlen = areq->outlen;
 
-       af_alg_free_areq_sgls(areq);
-       sock_kfree_s(sk, areq, areq->areqlen);
-       __sock_put(sk);
+       af_alg_free_resources(areq);
+       sock_put(sk);
 
        iocb->ki_complete(iocb, err ? err : resultlen, 0);
-
-       release_sock(sk);
 }
 EXPORT_SYMBOL_GPL(af_alg_async_cb);
 

diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index ac8f101ab29c658a294e85ed9c4f80e8a6719a74..d0b45145cb307b6c77d9de5dc53697d5e130ce8e 100644 (file)
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -283,12 +283,23 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
 
        if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
                /* AIO operation */
+               sock_hold(sk);
                areq->iocb = msg->msg_iocb;
                aead_request_set_callback(&areq->cra_u.aead_req,
                                          CRYPTO_TFM_REQ_MAY_BACKLOG,
                                          af_alg_async_cb, areq);
                err = ctx->enc ? crypto_aead_encrypt(&areq->cra_u.aead_req) :
                                 crypto_aead_decrypt(&areq->cra_u.aead_req);
+
+               /* AIO operation in progress */
+               if (err == -EINPROGRESS || err == -EBUSY) {
+                       /* Remember output size that will be generated. */
+                       areq->outlen = outlen;
+
+                       return -EIOCBQUEUED;
+               }
+
+               sock_put(sk);
        } else {
                /* Synchronous operation */
                aead_request_set_callback(&areq->cra_u.aead_req,
@@ -300,19 +311,9 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
                                                 &ctx->completion);
        }
 
-       /* AIO operation in progress */
-       if (err == -EINPROGRESS) {
-               sock_hold(sk);
-
-               /* Remember output size that will be generated. */
-               areq->outlen = outlen;
-
-               return -EIOCBQUEUED;
-       }
 
 free:
-       af_alg_free_areq_sgls(areq);
-       sock_kfree_s(sk, areq, areq->areqlen);
+       af_alg_free_resources(areq);
 
        return err ? err : outlen;
 }

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 8ae4170aaeb4f2ef533f7461b81cd8b828879dd5..30ee2a8e8f42929b0a2eb92b370ec3fdac03eb53 100644 (file)
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -117,6 +117,7 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
 
        if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
                /* AIO operation */
+               sock_hold(sk);
                areq->iocb = msg->msg_iocb;
                skcipher_request_set_callback(&areq->cra_u.skcipher_req,
                                              CRYPTO_TFM_REQ_MAY_SLEEP,
@@ -124,6 +125,16 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
                err = ctx->enc ?
                        crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) :
                        crypto_skcipher_decrypt(&areq->cra_u.skcipher_req);
+
+               /* AIO operation in progress */
+               if (err == -EINPROGRESS || err == -EBUSY) {
+                       /* Remember output size that will be generated. */
+                       areq->outlen = len;
+
+                       return -EIOCBQUEUED;
+               }
+
+               sock_put(sk);
        } else {
                /* Synchronous operation */
                skcipher_request_set_callback(&areq->cra_u.skcipher_req,
@@ -137,19 +148,9 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
                                                 &ctx->completion);
        }
 
-       /* AIO operation in progress */
-       if (err == -EINPROGRESS) {
-               sock_hold(sk);
-
-               /* Remember output size that will be generated. */
-               areq->outlen = len;
-
-               return -EIOCBQUEUED;
-       }
 
 free:
-       af_alg_free_areq_sgls(areq);
-       sock_kfree_s(sk, areq, areq->areqlen);
+       af_alg_free_resources(areq);
 
        return err ? err : len;
 }

diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 75ec9c662268bc6d0fcd1226116b4faa7469c0e5..aeec003a566b56e794a575493575a73c7942fb59 100644 (file)
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -255,6 +255,7 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
                   unsigned int ivsize);
 ssize_t af_alg_sendpage(struct socket *sock, struct page *page,
                        int offset, size_t size, int flags);
+void af_alg_free_resources(struct af_alg_async_req *areq);
 void af_alg_async_cb(struct crypto_async_request *_req, int err);
 unsigned int af_alg_poll(struct file *file, struct socket *sock,
                         poll_table *wait);