From: Harald Freudenberger <freude@linux.vnet.ibm.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org,
Martin Schwidefsky <schwidefsky@de.ibm.com>,
Ingo Tuchscherer <ingo.tuchscherer@linux.vnet.ibm.com>,
Gerald Schaefer <gerald.schaefer@de.ibm.com>,
Harald Freudenberger <freude@de.ibm.com>,
Harald Freudenberger <freude@linux.vnet.ibm.com>
Subject: [PATCH] s390/crypto: fix aes ctr concurrency issue
Date: Tue, 19 Nov 2013 11:22:12 +0100 [thread overview]
Message-ID: <1384856532-17247-2-git-send-email-freude@linux.vnet.ibm.com> (raw)
In-Reply-To: <1384856532-17247-1-git-send-email-freude@linux.vnet.ibm.com>
The aes-ctr mode used one preallocated page without any concurrency
protection. When multiple threads run aes-ctr encryption or decryption
this could lead to data corruption.
The patch introduces locking for the preallocated page and alternatively
allocating and freeing of an temp page in concurrency situations.
---
arch/s390/crypto/aes_s390.c | 55 ++++++++++++++++++++++++++++++++----------
1 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c
index 4363528..fcb5297 100644
--- a/arch/s390/crypto/aes_s390.c
+++ b/arch/s390/crypto/aes_s390.c
@@ -25,6 +25,7 @@
#include <linux/err.h>
#include <linux/module.h>
#include <linux/init.h>
+#include <linux/mutex.h>
#include "crypt_s390.h"
#define AES_KEYLEN_128 1
@@ -32,6 +33,7 @@
#define AES_KEYLEN_256 4
static u8 *ctrblk;
+static DEFINE_MUTEX(ctrblk_lock);
static char keylen_flag;
struct s390_aes_ctx {
@@ -762,11 +764,25 @@ static int ctr_aes_crypt(struct blkcipher_desc *desc, long func,
unsigned int i, n, nbytes;
u8 buf[AES_BLOCK_SIZE];
u8 *out, *in;
+ u8 *ctrpage;
if (!walk->nbytes)
return ret;
- memcpy(ctrblk, walk->iv, AES_BLOCK_SIZE);
+ if (mutex_trylock(&ctrblk_lock)) {
+ /* ctrblk is now reserved for us */
+ ctrpage = ctrblk;
+ } else {
+ /* ctrblk is in use by someone else, alloc our own page */
+ ctrpage = (u8*) __get_free_page(GFP_ATOMIC);
+ if (!ctrpage) {
+ /* gfp failed, wait until ctrblk becomes available */
+ mutex_lock(&ctrblk_lock);
+ ctrpage = ctrblk;
+ }
+ }
+
+ memcpy(ctrpage, walk->iv, AES_BLOCK_SIZE);
while ((nbytes = walk->nbytes) >= AES_BLOCK_SIZE) {
out = walk->dst.virt.addr;
in = walk->src.virt.addr;
@@ -775,17 +791,19 @@ static int ctr_aes_crypt(struct blkcipher_desc *desc, long func,
n = (nbytes > PAGE_SIZE) ? PAGE_SIZE :
nbytes & ~(AES_BLOCK_SIZE - 1);
for (i = AES_BLOCK_SIZE; i < n; i += AES_BLOCK_SIZE) {
- memcpy(ctrblk + i, ctrblk + i - AES_BLOCK_SIZE,
+ memcpy(ctrpage + i, ctrpage + i - AES_BLOCK_SIZE,
AES_BLOCK_SIZE);
- crypto_inc(ctrblk + i, AES_BLOCK_SIZE);
+ crypto_inc(ctrpage + i, AES_BLOCK_SIZE);
+ }
+ ret = crypt_s390_kmctr(func, sctx->key, out, in, n, ctrpage);
+ if (ret < 0 || ret != n) {
+ ret = -EIO;
+ goto out;
}
- ret = crypt_s390_kmctr(func, sctx->key, out, in, n, ctrblk);
- if (ret < 0 || ret != n)
- return -EIO;
if (n > AES_BLOCK_SIZE)
- memcpy(ctrblk, ctrblk + n - AES_BLOCK_SIZE,
+ memcpy(ctrpage, ctrpage + n - AES_BLOCK_SIZE,
AES_BLOCK_SIZE);
- crypto_inc(ctrblk, AES_BLOCK_SIZE);
+ crypto_inc(ctrpage, AES_BLOCK_SIZE);
out += n;
in += n;
nbytes -= n;
@@ -799,14 +817,25 @@ static int ctr_aes_crypt(struct blkcipher_desc *desc, long func,
out = walk->dst.virt.addr;
in = walk->src.virt.addr;
ret = crypt_s390_kmctr(func, sctx->key, buf, in,
- AES_BLOCK_SIZE, ctrblk);
- if (ret < 0 || ret != AES_BLOCK_SIZE)
- return -EIO;
+ AES_BLOCK_SIZE, ctrpage);
+ if (ret < 0 || ret != AES_BLOCK_SIZE) {
+ ret = -EIO;
+ goto out;
+ }
memcpy(out, buf, nbytes);
- crypto_inc(ctrblk, AES_BLOCK_SIZE);
+ crypto_inc(ctrpage, AES_BLOCK_SIZE);
ret = blkcipher_walk_done(desc, walk, 0);
}
- memcpy(walk->iv, ctrblk, AES_BLOCK_SIZE);
+ memcpy(walk->iv, ctrpage, AES_BLOCK_SIZE);
+
+out:
+ if (ctrpage == ctrblk) {
+ /* free the reservation for ctrblk now */
+ mutex_unlock(&ctrblk_lock);
+ } else {
+ /* free the page allocated above */
+ free_page((unsigned long) ctrpage);
+ }
return ret;
}
--
1.7.0.4
next prev parent reply other threads:[~2013-11-19 10:22 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-19 10:22 [PATCH] s390/crypto: fix aes ctr concurrency issue Harald Freudenberger
2013-11-19 10:22 ` Harald Freudenberger [this message]
2013-11-28 14:00 ` Herbert Xu
2013-11-28 15:39 ` Harald Freudenberger
2013-11-29 1:50 ` Herbert Xu
2013-11-29 8:58 ` Harald Freudenberger
2013-11-22 13:58 ` Gerald Schaefer
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=1384856532-17247-2-git-send-email-freude@linux.vnet.ibm.com \
--to=freude@linux.vnet.ibm.com \
--cc=freude@de.ibm.com \
--cc=gerald.schaefer@de.ibm.com \
--cc=herbert@gondor.apana.org.au \
--cc=ingo.tuchscherer@linux.vnet.ibm.com \
--cc=linux-crypto@vger.kernel.org \
--cc=schwidefsky@de.ibm.com \
/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