[PATCH 0/3] crypto: afalg_skcipher - fixes after skcipher conversion

[PATCH 0/3] crypto: afalg_skcipher - fixes after skcipher conversion

[Tadeusz Struk]

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(-)

--

[PATCH 1/3] crypto: skcipher - return the correct request to the user

[Tadeusz Struk]

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: <stable@vger.kernel.org> # 4.4.x-
Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
---
 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);
 

[PATCH 2/3] crypto: algif_skcipher - fix async callback after skcipher convertion

[Tadeusz Struk]

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: <stable@vger.kernel.org> # 4.4.x-
Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
---
 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;
 

[PATCH 3/3] crypto: algif_skcipher - increase private contex space for async request

[Tadeusz Struk]

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: <stable@vger.kernel.org> # 4.4.x-
Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
---
 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))

Re: [PATCH 1/3] crypto: skcipher - return the correct request to the user

[Herbert Xu]

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 2/3] crypto: algif_skcipher - fix async callback after skcipher convertion

[Herbert Xu]

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: <stable@vger.kernel.org> # 4.4.x-
> Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
> ---
>  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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt