Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: [PATCH] crypto: acomp: export all scomp interfaces
From: Arnd Bergmann @ 2016-10-28 10:36 UTC (permalink / raw)
  To: Giovanni Cabiddu; +Cc: Herbert Xu, David S. Miller, linux-crypto, linux-kernel
In-Reply-To: <20161027210328.GA12705@SILVIXA00369791-F22-1>

On Thursday, October 27, 2016 10:03:29 PM CEST Giovanni Cabiddu wrote:
> Hi Arnd,
> 
> On Thu, Oct 27, 2016 at 04:09:49PM +0200, Arnd Bergmann wrote:
> > The newly added scomp/acomp interface has a couple of functions
> > that should be exported to allow linking the two drivers
> > as loadable modules:
> >
> > ERROR: "crypto_acomp_scomp_free_ctx" [crypto/acompress.ko] undefined!
> > ERROR: "crypto_acomp_scomp_alloc_ctx" [crypto/acompress.ko] undefined!
> > ERROR: "crypto_init_scomp_ops_async" [crypto/acompress.ko] undefined!
> These functions were not exported on purpose. They should not be called
> by other modules.
> 
> > Alternatively, both of them could be combined into one loadable module.
> I think this is the correct approach. Here is a patch with an
> alternative fix: https://patchwork.kernel.org/patch/9396503/

Yes, your patch is exactly what I had in mind as the alternative,
looks good to me.

	Arnd

^ permalink raw reply

* [PATCH v3] char: hw_random: atmel-rng: disable TRNG during suspend
From: Wenyou Yang @ 2016-10-28  8:00 UTC (permalink / raw)
  To: Herbert Xu, Matt Mackall
  Cc: linux-arm-kernel, Nicolas Ferre, linux-crypto, Wenyou Yang,
	Wenyou Yang

To fix the over consumption on the VDDCore due to the TRNG enabled,
disable the TRNG during suspend, not only disable the user interface
clock (which is controlled by PMC). Because the user interface clock
is independent from any clock that may be used in the entropy source
logic circuitry.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---

Changes in v3:
 - Add Acked-by tag.

Changes in v2:
 - Enable the user interface first, then enable the internal clock
   when resume.

 drivers/char/hw_random/atmel-rng.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/char/hw_random/atmel-rng.c b/drivers/char/hw_random/atmel-rng.c
index 0fcc9e6..ae7cae5 100644
--- a/drivers/char/hw_random/atmel-rng.c
+++ b/drivers/char/hw_random/atmel-rng.c
@@ -48,6 +48,16 @@ static int atmel_trng_read(struct hwrng *rng, void *buf, size_t max,
 		return 0;
 }
 
+static void atmel_trng_enable(struct atmel_trng *trng)
+{
+	writel(TRNG_KEY | 1, trng->base + TRNG_CR);
+}
+
+static void atmel_trng_disable(struct atmel_trng *trng)
+{
+	writel(TRNG_KEY, trng->base + TRNG_CR);
+}
+
 static int atmel_trng_probe(struct platform_device *pdev)
 {
 	struct atmel_trng *trng;
@@ -71,7 +81,7 @@ static int atmel_trng_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	writel(TRNG_KEY | 1, trng->base + TRNG_CR);
+	atmel_trng_enable(trng);
 	trng->rng.name = pdev->name;
 	trng->rng.read = atmel_trng_read;
 
@@ -94,7 +104,7 @@ static int atmel_trng_remove(struct platform_device *pdev)
 
 	hwrng_unregister(&trng->rng);
 
-	writel(TRNG_KEY, trng->base + TRNG_CR);
+	atmel_trng_disable(trng);
 	clk_disable_unprepare(trng->clk);
 
 	return 0;
@@ -105,6 +115,7 @@ static int atmel_trng_suspend(struct device *dev)
 {
 	struct atmel_trng *trng = dev_get_drvdata(dev);
 
+	atmel_trng_disable(trng);
 	clk_disable_unprepare(trng->clk);
 
 	return 0;
@@ -113,8 +124,15 @@ static int atmel_trng_suspend(struct device *dev)
 static int atmel_trng_resume(struct device *dev)
 {
 	struct atmel_trng *trng = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(trng->clk);
+	if (ret)
+		return ret;
 
-	return clk_prepare_enable(trng->clk);
+	atmel_trng_enable(trng);
+
+	return 0;
 }
 
 static const struct dev_pm_ops atmel_trng_pm_ops = {
-- 
2.7.4

^ permalink raw reply related

* [PATCH stable 3.10.y 17/18] crypto: cryptd - initialize child shash_desc on import
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable
  Cc: Herbert Xu, linux-crypto, Ard Biesheuvel, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

commit 0bd2223594a4dcddc1e34b15774a3a4776f7749e upstream.

When calling .import() on a cryptd ahash_request, the structure members
that describe the child transform in the shash_desc need to be initialized
like they are when calling .init()

Cc: stable@vger.kernel.org
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/cryptd.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/crypto/cryptd.c b/crypto/cryptd.c
index 75c415d..d85fab9 100644
--- a/crypto/cryptd.c
+++ b/crypto/cryptd.c
@@ -565,9 +565,14 @@ static int cryptd_hash_export(struct ahash_request *req, void *out)
 
 static int cryptd_hash_import(struct ahash_request *req, const void *in)
 {
-	struct cryptd_hash_request_ctx *rctx = ahash_request_ctx(req);
+	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+	struct cryptd_hash_ctx *ctx = crypto_ahash_ctx(tfm);
+	struct shash_desc *desc = cryptd_shash_desc(req);
+
+	desc->tfm = ctx->child;
+	desc->flags = req->base.flags;
 
-	return crypto_shash_import(&rctx->desc, in);
+	return crypto_shash_import(desc, in);
 }
 
 static int cryptd_create_hash(struct crypto_template *tmpl, struct rtattr **tb,
-- 
2.7.3

^ permalink raw reply related

* Re: [ANNOUNCE] libkcapi v0.12.0 released
From: Stephan Mueller @ 2016-10-27 23:04 UTC (permalink / raw)
  To: noloader; +Cc: linux-crypto
In-Reply-To: <CAH8yC8kvADeRgjxkBbdA5aMBDnVVKauTyfS-anaHr+bsF-U1iQ@mail.gmail.com>

Am Donnerstag, 27. Oktober 2016, 18:53:45 CEST schrieb Jeffrey Walton:

Hi Jeffrey,


> >> preprocessor macros we can use to guard it?
> > 
> > I am not entirely sure I understand the question.
> 
> See, for example,
> https://github.com/openssl/openssl/blob/master/engines/afalg/e_afalg.c
> 
> The versions are kind of arbitrary because there's no easy way to tell
> when the gear is available. If I recall from researching things, there
> were two components needed for afalg, and they were supposedly
> available back to later 2.x kernels.
> 
> Things should "just work" for 3.x and 4.x kernels. But if "too early"
> a kernel is encountered, then users experience the spectrum from
> compile problems to unexplained runtime errors.

I see. There are guards around zerocopy.

AF_ALG appeared with 2.6.38 / 3.0 which already had zerocopy.

I will try a 3.0 kernel with my current lib to see whether it works out of the 
box and apply fixes and guards if it does not.

But I am not sure whether I should guard anything for 2.x kernels where we 
know that some folks simply backported the AF_ALG interface.


Thanks a lot for the reminder.

Ciao
Stephan

^ permalink raw reply

* Re: [ANNOUNCE] libkcapi v0.12.0 released
From: Jeffrey Walton @ 2016-10-27 22:53 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: linux-crypto
In-Reply-To: <10631833.2nPcTagHHZ@positron.chronox.de>

>> > The Linux kernel exports a network interface of type AF_ALG to allow user
>> > space to utilize the kernel crypto API. libkcapi uses this network
>> > interface and exports an easy to use API so that a developer does not
>> > need to consider the low-level network interface handling.
...
>> any preprocessor macros to guard code paths in userland? What are the
>
> There are no special guards. If AF_ALG is available, all user space processes
> can use it.
>
>> preprocessor macros we can use to guard it?
>
> I am not entirely sure I understand the question.

See, for example,
https://github.com/openssl/openssl/blob/master/engines/afalg/e_afalg.c

The versions are kind of arbitrary because there's no easy way to tell
when the gear is available. If I recall from researching things, there
were two components needed for afalg, and they were supposedly
available back to later 2.x kernels.

Things should "just work" for 3.x and 4.x kernels. But if "too early"
a kernel is encountered, then users experience the spectrum from
compile problems to unexplained runtime errors.

Jeff

^ permalink raw reply

* Re: [PATCH] crypto: fix AEAD tag memory handling
From: Stephan Mueller @ 2016-10-27 22:20 UTC (permalink / raw)
  To: Mat Martineau; +Cc: herbert, linux-crypto
In-Reply-To: <alpine.OSX.2.20.1610271419040.52583@mjmartin-mac01.local>

Am Donnerstag, 27. Oktober 2016, 14:42:14 CEST schrieb Mat Martineau:

Hi Mat,

> Stephan and Herbert,
> 
> On Thu, 27 Oct 2016, Stephan Mueller wrote:
> > Hi Herbert,
> > 
> > for this patch, I have updated the testing for libkcapi accordingly and
> > all
> > tests pass. I will push the libkcapi code 0.12 with that test code change
> > out shortly. Using the current upstream version of 0.11.1 will show
> > failures as it expects the correct recv return code. As stated below,
> > that return code has changed which implies that some of the tests will
> > fail.
> > 
> > ---8<---
> > 
> > For encryption, the AEAD ciphers require AAD || PT as input and generate
> > AAD || CT || Tag as output and vice versa for decryption. Prior to this
> > patch, the AF_ALG interface for AEAD ciphers requires the buffer to be
> > present as input for encryption. Similarly, the output buffer for
> > decryption required the presence of the tag buffer too. This implies
> > that the kernel reads / writes data buffers from/to user space
> > even though this operation is not required.
> > 
> > This patch changes the AF_ALG AEAD interface to be consistent with the
> > in-kernel AEAD cipher memory requirements.
> > 
> > In addition, the code now handles the situation where the provided
> > output buffer is too small by reducing the size of the processed
> > input buffer accordingly. Due to this handling, the changes are
> > transparent to user space with one exception: the return code of recv
> > indicates the processed of output buffer size. That output buffer has a
> > different size compared to before the patch which implies that the
> > return code of recv will also be different. For example, a decryption
> > operation uses 16 bytes AAD, 16 bytes CT and 16 bytes tag, the AF_ALG
> > AEAD interface before showed a recv return code of 48 (bytes) whereas
> > after this patch, the return code is 32 since the tag is not returned
> > any more.
> 
> I tested out these changes and found that recv() or read() do not update
> all of the indicated bytes. In your decrypt example where there are 16
> bytes AAD, 16 bytes CT, and 16 bytes tag, read(sk, buf, 32) returns 32.

Please check the current patch (the one I sent to you as a pre-release did not 
contain the fix for the decryption part -- the patch sent to the list and what 
we discuss here contains the appropriate handling for the decryption side).

With your example using 16 bytes AAD, 16 bytes CT and 16 bytes Tag, the read 
will *only* show the return code of 16 (bytes) now, because only the CT part 
is converted into PT.

Yet, you are completely correct that the first 16 bytes of the AAD are skipped 
by the read call.

Thus, the read call returns exactly the amount of changed bytes, but it 
deviates from the POSIX logic by seeking to the end of the AAD buffer to find 
the offset it shall place the resulting data to.

> However, the first 16 bytes of buf are *unchanged* and the second 16 bytes
> contain the plaintext. In other words, 16 bytes were skipped over and 16
> bytes were read.

Correct. Again, with the current patch we discuss here, the read will return 
16 as it processed 16 bytes.
> 
> I see how this is similar to the documented use of the dst buffer in
> aead_request_set_crypt(), but it is not consistent with POSIX read()
> semantics.

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH] crypto: fix AEAD tag memory handling
From: Mat Martineau @ 2016-10-27 21:42 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: herbert, linux-crypto
In-Reply-To: <4006219.iWaCqWYv1B@positron.chronox.de>


Stephan and Herbert,

On Thu, 27 Oct 2016, Stephan Mueller wrote:

> Hi Herbert,
>
> for this patch, I have updated the testing for libkcapi accordingly and all
> tests pass. I will push the libkcapi code 0.12 with that test code change
> out shortly. Using the current upstream version of 0.11.1 will show failures
> as it expects the correct recv return code. As stated below, that return
> code has changed which implies that some of the tests will fail.
>
> ---8<---
>
> For encryption, the AEAD ciphers require AAD || PT as input and generate
> AAD || CT || Tag as output and vice versa for decryption. Prior to this
> patch, the AF_ALG interface for AEAD ciphers requires the buffer to be
> present as input for encryption. Similarly, the output buffer for
> decryption required the presence of the tag buffer too. This implies
> that the kernel reads / writes data buffers from/to user space
> even though this operation is not required.
>
> This patch changes the AF_ALG AEAD interface to be consistent with the
> in-kernel AEAD cipher memory requirements.
>
> In addition, the code now handles the situation where the provided
> output buffer is too small by reducing the size of the processed
> input buffer accordingly. Due to this handling, the changes are
> transparent to user space with one exception: the return code of recv
> indicates the processed of output buffer size. That output buffer has a
> different size compared to before the patch which implies that the
> return code of recv will also be different. For example, a decryption
> operation uses 16 bytes AAD, 16 bytes CT and 16 bytes tag, the AF_ALG
> AEAD interface before showed a recv return code of 48 (bytes) whereas
> after this patch, the return code is 32 since the tag is not returned
> any more.

I tested out these changes and found that recv() or read() do not update 
all of the indicated bytes. In your decrypt example where there are 16 
bytes AAD, 16 bytes CT, and 16 bytes tag, read(sk, buf, 32) returns 32. 
However, the first 16 bytes of buf are *unchanged* and the second 16 bytes 
contain the plaintext. In other words, 16 bytes were skipped over and 16 
bytes were read.

I see how this is similar to the documented use of the dst buffer in 
aead_request_set_crypt(), but it is not consistent with POSIX read() 
semantics.


--
Mat Martineau
Intel OTC

^ permalink raw reply

* Re: [PATCH] crypto: acomp: export all scomp interfaces
From: Giovanni Cabiddu @ 2016-10-27 21:03 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Herbert Xu, David S. Miller, linux-crypto, linux-kernel
In-Reply-To: <20161027141029.2566709-1-arnd@arndb.de>

Hi Arnd,

On Thu, Oct 27, 2016 at 04:09:49PM +0200, Arnd Bergmann wrote:
> The newly added scomp/acomp interface has a couple of functions
> that should be exported to allow linking the two drivers
> as loadable modules:
>
> ERROR: "crypto_acomp_scomp_free_ctx" [crypto/acompress.ko] undefined!
> ERROR: "crypto_acomp_scomp_alloc_ctx" [crypto/acompress.ko] undefined!
> ERROR: "crypto_init_scomp_ops_async" [crypto/acompress.ko] undefined!
These functions were not exported on purpose. They should not be called
by other modules.

> Alternatively, both of them could be combined into one loadable module.
I think this is the correct approach. Here is a patch with an
alternative fix: https://patchwork.kernel.org/patch/9396503/

Thanks,

--
Giovanni

^ permalink raw reply

* [PATCH stable 3.10.y 02/18] crypto: af_alg - Disallow bind/setkey/... after accept(2)
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit c840ac6af3f8713a71b4d2363419145760bd6044 upstream.

Each af_alg parent socket obtained by socket(2) corresponds to a
tfm object once bind(2) has succeeded.  An accept(2) call on that
parent socket creates a context which then uses the tfm object.

Therefore as long as any child sockets created by accept(2) exist
the parent socket must not be modified or freed.

This patch guarantees this by using locks and a reference count
on the parent socket.  Any attempt to modify the parent socket will
fail with EBUSY.

Cc: stable@vger.kernel.org
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/af_alg.c         | 35 ++++++++++++++++++++++++++++++++---
 include/crypto/if_alg.h |  8 +++-----
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 1aaa555..0ca108f 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -125,6 +125,23 @@ int af_alg_release(struct socket *sock)
 }
 EXPORT_SYMBOL_GPL(af_alg_release);
 
+void af_alg_release_parent(struct sock *sk)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	bool last;
+
+	sk = ask->parent;
+	ask = alg_sk(sk);
+
+	lock_sock(sk);
+	last = !--ask->refcnt;
+	release_sock(sk);
+
+	if (last)
+		sock_put(sk);
+}
+EXPORT_SYMBOL_GPL(af_alg_release_parent);
+
 static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 {
 	struct sock *sk = sock->sk;
@@ -132,6 +149,7 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 	struct sockaddr_alg *sa = (void *)uaddr;
 	const struct af_alg_type *type;
 	void *private;
+	int err;
 
 	if (sock->state == SS_CONNECTED)
 		return -EINVAL;
@@ -157,16 +175,22 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 		return PTR_ERR(private);
 	}
 
+	err = -EBUSY;
 	lock_sock(sk);
+	if (ask->refcnt)
+		goto unlock;
 
 	swap(ask->type, type);
 	swap(ask->private, private);
 
+	err = 0;
+
+unlock:
 	release_sock(sk);
 
 	alg_do_release(type, private);
 
-	return 0;
+	return err;
 }
 
 static int alg_setkey(struct sock *sk, char __user *ukey,
@@ -199,11 +223,15 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	const struct af_alg_type *type;
-	int err = -ENOPROTOOPT;
+	int err = -EBUSY;
 
 	lock_sock(sk);
+	if (ask->refcnt)
+		goto unlock;
+
 	type = ask->type;
 
+	err = -ENOPROTOOPT;
 	if (level != SOL_ALG || !type)
 		goto unlock;
 
@@ -252,7 +280,8 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)
 
 	sk2->sk_family = PF_ALG;
 
-	sock_hold(sk);
+	if (!ask->refcnt++)
+		sock_hold(sk);
 	alg_sk(sk2)->parent = sk;
 	alg_sk(sk2)->type = type;
 
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index d61c111..2f38daa 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -30,6 +30,8 @@ struct alg_sock {
 
 	struct sock *parent;
 
+	unsigned int refcnt;
+
 	const struct af_alg_type *type;
 	void *private;
 };
@@ -64,6 +66,7 @@ int af_alg_register_type(const struct af_alg_type *type);
 int af_alg_unregister_type(const struct af_alg_type *type);
 
 int af_alg_release(struct socket *sock);
+void af_alg_release_parent(struct sock *sk);
 int af_alg_accept(struct sock *sk, struct socket *newsock);
 
 int af_alg_make_sg(struct af_alg_sgl *sgl, void __user *addr, int len,
@@ -80,11 +83,6 @@ static inline struct alg_sock *alg_sk(struct sock *sk)
 	return (struct alg_sock *)sk;
 }
 
-static inline void af_alg_release_parent(struct sock *sk)
-{
-	sock_put(alg_sk(sk)->parent);
-}
-
 static inline void af_alg_init_completion(struct af_alg_completion *completion)
 {
 	init_completion(&completion->completion);
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 16/18] crypto: shash - Fix has_key setting
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit 00420a65fa2beb3206090ead86942484df2275f3 upstream.

The has_key logic is wrong for shash algorithms as they always
have a setkey function.  So we should instead be testing against
shash_no_setkey.

Fixes: a5596d633278 ("crypto: hash - Add crypto_ahash_has_setkey")
Cc: stable@vger.kernel.org
Reported-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Tested-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/shash.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/crypto/shash.c b/crypto/shash.c
index 8e4256a..ac4d763 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -353,11 +353,10 @@ int crypto_init_shash_ops_async(struct crypto_tfm *tfm)
 	crt->final = shash_async_final;
 	crt->finup = shash_async_finup;
 	crt->digest = shash_async_digest;
+	crt->setkey = shash_async_setkey;
+
+	crt->has_setkey = alg->setkey != shash_no_setkey;
 
-	if (alg->setkey) {
-		crt->setkey = shash_async_setkey;
-		crt->has_setkey = true;
-	}
 	if (alg->export)
 		crt->export = shash_async_export;
 	if (alg->import)
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 15/18] crypto: algif_skcipher - Load TX SG list after waiting
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit 4f0414e54e4d1893c6f08260693f8ef84c929293 upstream.

We need to load the TX SG list in sendmsg(2) after waiting for
incoming data, not before.

Cc: stable@vger.kernel.org
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Tested-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/algif_skcipher.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 13fd26e..ea05c53 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -446,13 +446,6 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
 		char __user *from = iov->iov_base;
 
 		while (seglen) {
-			sgl = list_first_entry(&ctx->tsgl,
-					       struct skcipher_sg_list, list);
-			sg = sgl->sg;
-
-			while (!sg->length)
-				sg++;
-
 			used = ctx->used;
 			if (!used) {
 				err = skcipher_wait_for_data(sk, flags);
@@ -474,6 +467,13 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
 			if (!used)
 				goto free;
 
+			sgl = list_first_entry(&ctx->tsgl,
+					       struct skcipher_sg_list, list);
+			sg = sgl->sg;
+
+			while (!sg->length)
+				sg++;
+
 			ablkcipher_request_set_crypt(&ctx->req, sg,
 						     ctx->rsgl.sg, used,
 						     ctx->iv);
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 05/18] crypto: hash - Add crypto_ahash_has_setkey
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit a5596d6332787fd383b3b5427b41f94254430827 upstream.

This patch adds a way for ahash users to determine whether a key
is required by a crypto_ahash transform.

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/ahash.c        | 5 ++++-
 crypto/shash.c        | 4 +++-
 include/crypto/hash.h | 6 ++++++
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/crypto/ahash.c b/crypto/ahash.c
index bcd5efc..781a8a7 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -370,6 +370,7 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
 	struct ahash_alg *alg = crypto_ahash_alg(hash);
 
 	hash->setkey = ahash_nosetkey;
+	hash->has_setkey = false;
 	hash->export = ahash_no_export;
 	hash->import = ahash_no_import;
 
@@ -382,8 +383,10 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
 	hash->finup = alg->finup ?: ahash_def_finup;
 	hash->digest = alg->digest;
 
-	if (alg->setkey)
+	if (alg->setkey) {
 		hash->setkey = alg->setkey;
+		hash->has_setkey = true;
+	}
 	if (alg->export)
 		hash->export = alg->export;
 	if (alg->import)
diff --git a/crypto/shash.c b/crypto/shash.c
index 929058a..8e4256a 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -354,8 +354,10 @@ int crypto_init_shash_ops_async(struct crypto_tfm *tfm)
 	crt->finup = shash_async_finup;
 	crt->digest = shash_async_digest;
 
-	if (alg->setkey)
+	if (alg->setkey) {
 		crt->setkey = shash_async_setkey;
+		crt->has_setkey = true;
+	}
 	if (alg->export)
 		crt->export = shash_async_export;
 	if (alg->import)
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index 26cb1eb..c8c7987 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -94,6 +94,7 @@ struct crypto_ahash {
 		      unsigned int keylen);
 
 	unsigned int reqsize;
+	bool has_setkey;
 	struct crypto_tfm base;
 };
 
@@ -181,6 +182,11 @@ static inline void *ahash_request_ctx(struct ahash_request *req)
 
 int crypto_ahash_setkey(struct crypto_ahash *tfm, const u8 *key,
 			unsigned int keylen);
+static inline bool crypto_ahash_has_setkey(struct crypto_ahash *tfm)
+{
+	return tfm->has_setkey;
+}
+
 int crypto_ahash_finup(struct ahash_request *req);
 int crypto_ahash_final(struct ahash_request *req);
 int crypto_ahash_digest(struct ahash_request *req);
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 10/18] crypto: algif_hash - Remove custom release parent function
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit f1d84af1835846a5a2b827382c5848faf2bb0e75 upstream.

This patch removes the custom release parent function as the
generic af_alg_release_parent now works for nokey sockets too.

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/algif_hash.c | 43 +++----------------------------------------
 1 file changed, 3 insertions(+), 40 deletions(-)

diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 7bc3f89..512aa36 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -397,7 +397,7 @@ static int hash_setkey(void *private, const u8 *key, unsigned int keylen)
 	return err;
 }
 
-static void hash_sock_destruct_common(struct sock *sk)
+static void hash_sock_destruct(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct hash_ctx *ctx = ask->private;
@@ -405,33 +405,10 @@ static void hash_sock_destruct_common(struct sock *sk)
 	sock_kfree_s(sk, ctx->result,
 		     crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req)));
 	sock_kfree_s(sk, ctx, ctx->len);
-}
-
-static void hash_sock_destruct(struct sock *sk)
-{
-	hash_sock_destruct_common(sk);
-	af_alg_release_parent(sk);
-}
-
-static void hash_release_parent_nokey(struct sock *sk)
-{
-	struct alg_sock *ask = alg_sk(sk);
-
-	if (!ask->refcnt) {
-		sock_put(ask->parent);
-		return;
-	}
-
 	af_alg_release_parent(sk);
 }
 
-static void hash_sock_destruct_nokey(struct sock *sk)
-{
-	hash_sock_destruct_common(sk);
-	hash_release_parent_nokey(sk);
-}
-
-static int hash_accept_parent_common(void *private, struct sock *sk)
+static int hash_accept_parent_nokey(void *private, struct sock *sk)
 {
 	struct hash_ctx *ctx;
 	struct alg_sock *ask = alg_sk(sk);
@@ -474,21 +451,7 @@ static int hash_accept_parent(void *private, struct sock *sk)
 	if (!tfm->has_key && crypto_ahash_has_setkey(tfm->hash))
 		return -ENOKEY;
 
-	return hash_accept_parent_common(private, sk);
-}
-
-static int hash_accept_parent_nokey(void *private, struct sock *sk)
-{
-	int err;
-
-	err = hash_accept_parent_common(private, sk);
-	if (err)
-		goto out;
-
-	sk->sk_destruct = hash_sock_destruct_nokey;
-
-out:
-	return err;
+	return hash_accept_parent_nokey(private, sk);
 }
 
 static const struct af_alg_type algif_type_hash = {
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 04/18] crypto: algif_skcipher - Add nokey compatibility path
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit a0fa2d037129a9849918a92d91b79ed6c7bd2818 upstream.

This patch adds a compatibility path to support old applications
that do acept(2) before setkey.

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/algif_skcipher.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 144 insertions(+), 5 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index c4c121a..db5f0f0 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -549,6 +549,99 @@ static struct proto_ops algif_skcipher_ops = {
 	.poll		=	skcipher_poll,
 };
 
+static int skcipher_check_key(struct socket *sock)
+{
+	int err;
+	struct sock *psk;
+	struct alg_sock *pask;
+	struct skcipher_tfm *tfm;
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+
+	if (ask->refcnt)
+		return 0;
+
+	psk = ask->parent;
+	pask = alg_sk(ask->parent);
+	tfm = pask->private;
+
+	err = -ENOKEY;
+	lock_sock(psk);
+	if (!tfm->has_key)
+		goto unlock;
+
+	if (!pask->refcnt++)
+		sock_hold(psk);
+
+	ask->refcnt = 1;
+	sock_put(psk);
+
+	err = 0;
+
+unlock:
+	release_sock(psk);
+
+	return err;
+}
+
+static int skcipher_sendmsg_nokey(struct kiocb *unused, struct socket *sock,
+				  struct msghdr *msg, size_t size)
+{
+	int err;
+
+	err = skcipher_check_key(sock);
+	if (err)
+		return err;
+
+	return skcipher_sendmsg(unused, sock, msg, size);
+}
+
+static ssize_t skcipher_sendpage_nokey(struct socket *sock, struct page *page,
+				       int offset, size_t size, int flags)
+{
+	int err;
+
+	err = skcipher_check_key(sock);
+	if (err)
+		return err;
+
+	return skcipher_sendpage(sock, page, offset, size, flags);
+}
+
+static int skcipher_recvmsg_nokey(struct kiocb *unused, struct socket *sock,
+				  struct msghdr *msg, size_t ignored, int flags)
+{
+	int err;
+
+	err = skcipher_check_key(sock);
+	if (err)
+		return err;
+
+	return skcipher_recvmsg(unused, sock, msg, ignored, flags);
+}
+
+static struct proto_ops algif_skcipher_ops_nokey = {
+	.family		=	PF_ALG,
+
+	.connect	=	sock_no_connect,
+	.socketpair	=	sock_no_socketpair,
+	.getname	=	sock_no_getname,
+	.ioctl		=	sock_no_ioctl,
+	.listen		=	sock_no_listen,
+	.shutdown	=	sock_no_shutdown,
+	.getsockopt	=	sock_no_getsockopt,
+	.mmap		=	sock_no_mmap,
+	.bind		=	sock_no_bind,
+	.accept		=	sock_no_accept,
+	.setsockopt	=	sock_no_setsockopt,
+
+	.release	=	af_alg_release,
+	.sendmsg	=	skcipher_sendmsg_nokey,
+	.sendpage	=	skcipher_sendpage_nokey,
+	.recvmsg	=	skcipher_recvmsg_nokey,
+	.poll		=	skcipher_poll,
+};
+
 static void *skcipher_bind(const char *name, u32 type, u32 mask)
 {
 	struct skcipher_tfm *tfm;
@@ -588,7 +681,7 @@ static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
 	return err;
 }
 
-static void skcipher_sock_destruct(struct sock *sk)
+static void skcipher_sock_destruct_common(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
@@ -597,10 +690,33 @@ static void skcipher_sock_destruct(struct sock *sk)
 	skcipher_free_sgl(sk);
 	sock_kfree_s(sk, ctx->iv, crypto_ablkcipher_ivsize(tfm));
 	sock_kfree_s(sk, ctx, ctx->len);
+}
+
+static void skcipher_sock_destruct(struct sock *sk)
+{
+	skcipher_sock_destruct_common(sk);
 	af_alg_release_parent(sk);
 }
 
-static int skcipher_accept_parent(void *private, struct sock *sk)
+static void skcipher_release_parent_nokey(struct sock *sk)
+{
+	struct alg_sock *ask = alg_sk(sk);
+
+	if (!ask->refcnt) {
+		sock_put(ask->parent);
+		return;
+	}
+
+	af_alg_release_parent(sk);
+}
+
+static void skcipher_sock_destruct_nokey(struct sock *sk)
+{
+	skcipher_sock_destruct_common(sk);
+	skcipher_release_parent_nokey(sk);
+}
+
+static int skcipher_accept_parent_common(void *private, struct sock *sk)
 {
 	struct skcipher_ctx *ctx;
 	struct alg_sock *ask = alg_sk(sk);
@@ -608,9 +724,6 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
 	struct crypto_ablkcipher *skcipher = tfm->skcipher;
 	unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(skcipher);
 
-	if (!tfm->has_key)
-		return -ENOKEY;
-
 	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
@@ -642,12 +755,38 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
 	return 0;
 }
 
+static int skcipher_accept_parent(void *private, struct sock *sk)
+{
+	struct skcipher_tfm *tfm = private;
+
+	if (!tfm->has_key)
+		return -ENOKEY;
+
+	return skcipher_accept_parent_common(private, sk);
+}
+
+static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
+{
+	int err;
+
+	err = skcipher_accept_parent_common(private, sk);
+	if (err)
+		goto out;
+
+	sk->sk_destruct = skcipher_sock_destruct_nokey;
+
+out:
+	return err;
+}
+
 static const struct af_alg_type algif_type_skcipher = {
 	.bind		=	skcipher_bind,
 	.release	=	skcipher_release,
 	.setkey		=	skcipher_setkey,
 	.accept		=	skcipher_accept_parent,
+	.accept_nokey	=	skcipher_accept_parent_nokey,
 	.ops		=	&algif_skcipher_ops,
+	.ops_nokey	=	&algif_skcipher_ops_nokey,
 	.name		=	"skcipher",
 	.owner		=	THIS_MODULE
 };
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 14/18] crypto: algif_skcipher - Fix race condition in skcipher_check_key
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit 1822793a523e5d5730b19cc21160ff1717421bc8 upstream.

We need to lock the child socket in skcipher_check_key as otherwise
two simultaneous calls can cause the parent socket to be freed.

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/algif_skcipher.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index a7800b7..13fd26e 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -551,22 +551,23 @@ static struct proto_ops algif_skcipher_ops = {
 
 static int skcipher_check_key(struct socket *sock)
 {
-	int err;
+	int err = 0;
 	struct sock *psk;
 	struct alg_sock *pask;
 	struct skcipher_tfm *tfm;
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 
+	lock_sock(sk);
 	if (ask->refcnt)
-		return 0;
+		goto unlock_child;
 
 	psk = ask->parent;
 	pask = alg_sk(ask->parent);
 	tfm = pask->private;
 
 	err = -ENOKEY;
-	lock_sock(psk);
+	lock_sock_nested(psk, SINGLE_DEPTH_NESTING);
 	if (!tfm->has_key)
 		goto unlock;
 
@@ -580,6 +581,8 @@ static int skcipher_check_key(struct socket *sock)
 
 unlock:
 	release_sock(psk);
+unlock_child:
+	release_sock(sk);
 
 	return err;
 }
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 09/18] crypto: af_alg - Allow af_af_alg_release_parent to be called on nokey path
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit 6a935170a980024dd29199e9dbb5c4da4767a1b9 upstream.

This patch allows af_alg_release_parent to be called even for
nokey sockets.

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/af_alg.c         | 9 ++++++++-
 include/crypto/if_alg.h | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index de130c2..2f8fd84 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -133,6 +133,12 @@ void af_alg_release_parent(struct sock *sk)
 	bool last;
 
 	sk = ask->parent;
+
+	if (ask->nokey_refcnt && !ask->refcnt) {
+		sock_put(sk);
+		return;
+	}
+
 	ask = alg_sk(sk);
 
 	lock_sock(sk);
@@ -258,8 +264,8 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)
 	struct alg_sock *ask = alg_sk(sk);
 	const struct af_alg_type *type;
 	struct sock *sk2;
+	unsigned int nokey;
 	int err;
-	bool nokey;
 
 	lock_sock(sk);
 	type = ask->type;
@@ -292,6 +298,7 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)
 		sock_hold(sk);
 	alg_sk(sk2)->parent = sk;
 	alg_sk(sk2)->type = type;
+	alg_sk(sk2)->nokey_refcnt = nokey;
 
 	newsock->ops = type->ops;
 	newsock->state = SS_CONNECTED;
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 9e6a2f3..bfefd81 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -31,6 +31,7 @@ struct alg_sock {
 	struct sock *parent;
 
 	unsigned int refcnt;
+	unsigned int nokey_refcnt;
 
 	const struct af_alg_type *type;
 	void *private;
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 13/18] crypto: algif_hash - Fix race condition in hash_check_key
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit ad46d7e33219218605ea619e32553daf4f346b9f upstream.

We need to lock the child socket in hash_check_key as otherwise
two simultaneous calls can cause the parent socket to be freed.

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/algif_hash.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 512aa36..d11d431 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -255,22 +255,23 @@ static struct proto_ops algif_hash_ops = {
 
 static int hash_check_key(struct socket *sock)
 {
-	int err;
+	int err = 0;
 	struct sock *psk;
 	struct alg_sock *pask;
 	struct algif_hash_tfm *tfm;
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 
+	lock_sock(sk);
 	if (ask->refcnt)
-		return 0;
+		goto unlock_child;
 
 	psk = ask->parent;
 	pask = alg_sk(ask->parent);
 	tfm = pask->private;
 
 	err = -ENOKEY;
-	lock_sock(psk);
+	lock_sock_nested(psk, SINGLE_DEPTH_NESTING);
 	if (!tfm->has_key)
 		goto unlock;
 
@@ -284,6 +285,8 @@ static int hash_check_key(struct socket *sock)
 
 unlock:
 	release_sock(psk);
+unlock_child:
+	release_sock(sk);
 
 	return err;
 }
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 01/18] crypto: algif_skcipher - Require setkey before accept(2)
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit dd504589577d8e8e70f51f997ad487a4cb6c026f upstream.

Some cipher implementations will crash if you try to use them
without calling setkey first.  This patch adds a check so that
the accept(2) call will fail with -ENOKEY if setkey hasn't been
done on the socket yet.

Cc: stable@vger.kernel.org
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Tested-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/algif_skcipher.c | 51 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 9 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 83187f4..c4c121a 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -31,6 +31,11 @@ struct skcipher_sg_list {
 	struct scatterlist sg[0];
 };
 
+struct skcipher_tfm {
+	struct crypto_ablkcipher *skcipher;
+	bool has_key;
+};
+
 struct skcipher_ctx {
 	struct list_head tsgl;
 	struct af_alg_sgl rsgl;
@@ -546,17 +551,41 @@ static struct proto_ops algif_skcipher_ops = {
 
 static void *skcipher_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_ablkcipher(name, type, mask);
+	struct skcipher_tfm *tfm;
+	struct crypto_ablkcipher *skcipher;
+
+	tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
+	if (!tfm)
+		return ERR_PTR(-ENOMEM);
+
+	skcipher = crypto_alloc_ablkcipher(name, type, mask);
+	if (IS_ERR(skcipher)) {
+		kfree(tfm);
+		return ERR_CAST(skcipher);
+	}
+
+	tfm->skcipher = skcipher;
+
+	return tfm;
 }
 
 static void skcipher_release(void *private)
 {
-	crypto_free_ablkcipher(private);
+	struct skcipher_tfm *tfm = private;
+
+	crypto_free_ablkcipher(tfm->skcipher);
+	kfree(tfm);
 }
 
 static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
 {
-	return crypto_ablkcipher_setkey(private, key, keylen);
+	struct skcipher_tfm *tfm = private;
+	int err;
+
+	err = crypto_ablkcipher_setkey(tfm->skcipher, key, keylen);
+	tfm->has_key = !err;
+
+	return err;
 }
 
 static void skcipher_sock_destruct(struct sock *sk)
@@ -575,20 +604,24 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
 {
 	struct skcipher_ctx *ctx;
 	struct alg_sock *ask = alg_sk(sk);
-	unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(private);
+	struct skcipher_tfm *tfm = private;
+	struct crypto_ablkcipher *skcipher = tfm->skcipher;
+	unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(skcipher);
+
+	if (!tfm->has_key)
+		return -ENOKEY;
 
 	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
-
-	ctx->iv = sock_kmalloc(sk, crypto_ablkcipher_ivsize(private),
+	ctx->iv = sock_kmalloc(sk, crypto_ablkcipher_ivsize(skcipher),
 			       GFP_KERNEL);
 	if (!ctx->iv) {
 		sock_kfree_s(sk, ctx, len);
 		return -ENOMEM;
 	}
 
-	memset(ctx->iv, 0, crypto_ablkcipher_ivsize(private));
+	memset(ctx->iv, 0, crypto_ablkcipher_ivsize(skcipher));
 
 	INIT_LIST_HEAD(&ctx->tsgl);
 	ctx->len = len;
@@ -600,9 +633,9 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
 
 	ask->private = ctx;
 
-	ablkcipher_request_set_tfm(&ctx->req, private);
+	ablkcipher_request_set_tfm(&ctx->req, skcipher);
 	ablkcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-					af_alg_complete, &ctx->completion);
+				      af_alg_complete, &ctx->completion);
 
 	sk->sk_destruct = skcipher_sock_destruct;
 
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 03/18] crypto: af_alg - Add nokey compatibility path
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit 37766586c965d63758ad542325a96d5384f4a8c9 upstream.

This patch adds a compatibility path to support old applications
that do acept(2) before setkey.

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/af_alg.c         | 13 ++++++++++++-
 include/crypto/if_alg.h |  2 ++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 0ca108f..de130c2 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -76,6 +76,8 @@ int af_alg_register_type(const struct af_alg_type *type)
 		goto unlock;
 
 	type->ops->owner = THIS_MODULE;
+	if (type->ops_nokey)
+		type->ops_nokey->owner = THIS_MODULE;
 	node->type = type;
 	list_add(&node->list, &alg_types);
 	err = 0;
@@ -257,6 +259,7 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)
 	const struct af_alg_type *type;
 	struct sock *sk2;
 	int err;
+	bool nokey;
 
 	lock_sock(sk);
 	type = ask->type;
@@ -275,12 +278,17 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)
 	security_sk_clone(sk, sk2);
 
 	err = type->accept(ask->private, sk2);
+
+	nokey = err == -ENOKEY;
+	if (nokey && type->accept_nokey)
+		err = type->accept_nokey(ask->private, sk2);
+
 	if (err)
 		goto unlock;
 
 	sk2->sk_family = PF_ALG;
 
-	if (!ask->refcnt++)
+	if (nokey || !ask->refcnt++)
 		sock_hold(sk);
 	alg_sk(sk2)->parent = sk;
 	alg_sk(sk2)->type = type;
@@ -288,6 +296,9 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)
 	newsock->ops = type->ops;
 	newsock->state = SS_CONNECTED;
 
+	if (nokey)
+		newsock->ops = type->ops_nokey;
+
 	err = 0;
 
 unlock:
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 2f38daa..9e6a2f3 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -51,8 +51,10 @@ struct af_alg_type {
 	void (*release)(void *private);
 	int (*setkey)(void *private, const u8 *key, unsigned int keylen);
 	int (*accept)(void *private, struct sock *sk);
+	int (*accept_nokey)(void *private, struct sock *sk);
 
 	struct proto_ops *ops;
+	struct proto_ops *ops_nokey;
 	struct module *owner;
 	char name[14];
 };
-- 
2.7.3

^ permalink raw reply related

* Re: [PATCH stable 3.10.y 00/18] Backport crypto fixes.
From: Willy Tarreau @ 2016-10-27 15:06 UTC (permalink / raw)
  To: Andrey Ryabinin; +Cc: stable, Herbert Xu, linux-crypto
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

Hi Andrey,

On Thu, Oct 27, 2016 at 05:29:33PM +0300, Andrey Ryabinin wrote:
> While fuzzing our 3.10 based kernel we observed some crashes, e.g:
>       http://lkml.kernel.org/r/<CACT4Y+aE-pNUnCLbDQkFQ9Y7QivN_g6hEuS7t1RpknPHFmHg+g@mail.gmail.com>
> 
> It appears that 3.10-stable lacks some crypto related fixes. Here is my attempt
> to backport them.
> Many patches didn't apply cleanly, so to make sure that my conflict resolution
> is correct I compared it with other stable trees.
(...)

Thanks a lot for doing this! I'm queueing them for next 3.10 and will merge
them unless I receive a nack for any of them.

Best regards,
Willy

^ permalink raw reply

* Re: AEAD Question
From: Juan Pablo Nariño Mendoza @ 2016-10-27  8:05 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: linux-crypto
In-Reply-To: <1536924.VKbsE1YqZP@positron.chronox.de>

Hello Stephan
Thank you for your fast answer

My idea is to do zero copy encryption and the cipher I am using is
authenc(hmac(sha1),cbc(aes).

The layout of my buffer is

espHeader(AD) || IV || plaintext || Integrity (TAG)

As I see it, the SGs do point to the same buffer, but they do not overlap

Below, some parts of my code


// Prepare data
sg_init_table(sg,ARRAY_SIZE(sg));


sg_set_buf(&sg[1],cipherText_p,espHdrLen);
sg_set_buf(&sg[0],cipherText_p+espHdrLen+ivLen,plen+intLen);

// Put plaintext in cipherText + espHdr + ivLen
// put esp header info in ciphertext

req_p = aead_givcrypt_alloc(aead_p,GFP_KERNEL);
aead_givcrypt_set_callback(req_p,0,hwencpoc_testCipher_cb,cipherText_p);
aead_givcrypt_set_crypt(req_p,&sg[0],&sg[0],plen,iv_p);
aead_givcrypt_set_assoc(req_p,&sg[1],espHdrLen);
aead_givcrypt_set_giv(req_p,cipherText_p+espHdrLen,0);

ret=crypto_aead_givencrypt(req_p);


However, when I get the callback, the associated data has been
overwritten (first 8 bytes of the cipherText buffer), and the
integrity is wrong. The IV and the encryption are correct. What I am
doing wrong? Thank you again

Juan

2016-10-26 18:32 GMT+02:00 Stephan Mueller <smueller@chronox.de>:
> Am Mittwoch, 26. Oktober 2016, 18:17:14 CEST schrieb Juan Pablo Nariño
> Mendoza:
>
> Hi Juan,
>
>> Sorry in advance for making what shall be a basic question for this
>> list, but I have really ran out of ideas.
>>
>> Can someone explain me please, how does the memory layout and sg lists
>> work for the IPSec case, with generated IV case?
>>
>> As I understand, it is like this (ascii art)
>>
>> AD | IV Space | Plain Text | Auth Tag
>>  ^                   ^
>>
>> asg              sg
>
> The memory layout is correct when using the RFC4106 cipher.
>
> How you partition your physical memory with the SGL is your choice as long as
> your data in the SGL is lined up as you mentioned above.
>>
>> But I am having the AD overwritten, and the AUTH tag and encryption is
>> not correct? Could someone please point me in the right direction?
>> Thank you again
>
> Maybe the src and dst SGL somehow partially overlap?
>>
>> Regards
>>
>> Juan
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> Ciao
> Stephan

^ permalink raw reply

* [PATCH stable 3.10.y 18/18] crypto: skcipher - Fix blkcipher walk OOM crash
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit acdb04d0b36769b3e05990c488dc74d8b7ac8060 upstream.

When we need to allocate a temporary blkcipher_walk_next and it
fails, the code is supposed to take the slow path of processing
the data block by block.  However, due to an unrelated change
we instead end up dereferencing the NULL pointer.

This patch fixes it by moving the unrelated bsize setting out
of the way so that we enter the slow path as inteded.

Fixes: 7607bd8ff03b ("[CRYPTO] blkcipher: Added blkcipher_walk_virt_block")
Cc: stable@vger.kernel.org
Reported-by: xiakaixu <xiakaixu@huawei.com>
Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/blkcipher.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c
index 37af08e..39b09f25 100644
--- a/crypto/blkcipher.c
+++ b/crypto/blkcipher.c
@@ -238,6 +238,8 @@ static int blkcipher_walk_next(struct blkcipher_desc *desc,
 		return blkcipher_walk_done(desc, walk, -EINVAL);
 	}
 
+	bsize = min(walk->blocksize, n);
+
 	walk->flags &= ~(BLKCIPHER_WALK_SLOW | BLKCIPHER_WALK_COPY |
 			 BLKCIPHER_WALK_DIFF);
 	if (!scatterwalk_aligned(&walk->in, alignmask) ||
@@ -250,7 +252,6 @@ static int blkcipher_walk_next(struct blkcipher_desc *desc,
 		}
 	}
 
-	bsize = min(walk->blocksize, n);
 	n = scatterwalk_clamp(&walk->in, n);
 	n = scatterwalk_clamp(&walk->out, n);
 
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 12/18] crypto: af_alg - Forbid bind(2) when nokey child sockets are present
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit a6a48c565f6f112c6983e2a02b1602189ed6e26e upstream.

This patch forbids the calling of bind(2) when there are child
sockets created by accept(2) in existence, even if they are created
on the nokey path.

This is needed as those child sockets have references to the tfm
object which bind(2) will destroy.

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/af_alg.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 2f8fd84..68ec1ac 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -130,19 +130,16 @@ EXPORT_SYMBOL_GPL(af_alg_release);
 void af_alg_release_parent(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
-	bool last;
+	unsigned int nokey = ask->nokey_refcnt;
+	bool last = nokey && !ask->refcnt;
 
 	sk = ask->parent;
-
-	if (ask->nokey_refcnt && !ask->refcnt) {
-		sock_put(sk);
-		return;
-	}
-
 	ask = alg_sk(sk);
 
 	lock_sock(sk);
-	last = !--ask->refcnt;
+	ask->nokey_refcnt -= nokey;
+	if (!last)
+		last = !--ask->refcnt;
 	release_sock(sk);
 
 	if (last)
@@ -185,7 +182,7 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 
 	err = -EBUSY;
 	lock_sock(sk);
-	if (ask->refcnt)
+	if (ask->refcnt | ask->nokey_refcnt)
 		goto unlock;
 
 	swap(ask->type, type);
@@ -296,6 +293,7 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)
 
 	if (nokey || !ask->refcnt++)
 		sock_hold(sk);
+	ask->nokey_refcnt += nokey;
 	alg_sk(sk2)->parent = sk;
 	alg_sk(sk2)->type = type;
 	alg_sk(sk2)->nokey_refcnt = nokey;
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 11/18] crypto: algif_skcipher - Remove custom release parent function
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit d7b65aee1e7b4c87922b0232eaba56a8a143a4a0 upstream.

This patch removes the custom release parent function as the
generic af_alg_release_parent now works for nokey sockets too.

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/algif_skcipher.c | 43 +++----------------------------------------
 1 file changed, 3 insertions(+), 40 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 4677a45..a7800b7 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -681,7 +681,7 @@ static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
 	return err;
 }
 
-static void skcipher_sock_destruct_common(struct sock *sk)
+static void skcipher_sock_destruct(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
@@ -690,33 +690,10 @@ static void skcipher_sock_destruct_common(struct sock *sk)
 	skcipher_free_sgl(sk);
 	sock_kfree_s(sk, ctx->iv, crypto_ablkcipher_ivsize(tfm));
 	sock_kfree_s(sk, ctx, ctx->len);
-}
-
-static void skcipher_sock_destruct(struct sock *sk)
-{
-	skcipher_sock_destruct_common(sk);
-	af_alg_release_parent(sk);
-}
-
-static void skcipher_release_parent_nokey(struct sock *sk)
-{
-	struct alg_sock *ask = alg_sk(sk);
-
-	if (!ask->refcnt) {
-		sock_put(ask->parent);
-		return;
-	}
-
 	af_alg_release_parent(sk);
 }
 
-static void skcipher_sock_destruct_nokey(struct sock *sk)
-{
-	skcipher_sock_destruct_common(sk);
-	skcipher_release_parent_nokey(sk);
-}
-
-static int skcipher_accept_parent_common(void *private, struct sock *sk)
+static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
 {
 	struct skcipher_ctx *ctx;
 	struct alg_sock *ask = alg_sk(sk);
@@ -762,21 +739,7 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
 	if (!tfm->has_key && crypto_ablkcipher_has_setkey(tfm->skcipher))
 		return -ENOKEY;
 
-	return skcipher_accept_parent_common(private, sk);
-}
-
-static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
-{
-	int err;
-
-	err = skcipher_accept_parent_common(private, sk);
-	if (err)
-		goto out;
-
-	sk->sk_destruct = skcipher_sock_destruct_nokey;
-
-out:
-	return err;
+	return skcipher_accept_parent_nokey(private, sk);
 }
 
 static const struct af_alg_type algif_type_skcipher = {
-- 
2.7.3

^ permalink raw reply related

* [PATCH stable 3.10.y 08/18] crypto: algif_skcipher - Add key check exception for cipher_null
From: Andrey Ryabinin @ 2016-10-27 14:29 UTC (permalink / raw)
  To: Willy Tarreau, stable; +Cc: Herbert Xu, linux-crypto, Andrey Ryabinin
In-Reply-To: <1477578591-28924-1-git-send-email-aryabinin@virtuozzo.com>

From: Herbert Xu <herbert@gondor.apana.org.au>

commit 6e8d8ecf438792ecf7a3207488fb4eebc4edb040 upstream.

This patch adds an exception to the key check so that cipher_null
users may continue to use algif_skcipher without setting a key.

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/algif_skcipher.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index db5f0f0..4677a45 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -759,7 +759,7 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
 {
 	struct skcipher_tfm *tfm = private;
 
-	if (!tfm->has_key)
+	if (!tfm->has_key && crypto_ablkcipher_has_setkey(tfm->skcipher))
 		return -ENOKEY;
 
 	return skcipher_accept_parent_common(private, sk);
-- 
2.7.3

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox