[PATCH 3.10 003/319] crypto: algif_skcipher - Require setkey before accept(2)

2017-02-05 Thread Willy Tarreau 
From: Herbert Xu 

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: sta...@vger.kernel.org
Reported-by: Dmitry Vyukov 
Signed-off-by: Herbert Xu 
Tested-by: Dmitry Vyukov 
Signed-off-by: Andrey Ryabinin 
Signed-off-by: Willy Tarreau 
---
 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.8.0.rc2.1.gbe9624a

[PATCH 3.19.y-ckt 001/170] crypto: algif_skcipher - Require setkey before accept(2)

2016-04-01 Thread Kamal Mostafa 
3.19.8-ckt18 -stable review patch.  If anyone has any objections, please let me 
know.

---8<

From: Herbert Xu 

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.

Reported-by: Dmitry Vyukov 
Signed-off-by: Herbert Xu 
Tested-by: Dmitry Vyukov 
[ kamal: backport to 4.2-stable: crypto_alloc_ablkcipher API ]
Signed-off-by: Kamal Mostafa 
---
 crypto/algif_skcipher.c | 48 +---
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 2931d8e..e8eedce 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;
@@ -543,17 +548,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)
@@ -572,20 +601,25 @@ 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;
@@ -597,7 +631,7 @@ 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);
 
-- 
2.7.4

[PATCH 3.16.y-ckt 044/129] crypto: algif_skcipher - Require setkey before accept(2)

2016-02-26 Thread Luis Henriques 
3.16.7-ckt25 -stable review patch.  If anyone has any objections, please let me 
know.

---8<

From: Herbert Xu 

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.

Reported-by: Dmitry Vyukov 
Signed-off-by: Herbert Xu 
Tested-by: Dmitry Vyukov 
[bwh: Backported to 3.2: s/crypto_(alloc_|free_)?skcipher/crypto_\1ablkcipher/]
Signed-off-by: Ben Hutchings 
Signed-off-by: Luis Henriques 
---
 crypto/algif_skcipher.c | 48 +---
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index bb68c92e68c6..10c1fd04f1ed 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,25 @@ 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,7 +634,7 @@ 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);

[PATCH 3.12 059/142] crypto: algif_skcipher - Require setkey before accept(2)

2016-02-24 Thread Jiri Slaby 
From: Herbert Xu 

3.12-stable review patch.  If anyone has any objections, please let me know.

===

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.

Reported-by: Dmitry Vyukov 
Signed-off-by: Herbert Xu 
Tested-by: Dmitry Vyukov 
Signed-off-by: Jiri Slaby 
---
 crypto/algif_skcipher.c | 48 +---
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 83187f497c7c..377010cee09b 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,25 @@ 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,7 +634,7 @@ 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);
 
-- 
2.7.1

[PATCH 4.3 157/200] crypto: algif_skcipher - Require setkey before accept(2)

2016-02-14 Thread Greg Kroah-Hartman 
4.3-stable review patch.  If anyone has any objections, please let me know.

--

From: Herbert Xu 

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.

Reported-by: Dmitry Vyukov 
Signed-off-by: Herbert Xu 
Tested-by: Dmitry Vyukov 
Signed-off-by: Greg Kroah-Hartman 

---
 crypto/algif_skcipher.c |   48 +---
 1 file changed, 41 insertions(+), 7 deletions(-)

--- 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_skcipher *skcipher;
+   bool has_key;
+};
+
 struct skcipher_ctx {
struct list_head tsgl;
struct af_alg_sgl rsgl;
@@ -750,17 +755,41 @@ static struct proto_ops algif_skcipher_o
 
 static void *skcipher_bind(const char *name, u32 type, u32 mask)
 {
-   return crypto_alloc_skcipher(name, type, mask);
+   struct skcipher_tfm *tfm;
+   struct crypto_skcipher *skcipher;
+
+   tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
+   if (!tfm)
+   return ERR_PTR(-ENOMEM);
+
+   skcipher = crypto_alloc_skcipher(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_skcipher(private);
+   struct skcipher_tfm *tfm = private;
+
+   crypto_free_skcipher(tfm->skcipher);
+   kfree(tfm);
 }
 
 static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
 {
-   return crypto_skcipher_setkey(private, key, keylen);
+   struct skcipher_tfm *tfm = private;
+   int err;
+
+   err = crypto_skcipher_setkey(tfm->skcipher, key, keylen);
+   tfm->has_key = !err;
+
+   return err;
 }
 
 static void skcipher_wait(struct sock *sk)
@@ -792,20 +821,25 @@ static int skcipher_accept_parent(void *
 {
struct skcipher_ctx *ctx;
struct alg_sock *ask = alg_sk(sk);
-   unsigned int len = sizeof(*ctx) + crypto_skcipher_reqsize(private);
+   struct skcipher_tfm *tfm = private;
+   struct crypto_skcipher *skcipher = tfm->skcipher;
+   unsigned int len = sizeof(*ctx) + crypto_skcipher_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_skcipher_ivsize(private),
+   ctx->iv = sock_kmalloc(sk, crypto_skcipher_ivsize(skcipher),
   GFP_KERNEL);
if (!ctx->iv) {
sock_kfree_s(sk, ctx, len);
return -ENOMEM;
}
 
-   memset(ctx->iv, 0, crypto_skcipher_ivsize(private));
+   memset(ctx->iv, 0, crypto_skcipher_ivsize(skcipher));
 
INIT_LIST_HEAD(&ctx->tsgl);
ctx->len = len;
@@ -818,7 +852,7 @@ static int skcipher_accept_parent(void *
 
ask->private = ctx;
 
-   skcipher_request_set_tfm(&ctx->req, private);
+   skcipher_request_set_tfm(&ctx->req, skcipher);
skcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
  af_alg_complete, &ctx->completion);

[PATCH 4.4 080/117] crypto: algif_skcipher - Require setkey before accept(2)

2016-02-14 Thread Greg Kroah-Hartman 
4.4-stable review patch.  If anyone has any objections, please let me know.

--

From: Herbert Xu 

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.

Reported-by: Dmitry Vyukov 
Signed-off-by: Herbert Xu 
Tested-by: Dmitry Vyukov 
Signed-off-by: Greg Kroah-Hartman 

---
 crypto/algif_skcipher.c |   48 +---
 1 file changed, 41 insertions(+), 7 deletions(-)

--- 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_skcipher *skcipher;
+   bool has_key;
+};
+
 struct skcipher_ctx {
struct list_head tsgl;
struct af_alg_sgl rsgl;
@@ -750,17 +755,41 @@ static struct proto_ops algif_skcipher_o
 
 static void *skcipher_bind(const char *name, u32 type, u32 mask)
 {
-   return crypto_alloc_skcipher(name, type, mask);
+   struct skcipher_tfm *tfm;
+   struct crypto_skcipher *skcipher;
+
+   tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
+   if (!tfm)
+   return ERR_PTR(-ENOMEM);
+
+   skcipher = crypto_alloc_skcipher(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_skcipher(private);
+   struct skcipher_tfm *tfm = private;
+
+   crypto_free_skcipher(tfm->skcipher);
+   kfree(tfm);
 }
 
 static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
 {
-   return crypto_skcipher_setkey(private, key, keylen);
+   struct skcipher_tfm *tfm = private;
+   int err;
+
+   err = crypto_skcipher_setkey(tfm->skcipher, key, keylen);
+   tfm->has_key = !err;
+
+   return err;
 }
 
 static void skcipher_wait(struct sock *sk)
@@ -792,20 +821,25 @@ static int skcipher_accept_parent(void *
 {
struct skcipher_ctx *ctx;
struct alg_sock *ask = alg_sk(sk);
-   unsigned int len = sizeof(*ctx) + crypto_skcipher_reqsize(private);
+   struct skcipher_tfm *tfm = private;
+   struct crypto_skcipher *skcipher = tfm->skcipher;
+   unsigned int len = sizeof(*ctx) + crypto_skcipher_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_skcipher_ivsize(private),
+   ctx->iv = sock_kmalloc(sk, crypto_skcipher_ivsize(skcipher),
   GFP_KERNEL);
if (!ctx->iv) {
sock_kfree_s(sk, ctx, len);
return -ENOMEM;
}
 
-   memset(ctx->iv, 0, crypto_skcipher_ivsize(private));
+   memset(ctx->iv, 0, crypto_skcipher_ivsize(skcipher));
 
INIT_LIST_HEAD(&ctx->tsgl);
ctx->len = len;
@@ -818,7 +852,7 @@ static int skcipher_accept_parent(void *
 
ask->private = ctx;
 
-   skcipher_request_set_tfm(&ctx->req, private);
+   skcipher_request_set_tfm(&ctx->req, skcipher);
skcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
  af_alg_complete, &ctx->completion);

[PATCH 3.2 68/87] crypto: algif_skcipher - Require setkey before accept(2)

2016-02-08 Thread Ben Hutchings 
3.2.77-rc1 review patch.  If anyone has any objections, please let me know.

--

From: Herbert Xu 

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.

Reported-by: Dmitry Vyukov 
Signed-off-by: Herbert Xu 
Tested-by: Dmitry Vyukov 
[bwh: Backported to 3.2: s/crypto_(alloc_|free_)?skcipher/crypto_\1ablkcipher/]
Signed-off-by: Ben Hutchings 
---
 crypto/algif_skcipher.c | 48 +---
 1 file changed, 41 insertions(+), 7 deletions(-)

--- 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_o
 
 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,25 @@ static int skcipher_accept_parent(void *
 {
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,7 +634,7 @@ static int skcipher_accept_parent(void *
 
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);

[PATCH 4.2.y-ckt 148/268] crypto: algif_skcipher - Require setkey before accept(2)

2016-01-27 Thread Kamal Mostafa 
4.2.8-ckt3 -stable review patch.  If anyone has any objections, please let me 
know.

---8<

From: Herbert Xu 

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.

Reported-by: Dmitry Vyukov 
Signed-off-by: Herbert Xu 
Tested-by: Dmitry Vyukov 
[ kamal: backport to 4.2-stable: crypto_alloc_ablkcipher API ]
Signed-off-by: Kamal Mostafa 
---
 crypto/algif_skcipher.c | 48 +---
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 9450752..1e7e09b 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;
@@ -751,17 +756,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_wait(struct sock *sk)
@@ -793,20 +822,25 @@ 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;
@@ -819,7 +853,7 @@ 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);
 
-- 
1.9.1

Re: [PATCH v2] crypto: algif_skcipher - Require setkey before accept(2)

2016-01-03 Thread Milan Broz 
On 01/03/2016 02:31 AM, Herbert Xu wrote:
> On Sat, Jan 02, 2016 at 09:18:30PM +0100, Milan Broz wrote:
>>
>> But I cannot change thousands of cryptsetup installations that are actively 
>> using that code.
>> This is clear userspace breakage which should not happen this way.
> 
> I'll try to add some compatibility code for your case, assuming
> your modus operandi is accept(2) followed by a single setkey before
> proceeding to encryption/decryption.

Hi,

yes, basically it prepares socket()/bind()/accept() and then it calls setkey 
once.
(I'll try to fix in next releases to call setkey first though.)

I am doing exactly the same for AF_ALG HMAC (hmac() key,
does this requirement for order if accept/setkey applies there as well?
(It is not enforced yet.)

Anyway, you can easily simulate that skcipher API call just with running 
"cryptsetup benchmark"
(with accept() patch it will print N/A for all ciphers while without patch it 
measures some
more-or-less magic performance numbers :)

> 
>> (Moreover it still doesn't work for cipher_null that has min/max key size 0.)
> 
> Setkey works just fine on cipher_null.

Yes, it works if ALG_SET_KEY is set to zero-length key.
I just re-introduced old bug to code, sorry.

Thanks!
Milan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] crypto: algif_skcipher - Require setkey before accept(2)

2016-01-02 Thread Herbert Xu 
On Sat, Jan 02, 2016 at 09:18:30PM +0100, Milan Broz wrote:
>
> But I cannot change thousands of cryptsetup installations that are actively 
> using that code.
> This is clear userspace breakage which should not happen this way.

I'll try to add some compatibility code for your case, assuming
your modus operandi is accept(2) followed by a single setkey before
proceeding to encryption/decryption.

> (Moreover it still doesn't work for cipher_null that has min/max key size 0.)

Setkey works just fine on cipher_null.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] crypto: algif_skcipher - Require setkey before accept(2)

2016-01-02 Thread Milan Broz 
On 01/02/2016 09:03 PM, Stephan Mueller wrote:
> Am Samstag, 2. Januar 2016, 15:41:34 schrieb Milan Broz:
> 
> Hi Milan,
> 

...
>>> Hi Herbert,
>>>
>>> this patch breaks userspace in cryptsetup...
>>>
>>> We use algif_skcipher in cryptsetup (for years, even before
>>> there was Stephan's library) and with this patch applied
>>> I see fail in ALG_SET_IV call (patch from your git).
>>
>> (Obviously this was because of failing accept() call here, not set_iv.)
>>
>>> I can fix it upstream, but for thousands of installations it will
>>> be broken (for LUKS there is a fallback, cor TrueCrypt compatible devices
>>> it will be unusable. Also people who configured kernel crypto API as
>>> default backend will have non-working cryptsetup).
>>>
>>> Is it really thing for stable branch?
>>
>> Also how it is supposed to work for cipher_null, where there is no key?
>> Why it should call set_key if it is noop? (and set key length 0 is not
>> possible).
>>
>> (We are using cipher_null for testing and for offline re-encryption tool
>> to create temporary "fake" header for not-yet encrypted device...)
> 
> The change implies that any setkey or set IV operations (i.e. any operations 
> on the tfmfd) are done before the opfd(s) are created with one or more accept 
> calls.
> 
> Thus, after a bind that returns the tfmfd, the setkey and setiv operations 
> shall be called. This is followed by accept. If you change the order of 
> invocations in your code, it should work.

Hi,

I already changed it in cryptsetup upstream this way.

But I cannot change thousands of cryptsetup installations that are actively 
using that code.
This is clear userspace breakage which should not happen this way.

(Moreover it still doesn't work for cipher_null that has min/max key size 0.)

Milan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] crypto: algif_skcipher - Require setkey before accept(2)

2016-01-02 Thread Stephan Mueller 
Am Samstag, 2. Januar 2016, 15:41:34 schrieb Milan Broz:

Hi Milan,

> On 01/02/2016 12:52 PM, Milan Broz wrote:
> > On 12/25/2015 08:40 AM, Herbert Xu wrote:
> >> Dmitry Vyukov  wrote:
> >>> I am testing with your two patches:
> >>> crypto: algif_skcipher - Use new skcipher interface
> >>> crypto: algif_skcipher - Require setkey before accept(2)
> >>> on top of a88164345b81292b55a8d4829fdd35c8d611cd7d (Dec 23).
> >> 
> >> You sent the email to everyone on the original CC list except me.
> >> Please don't do that.
> >> 
> >>> Now the following program causes a bunch of use-after-frees and them
> >> 
> >>> kills kernel:
> >> Yes there is an obvious bug in the patch that Julia Lawall has
> >> responded to in another thread.  Here is a fixed version.
> >> 
> >> ---8<--
> >> 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.
> > 
> > Hi Herbert,
> > 
> > this patch breaks userspace in cryptsetup...
> > 
> > We use algif_skcipher in cryptsetup (for years, even before
> > there was Stephan's library) and with this patch applied
> > I see fail in ALG_SET_IV call (patch from your git).
> 
> (Obviously this was because of failing accept() call here, not set_iv.)
> 
> > I can fix it upstream, but for thousands of installations it will
> > be broken (for LUKS there is a fallback, cor TrueCrypt compatible devices
> > it will be unusable. Also people who configured kernel crypto API as
> > default backend will have non-working cryptsetup).
> > 
> > Is it really thing for stable branch?
> 
> Also how it is supposed to work for cipher_null, where there is no key?
> Why it should call set_key if it is noop? (and set key length 0 is not
> possible).
> 
> (We are using cipher_null for testing and for offline re-encryption tool
> to create temporary "fake" header for not-yet encrypted device...)

The change implies that any setkey or set IV operations (i.e. any operations 
on the tfmfd) are done before the opfd(s) are created with one or more accept 
calls.

Thus, after a bind that returns the tfmfd, the setkey and setiv operations 
shall be called. This is followed by accept. If you change the order of 
invocations in your code, it should work.
> 
> Milan


-- 
Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] crypto: algif_skcipher - Require setkey before accept(2)

2016-01-02 Thread Milan Broz 
On 01/02/2016 12:52 PM, Milan Broz wrote:
> On 12/25/2015 08:40 AM, Herbert Xu wrote:
>> Dmitry Vyukov  wrote:
>>>
>>> I am testing with your two patches:
>>> crypto: algif_skcipher - Use new skcipher interface
>>> crypto: algif_skcipher - Require setkey before accept(2)
>>> on top of a88164345b81292b55a8d4829fdd35c8d611cd7d (Dec 23).
>>
>> You sent the email to everyone on the original CC list except me.
>> Please don't do that.
>>
>>> Now the following program causes a bunch of use-after-frees and them
>>> kills kernel:
>>
>> Yes there is an obvious bug in the patch that Julia Lawall has
>> responded to in another thread.  Here is a fixed version.
>>
>> ---8<--
>> 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.
> 
> 
> Hi Herbert,
> 
> this patch breaks userspace in cryptsetup...
> 
> We use algif_skcipher in cryptsetup (for years, even before
> there was Stephan's library) and with this patch applied
> I see fail in ALG_SET_IV call (patch from your git).

(Obviously this was because of failing accept() call here, not set_iv.)

> 
> I can fix it upstream, but for thousands of installations it will
> be broken (for LUKS there is a fallback, cor TrueCrypt compatible devices
> it will be unusable. Also people who configured kernel crypto API as default
> backend will have non-working cryptsetup).
> 
> Is it really thing for stable branch?

Also how it is supposed to work for cipher_null, where there is no key?
Why it should call set_key if it is noop? (and set key length 0 is not 
possible).

(We are using cipher_null for testing and for offline re-encryption tool
to create temporary "fake" header for not-yet encrypted device...)

Milan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] crypto: algif_skcipher - Require setkey before accept(2)

2016-01-02 Thread Milan Broz 
On 12/25/2015 08:40 AM, Herbert Xu wrote:
> Dmitry Vyukov  wrote:
>>
>> I am testing with your two patches:
>> crypto: algif_skcipher - Use new skcipher interface
>> crypto: algif_skcipher - Require setkey before accept(2)
>> on top of a88164345b81292b55a8d4829fdd35c8d611cd7d (Dec 23).
> 
> You sent the email to everyone on the original CC list except me.
> Please don't do that.
> 
>> Now the following program causes a bunch of use-after-frees and them
>> kills kernel:
> 
> Yes there is an obvious bug in the patch that Julia Lawall has
> responded to in another thread.  Here is a fixed version.
> 
> ---8<--
> 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.


Hi Herbert,

this patch breaks userspace in cryptsetup...

We use algif_skcipher in cryptsetup (for years, even before
there was Stephan's library) and with this patch applied
I see fail in ALG_SET_IV call (patch from your git).

I can fix it upstream, but for thousands of installations it will
be broken (for LUKS there is a fallback, cor TrueCrypt compatible devices
it will be unusable. Also people who configured kernel crypto API as default
backend will have non-working cryptsetup).

Is it really thing for stable branch?

Milan

> 
> Cc: sta...@vger.kernel.org
> Reported-by: Dmitry Vyukov 
> Signed-off-by: Herbert Xu 
> 
> diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> index 5c756b3..f4431bc 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_skcipher *skcipher;
> + bool has_key;
> +};
> +
>  struct skcipher_ctx {
>   struct list_head tsgl;
>   struct af_alg_sgl rsgl;
> @@ -750,17 +755,41 @@ static struct proto_ops algif_skcipher_ops = {
>  
>  static void *skcipher_bind(const char *name, u32 type, u32 mask)
>  {
> - return crypto_alloc_skcipher(name, type, mask);
> + struct skcipher_tfm *tfm;
> + struct crypto_skcipher *skcipher;
> +
> + tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
> + if (!tfm)
> + return ERR_PTR(-ENOMEM);
> +
> + skcipher = crypto_alloc_skcipher(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_skcipher(private);
> + struct skcipher_tfm *tfm = private;
> +
> + crypto_free_skcipher(tfm->skcipher);
> + kfree(tfm);
>  }
>  
>  static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
>  {
> - return crypto_skcipher_setkey(private, key, keylen);
> + struct skcipher_tfm *tfm = private;
> + int err;
> +
> + err = crypto_skcipher_setkey(tfm->skcipher, key, keylen);
> + tfm->has_key = !err;
> +
> + return err;
>  }
>  
>  static void skcipher_wait(struct sock *sk)
> @@ -792,20 +821,25 @@ 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_skcipher_reqsize(private);
> + struct skcipher_tfm *tfm = private;
> + struct crypto_skcipher *skcipher = tfm->skcipher;
> + unsigned int len = sizeof(*ctx) + crypto_skcipher_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_skcipher_ivsize(private),
> + ctx->iv = sock_kmalloc(sk, crypto_skcipher_ivsize(skcipher),
>  GFP_KERNEL);
>   if (!ctx->iv) {
>   sock_kfree_s(sk, ctx, len);
>   return -ENOMEM;
>   }
>  
> - memset(ctx->iv, 0, crypto_skcipher_ivsize(private));
> + memset(ctx->iv, 0, crypto_skcipher_ivsize(skcipher));
>  
>   INIT_LIST_HEAD(&ctx->tsgl);
>   ctx->len = len;
> @@ -818,7 +852,7 @@ static int skcipher_accept_parent(void *private, struct 
> sock *sk)
>  
>   ask->private = ctx;
>  
> - skcipher_request_set_tfm(&ctx->req, private);
> + skcipher_request_set_tfm(&ctx->req, skcipher);
>   skcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> af_alg_complete, &ctx->completion);
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] crypto: algif_skcipher - Require setkey before accept(2)

2015-12-29 Thread Herbert Xu 
Dmitry Vyukov  wrote:
>
> My email client just followed instructions in your email. You've said
> to Reply-to: syzkal...@googlegroups.com.

I did not set this Reply-To header.  It's most likely set by your
broken Google Groups setup.  So either you should start actually
replying to my email address or your future emails will most
likely not be read by me.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] crypto: algif_skcipher - Require setkey before accept(2)

2015-12-28 Thread Dmitry Vyukov 
On Fri, Dec 25, 2015 at 8:40 AM, Herbert Xu  wrote:
> Dmitry Vyukov  wrote:
>>
>> I am testing with your two patches:
>> crypto: algif_skcipher - Use new skcipher interface
>> crypto: algif_skcipher - Require setkey before accept(2)
>> on top of a88164345b81292b55a8d4829fdd35c8d611cd7d (Dec 23).
>
> You sent the email to everyone on the original CC list except me.
> Please don't do that.

Hi Herbert,

My email client just followed instructions in your email. You've said
to Reply-to: syzkal...@googlegroups.com.

This version of the patch does fix the crash.

Tested-by: Dmitry Vyukov 

However, crypto is still considerably unstable. I will post reports
that I see separately.


>> Now the following program causes a bunch of use-after-frees and them
>> kills kernel:
>
> Yes there is an obvious bug in the patch that Julia Lawall has
> responded to in another thread.  Here is a fixed version.
>
> ---8<--
> 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: sta...@vger.kernel.org
> Reported-by: Dmitry Vyukov 
> Signed-off-by: Herbert Xu 
>
> diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> index 5c756b3..f4431bc 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_skcipher *skcipher;
> +   bool has_key;
> +};
> +
>  struct skcipher_ctx {
> struct list_head tsgl;
> struct af_alg_sgl rsgl;
> @@ -750,17 +755,41 @@ static struct proto_ops algif_skcipher_ops = {
>
>  static void *skcipher_bind(const char *name, u32 type, u32 mask)
>  {
> -   return crypto_alloc_skcipher(name, type, mask);
> +   struct skcipher_tfm *tfm;
> +   struct crypto_skcipher *skcipher;
> +
> +   tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
> +   if (!tfm)
> +   return ERR_PTR(-ENOMEM);
> +
> +   skcipher = crypto_alloc_skcipher(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_skcipher(private);
> +   struct skcipher_tfm *tfm = private;
> +
> +   crypto_free_skcipher(tfm->skcipher);
> +   kfree(tfm);
>  }
>
>  static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
>  {
> -   return crypto_skcipher_setkey(private, key, keylen);
> +   struct skcipher_tfm *tfm = private;
> +   int err;
> +
> +   err = crypto_skcipher_setkey(tfm->skcipher, key, keylen);
> +   tfm->has_key = !err;
> +
> +   return err;
>  }
>
>  static void skcipher_wait(struct sock *sk)
> @@ -792,20 +821,25 @@ 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_skcipher_reqsize(private);
> +   struct skcipher_tfm *tfm = private;
> +   struct crypto_skcipher *skcipher = tfm->skcipher;
> +   unsigned int len = sizeof(*ctx) + crypto_skcipher_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_skcipher_ivsize(private),
> +   ctx->iv = sock_kmalloc(sk, crypto_skcipher_ivsize(skcipher),
>GFP_KERNEL);
> if (!ctx->iv) {
> sock_kfree_s(sk, ctx, len);
> return -ENOMEM;
> }
>
> -   memset(ctx->iv, 0, crypto_skcipher_ivsize(private));
> +   memset(ctx->iv, 0, crypto_skcipher_ivsize(skcipher));
>
> INIT_LIST_HEAD(&ctx->tsgl);
> ctx->len = len;
> @@ -818,7 +852,7 @@ static int skcipher_accept_parent(void *private, struct 
> sock *sk)
>
> ask->private = ctx;
>
> -   skcipher_request_set_tfm(&ctx->req, private);
> +   skcipher_request_set_tfm(&ctx->req, skcipher);
> skcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
>   af_alg_complete, &ctx->completion);
>
> --
> Email: Herbert Xu

Re: crypto: algif_skcipher - Require setkey before accept(2)

2015-12-25 Thread Julia Lawall 


On Fri, 25 Dec 2015, Herbert Xu wrote:

> On Fri, Dec 25, 2015 at 07:54:48AM +0100, Julia Lawall wrote:
> > Lines 766, 767 don't look correct at all.
> 
> Thanks Julia.  I have sent an updated patch in the original thread.
> I wonder why I haven't received the original kbuild test robot
> email though.

I goes to me to check for false positives first.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: crypto: algif_skcipher - Require setkey before accept(2)

2015-12-24 Thread Herbert Xu 
On Fri, Dec 25, 2015 at 07:54:48AM +0100, Julia Lawall wrote:
> Lines 766, 767 don't look correct at all.

Thanks Julia.  I have sent an updated patch in the original thread.
I wonder why I haven't received the original kbuild test robot
email though.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2] crypto: algif_skcipher - Require setkey before accept(2)

2015-12-24 Thread Herbert Xu 
Dmitry Vyukov  wrote:
>
> I am testing with your two patches:
> crypto: algif_skcipher - Use new skcipher interface
> crypto: algif_skcipher - Require setkey before accept(2)
> on top of a88164345b81292b55a8d4829fdd35c8d611cd7d (Dec 23).

You sent the email to everyone on the original CC list except me.
Please don't do that.

> Now the following program causes a bunch of use-after-frees and them
> kills kernel:

Yes there is an obvious bug in the patch that Julia Lawall has
responded to in another thread.  Here is a fixed version.

---8<--
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: sta...@vger.kernel.org
Reported-by: Dmitry Vyukov 
Signed-off-by: Herbert Xu 

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 5c756b3..f4431bc 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_skcipher *skcipher;
+   bool has_key;
+};
+
 struct skcipher_ctx {
struct list_head tsgl;
struct af_alg_sgl rsgl;
@@ -750,17 +755,41 @@ static struct proto_ops algif_skcipher_ops = {
 
 static void *skcipher_bind(const char *name, u32 type, u32 mask)
 {
-   return crypto_alloc_skcipher(name, type, mask);
+   struct skcipher_tfm *tfm;
+   struct crypto_skcipher *skcipher;
+
+   tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
+   if (!tfm)
+   return ERR_PTR(-ENOMEM);
+
+   skcipher = crypto_alloc_skcipher(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_skcipher(private);
+   struct skcipher_tfm *tfm = private;
+
+   crypto_free_skcipher(tfm->skcipher);
+   kfree(tfm);
 }
 
 static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
 {
-   return crypto_skcipher_setkey(private, key, keylen);
+   struct skcipher_tfm *tfm = private;
+   int err;
+
+   err = crypto_skcipher_setkey(tfm->skcipher, key, keylen);
+   tfm->has_key = !err;
+
+   return err;
 }
 
 static void skcipher_wait(struct sock *sk)
@@ -792,20 +821,25 @@ 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_skcipher_reqsize(private);
+   struct skcipher_tfm *tfm = private;
+   struct crypto_skcipher *skcipher = tfm->skcipher;
+   unsigned int len = sizeof(*ctx) + crypto_skcipher_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_skcipher_ivsize(private),
+   ctx->iv = sock_kmalloc(sk, crypto_skcipher_ivsize(skcipher),
   GFP_KERNEL);
if (!ctx->iv) {
sock_kfree_s(sk, ctx, len);
return -ENOMEM;
}
 
-   memset(ctx->iv, 0, crypto_skcipher_ivsize(private));
+   memset(ctx->iv, 0, crypto_skcipher_ivsize(skcipher));
 
INIT_LIST_HEAD(&ctx->tsgl);
ctx->len = len;
@@ -818,7 +852,7 @@ static int skcipher_accept_parent(void *private, struct 
sock *sk)
 
ask->private = ctx;
 
-   skcipher_request_set_tfm(&ctx->req, private);
+   skcipher_request_set_tfm(&ctx->req, skcipher);
skcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
  af_alg_complete, &ctx->completion);
 
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: crypto: algif_skcipher - Require setkey before accept(2)

2015-12-24 Thread Julia Lawall 
Lines 766, 767 don't look correct at all.

julia

On Thu, 24 Dec 2015, kbuild test robot wrote:

> In-Reply-To: <20151224093902.ga8...@gondor.apana.org.au>
> 
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
> Hi Herbert,
> 
> [auto build test WARNING on crypto/master]
> [also build test WARNING on next-20151223]
> [cannot apply to v4.4-rc6]
> 
> url:
> https://github.com/0day-ci/linux/commits/Herbert-Xu/crypto-algif_skcipher-Require-setkey-before-accept-2/20151224-174142
> base:   
> https://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git master
> :: branch date: 63 minutes ago
> :: commit date: 63 minutes ago
> 
> >> crypto/algif_skcipher.c:767:18-21: ERROR: reference preceded by free on 
> >> line 766
> 
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 7f3e28b295bcfeb7fba928af15780f1bb3051430
> vim +767 crypto/algif_skcipher.c
> 
> 7f3e28b2 Herbert Xu 2015-12-24  760   tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
> 7f3e28b2 Herbert Xu 2015-12-24  761   if (!tfm)
> 7f3e28b2 Herbert Xu 2015-12-24  762   return ERR_PTR(-ENOMEM);
> 7f3e28b2 Herbert Xu 2015-12-24  763  
> 7f3e28b2 Herbert Xu 2015-12-24  764   tfm->skcipher = 
> crypto_alloc_skcipher(name, type, mask);
> 7f3e28b2 Herbert Xu 2015-12-24  765   if (IS_ERR(tfm->skcipher)) {
> 7f3e28b2 Herbert Xu 2015-12-24 @766   kfree(tfm);
> 7f3e28b2 Herbert Xu 2015-12-24 @767   return ERR_CAST(tfm->skcipher);
> 7f3e28b2 Herbert Xu 2015-12-24  768   }
> 7f3e28b2 Herbert Xu 2015-12-24  769  
> 7f3e28b2 Herbert Xu 2015-12-24  770   return tfm;
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/