Hi Herbert,
While testing the algif_aead async patch, I have rerun the async
algif_skcipher tests and I have found some problems.
There are three different issues around algif_skcipher and skcipher.
Two are skcipher conversion related, and one is a bug in the
algif_skcipher, not related to the conversion. It started to
manifest itself after a fix the the qat driver.
---
Tadeusz Struk (3):
crypto: skcipher - return the correct request to the user
crypto: algif_skcipher - fix async callback after skcipher convertion
crypto: algif_skcipher - fix increase private contex space for async request
crypto/algif_skcipher.c | 11 +++++++----
crypto/skcipher.c | 11 ++++++++++-
2 files changed, 17 insertions(+), 5 deletions(-)
--
A user of the skcipher api may have some private context associated with
a request, like for instance the algif_skcipher does, so the api needs to
return the original skcipher_request in the callback instead of the
ablkcipher_request subtype.
Cc: <[email protected]> # 4.4.x-
Signed-off-by: Tadeusz Struk <[email protected]>
---
crypto/skcipher.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/crypto/skcipher.c b/crypto/skcipher.c
index 69230e9..a9d54c0 100644
--- a/crypto/skcipher.c
+++ b/crypto/skcipher.c
@@ -142,6 +142,15 @@ static int skcipher_setkey_ablkcipher(struct crypto_skcipher *tfm,
return err;
}
+static void skcipher_complete(struct crypto_async_request *req_base, int err)
+{
+ void *subreq = ablkcipher_request_cast(req_base);
+ struct skcipher_request *req =
+ container_of(subreq, struct skcipher_request, __ctx);
+
+ req->base.complete(&req->base, err);
+}
+
static int skcipher_crypt_ablkcipher(struct skcipher_request *req,
int (*crypt)(struct ablkcipher_request *))
{
@@ -151,7 +160,7 @@ static int skcipher_crypt_ablkcipher(struct skcipher_request *req,
ablkcipher_request_set_tfm(subreq, *ctx);
ablkcipher_request_set_callback(subreq, skcipher_request_flags(req),
- req->base.complete, req->base.data);
+ skcipher_complete, req->base.data);
ablkcipher_request_set_crypt(subreq, req->src, req->dst, req->cryptlen,
req->iv);
Before the skcipher conversion the async callback used the
crypto_async_request directly as the ablkcipher_request.
It wasn't quite correct, but it worked fine since the
crypto_async_request *base was the first member of the ablkcipher_request
struct. After the skcipher conversion it is not the case anymore and
a proper cast helper needs to be used.
Cc: <[email protected]> # 4.4.x-
Signed-off-by: Tadeusz Struk <[email protected]>
---
crypto/algif_skcipher.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 38c1aa8..78529ce 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -100,11 +100,12 @@ static void skcipher_free_async_sgls(struct skcipher_async_req *sreq)
kfree(sreq->tsg);
}
-static void skcipher_async_cb(struct crypto_async_request *req, int err)
+static void skcipher_async_cb(struct crypto_async_request *_req, int err)
{
- struct sock *sk = req->data;
+ struct sock *sk = _req->data;
struct alg_sock *ask = alg_sk(sk);
struct skcipher_ctx *ctx = ask->private;
+ struct skcipher_request *req = skcipher_request_cast(_req);
struct skcipher_async_req *sreq = GET_SREQ(req, ctx);
struct kiocb *iocb = sreq->iocb;
The private context space that was allocated for the async request was too
small. It accidentally worked fine (at least with the qat driver) because
the qat driveri, by mistake, allocated too much.
The problem started to show up since the qat driver issue has been fixed in
commit: 7768fb2ee.
We also need another version of this patch applicable to the code before
skcipher conversion that adds sizeof(struct ablkcipher_request)
Cc: <[email protected]> # 4.4.x-
Signed-off-by: Tadeusz Struk <[email protected]>
---
crypto/algif_skcipher.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 78529ce..298605d 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -69,10 +69,12 @@ struct skcipher_async_req {
};
#define GET_SREQ(areq, ctx) (struct skcipher_async_req *)((char *)areq + \
- crypto_skcipher_reqsize(crypto_skcipher_reqtfm(&ctx->req)))
+ crypto_skcipher_reqsize(crypto_skcipher_reqtfm(&ctx->req)) + \
+ sizeof(struct skcipher_request))
#define GET_REQ_SIZE(ctx) \
- crypto_skcipher_reqsize(crypto_skcipher_reqtfm(&ctx->req))
+ crypto_skcipher_reqsize(crypto_skcipher_reqtfm(&ctx->req)) + \
+ sizeof(struct skcipher_request)
#define GET_IV_SIZE(ctx) \
crypto_skcipher_ivsize(crypto_skcipher_reqtfm(&ctx->req))
On Wed, Jan 27, 2016 at 09:13:56AM -0800, Tadeusz Struk wrote:
>
> diff --git a/crypto/skcipher.c b/crypto/skcipher.c
> index 69230e9..a9d54c0 100644
> --- a/crypto/skcipher.c
> +++ b/crypto/skcipher.c
> @@ -142,6 +142,15 @@ static int skcipher_setkey_ablkcipher(struct crypto_skcipher *tfm,
> return err;
> }
>
> +static void skcipher_complete(struct crypto_async_request *req_base, int err)
> +{
> + void *subreq = ablkcipher_request_cast(req_base);
> + struct skcipher_request *req =
> + container_of(subreq, struct skcipher_request, __ctx);
> +
> + req->base.complete(&req->base, err);
> +}
> +
> static int skcipher_crypt_ablkcipher(struct skcipher_request *req,
> int (*crypt)(struct ablkcipher_request *))
> {
> @@ -151,7 +160,7 @@ static int skcipher_crypt_ablkcipher(struct skcipher_request *req,
>
> ablkcipher_request_set_tfm(subreq, *ctx);
> ablkcipher_request_set_callback(subreq, skcipher_request_flags(req),
> - req->base.complete, req->base.data);
> + skcipher_complete, req->base.data);
> ablkcipher_request_set_crypt(subreq, req->src, req->dst, req->cryptlen,
> req->iv);
Rather than playing with lots of casts, just set the ablkcipher
callback data to req and be done with it.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Wed, Jan 27, 2016 at 09:14:01AM -0800, Tadeusz Struk wrote:
> Before the skcipher conversion the async callback used the
> crypto_async_request directly as the ablkcipher_request.
> It wasn't quite correct, but it worked fine since the
> crypto_async_request *base was the first member of the ablkcipher_request
> struct. After the skcipher conversion it is not the case anymore and
> a proper cast helper needs to be used.
>
> Cc: <[email protected]> # 4.4.x-
> Signed-off-by: Tadeusz Struk <[email protected]>
> ---
> crypto/algif_skcipher.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> index 38c1aa8..78529ce 100644
> --- a/crypto/algif_skcipher.c
> +++ b/crypto/algif_skcipher.c
> @@ -100,11 +100,12 @@ static void skcipher_free_async_sgls(struct skcipher_async_req *sreq)
> kfree(sreq->tsg);
> }
>
> -static void skcipher_async_cb(struct crypto_async_request *req, int err)
> +static void skcipher_async_cb(struct crypto_async_request *_req, int err)
Please call it req_base or something similar.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt