From: <gregkh@linuxfoundation.org>
To: smueller@chronox.de, gregkh@linuxfoundation.org,
herbert@gondor.apana.org.au, romain.izard.pro@gmail.com
Cc: <stable@vger.kernel.org>, <stable-commits@vger.kernel.org>
Subject: Patch "crypto: af_alg - remove locking in async callback" has been added to the 4.14-stable tree
Date: Mon, 04 Dec 2017 11:52:51 +0100 [thread overview]
Message-ID: <151238477198106@kroah.com> (raw)
This is a note to let you know that I've just added the patch titled
crypto: af_alg - remove locking in async callback
to the 4.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
crypto-af_alg-remove-locking-in-async-callback.patch
and it can be found in the queue-4.14 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
>From 7d2c3f54e6f646887d019faa45f35d6fe9fe82ce Mon Sep 17 00:00:00 2001
From: Stephan Mueller <smueller@chronox.de>
Date: Fri, 10 Nov 2017 13:20:55 +0100
Subject: crypto: af_alg - remove locking in async callback
From: Stephan Mueller <smueller@chronox.de>
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>
---
crypto/af_alg.c | 21 ++++++++++++++-------
crypto/algif_aead.c | 23 ++++++++++++-----------
crypto/algif_skcipher.c | 23 ++++++++++++-----------
include/crypto/if_alg.h | 1 +
4 files changed, 39 insertions(+), 29 deletions(-)
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -1048,6 +1048,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
*
* This handler cleans up the struct af_alg_async_req upon completion of the
@@ -1063,18 +1075,13 @@ void af_alg_async_cb(struct crypto_async
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);
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -283,12 +283,23 @@ static int _aead_recvmsg(struct socket *
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 *
&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;
}
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -117,6 +117,7 @@ static int _skcipher_recvmsg(struct sock
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 sock
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 sock
&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;
}
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -255,6 +255,7 @@ int af_alg_sendmsg(struct socket *sock,
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);
Patches currently in stable-queue which might be from smueller@chronox.de are
queue-4.14/crypto-af_alg-remove-locking-in-async-callback.patch
queue-4.14/crypto-algif_aead-skip-sgl-entries-with-null-page.patch
reply other threads:[~2017-12-04 10:52 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=151238477198106@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=herbert@gondor.apana.org.au \
--cc=romain.izard.pro@gmail.com \
--cc=smueller@chronox.de \
--cc=stable-commits@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).