[PATCH net-next] ppp: mppe: Remove VLA usage

2018-08-03 Thread Kees Cook
In the quest to remove all stack VLA usage from the kernel[1], this
removes the discouraged use of AHASH_REQUEST_ON_STACK (and associated
VLA) by switching to shash directly and keeping the associated descriptor
allocated with the regular state on the heap.

[1] 
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com

Signed-off-by: Kees Cook 
Acked-by: Arnd Bergmann 
---
Paul suggested this go via netdev directly:
https://lkml.kernel.org/r/20180803031315.GA30807@fergus
---
 drivers/net/ppp/ppp_mppe.c | 56 --
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index 6c7fd98cb00a..a205750b431b 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -96,7 +96,7 @@ static inline void sha_pad_init(struct sha_pad *shapad)
  */
 struct ppp_mppe_state {
struct crypto_skcipher *arc4;
-   struct crypto_ahash *sha1;
+   struct shash_desc *sha1;
unsigned char *sha1_digest;
unsigned char master_key[MPPE_MAX_KEY_LEN];
unsigned char session_key[MPPE_MAX_KEY_LEN];
@@ -136,25 +136,16 @@ struct ppp_mppe_state {
  */
 static void get_new_key_from_sha(struct ppp_mppe_state * state)
 {
-   AHASH_REQUEST_ON_STACK(req, state->sha1);
-   struct scatterlist sg[4];
-   unsigned int nbytes;
-
-   sg_init_table(sg, 4);
-
-   nbytes = setup_sg([0], state->master_key, state->keylen);
-   nbytes += setup_sg([1], sha_pad->sha_pad1,
-  sizeof(sha_pad->sha_pad1));
-   nbytes += setup_sg([2], state->session_key, state->keylen);
-   nbytes += setup_sg([3], sha_pad->sha_pad2,
-  sizeof(sha_pad->sha_pad2));
-
-   ahash_request_set_tfm(req, state->sha1);
-   ahash_request_set_callback(req, 0, NULL, NULL);
-   ahash_request_set_crypt(req, sg, state->sha1_digest, nbytes);
-
-   crypto_ahash_digest(req);
-   ahash_request_zero(req);
+   crypto_shash_init(state->sha1);
+   crypto_shash_update(state->sha1, state->master_key,
+   state->keylen);
+   crypto_shash_update(state->sha1, sha_pad->sha_pad1,
+   sizeof(sha_pad->sha_pad1));
+   crypto_shash_update(state->sha1, state->session_key,
+   state->keylen);
+   crypto_shash_update(state->sha1, sha_pad->sha_pad2,
+   sizeof(sha_pad->sha_pad2));
+   crypto_shash_final(state->sha1, state->sha1_digest);
 }
 
 /*
@@ -200,6 +191,7 @@ static void mppe_rekey(struct ppp_mppe_state * state, int 
initial_key)
 static void *mppe_alloc(unsigned char *options, int optlen)
 {
struct ppp_mppe_state *state;
+   struct crypto_shash *shash;
unsigned int digestsize;
 
if (optlen != CILEN_MPPE + sizeof(state->master_key) ||
@@ -217,13 +209,21 @@ static void *mppe_alloc(unsigned char *options, int 
optlen)
goto out_free;
}
 
-   state->sha1 = crypto_alloc_ahash("sha1", 0, CRYPTO_ALG_ASYNC);
-   if (IS_ERR(state->sha1)) {
-   state->sha1 = NULL;
+   shash = crypto_alloc_shash("sha1", 0, 0);
+   if (IS_ERR(shash))
+   goto out_free;
+
+   state->sha1 = kmalloc(sizeof(*state->sha1) +
+crypto_shash_descsize(shash),
+ GFP_KERNEL);
+   if (!state->sha1) {
+   crypto_free_shash(shash);
goto out_free;
}
+   state->sha1->tfm = shash;
+   state->sha1->flags = 0;
 
-   digestsize = crypto_ahash_digestsize(state->sha1);
+   digestsize = crypto_shash_digestsize(shash);
if (digestsize < MPPE_MAX_KEY_LEN)
goto out_free;
 
@@ -246,7 +246,10 @@ static void *mppe_alloc(unsigned char *options, int optlen)
 
 out_free:
kfree(state->sha1_digest);
-   crypto_free_ahash(state->sha1);
+   if (state->sha1) {
+   crypto_free_shash(state->sha1->tfm);
+   kzfree(state->sha1);
+   }
crypto_free_skcipher(state->arc4);
kfree(state);
 out:
@@ -261,7 +264,8 @@ static void mppe_free(void *arg)
struct ppp_mppe_state *state = (struct ppp_mppe_state *) arg;
if (state) {
kfree(state->sha1_digest);
-   crypto_free_ahash(state->sha1);
+   crypto_free_shash(state->sha1->tfm);
+   kzfree(state->sha1);
crypto_free_skcipher(state->arc4);
kfree(state);
}
-- 
2.17.1


-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-ppp" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ppp: mppe: Remove VLA usage

2018-08-02 Thread Kees Cook
On Mon, Jul 16, 2018 at 4:01 AM, Arnd Bergmann  wrote:
> On Mon, Jul 16, 2018 at 6:05 AM, Kees Cook  wrote:
>> In the quest to remove all stack VLA usage from the kernel[1], this
>> removes the discouraged use of AHASH_REQUEST_ON_STACK (and associated
>> VLA) by switching to shash directly and keeping the associated descriptor
>> allocated with the regular state on the heap.
>>
>> [1] 
>> https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com
>>
>> Signed-off-by: Kees Cook 
>
> I had concerns at first that this approach might make it slower, but
> upon reading through implementation of the shash_ahash_ implementation,
> I concluded that it can only be better than before, improving both
> performance and stack usage.
>
> Acked-by: Arnd Bergmann 

Paul,

Is this something you can take via your tree?

Thanks,

-Kees


-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-ppp" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 18/18] crypto: Remove AHASH_REQUEST_ON_STACK

2018-07-24 Thread Kees Cook
On Tue, Jul 24, 2018 at 10:31 AM, Joe Perches  wrote:
> On Tue, 2018-07-24 at 09:49 -0700, Kees Cook wrote:
>> All users of AHASH_REQUEST_ON_STACK have been removed from the kernel, so
>> drop it entirely so no VLAs get reintroduced by future users.
>
> checkpatch has a test for that.
> It could now be removed as well.
> ---
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 34e4683de7a3..a3517334d661 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -796,7 +796,7 @@ our $declaration_macros = qr{(?x:
> 
> (?:$Storage\s+)?(?:[A-Z_][A-Z0-9]*_){0,2}(?:DEFINE|DECLARE)(?:_[A-Z0-9]+){1,6}\s*\(|
> (?:$Storage\s+)?[HLP]?LIST_HEAD\s*\(|
> (?:$Storage\s+)?${Type}\s+uninitialized_var\s*\(|
> -   (?:SKCIPHER_REQUEST|SHASH_DESC|AHASH_REQUEST)_ON_STACK\s*\(
> +   (?:SKCIPHER_REQUEST|SHASH_DESC)_ON_STACK\s*\(
>  )};

Ah! Cool. I've added this now.

-Kees

-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-ppp" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 02/18] crypto: cbc: Remove VLA usage

2018-07-24 Thread Kees Cook
In the quest to remove all stack VLA usage from the kernel[1], this
uses the upper bounds on blocksize. Since this is always a cipher
blocksize, use the existing cipher max blocksize.

[1] 
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com

Signed-off-by: Kees Cook 
---
 include/crypto/cbc.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/crypto/cbc.h b/include/crypto/cbc.h
index f5b8bfc22e6d..47db0aac2ab9 100644
--- a/include/crypto/cbc.h
+++ b/include/crypto/cbc.h
@@ -113,7 +113,9 @@ static inline int crypto_cbc_decrypt_inplace(
unsigned int bsize = crypto_skcipher_blocksize(tfm);
unsigned int nbytes = walk->nbytes;
u8 *src = walk->src.virt.addr;
-   u8 last_iv[bsize];
+   u8 last_iv[MAX_CIPHER_BLOCKSIZE];
+
+   BUG_ON(bsize > sizeof(last_iv));
 
/* Start of the last block. */
src += nbytes - (nbytes & (bsize - 1)) - bsize;
-- 
2.17.1

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


[PATCH v6 03/18] crypto: hash: Remove VLA usage

2018-07-24 Thread Kees Cook
In the quest to remove all stack VLA usage from the kernel[1], this
removes the VLAs in SHASH_DESC_ON_STACK (via crypto_shash_descsize())
by using the maximum allowable size (which is now more clearly captured
in a macro), along with a few other cases. Similar limits are turned into
macros as well.

A review of existing sizes shows that SHA512_DIGEST_SIZE (64) is the
largest digest size and that sizeof(struct sha3_state) (360) is the
largest descriptor size. The corresponding maximums are reduced.

[1] 
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com

Signed-off-by: Kees Cook 
---
 crypto/ahash.c| 4 ++--
 crypto/algif_hash.c   | 2 +-
 crypto/shash.c| 6 +++---
 include/crypto/hash.h | 6 +-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/crypto/ahash.c b/crypto/ahash.c
index a64c143165b1..78aaf2158c43 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -550,8 +550,8 @@ static int ahash_prepare_alg(struct ahash_alg *alg)
 {
struct crypto_alg *base = >halg.base;
 
-   if (alg->halg.digestsize > PAGE_SIZE / 8 ||
-   alg->halg.statesize > PAGE_SIZE / 8 ||
+   if (alg->halg.digestsize > HASH_MAX_DIGESTSIZE ||
+   alg->halg.statesize > HASH_MAX_STATESIZE ||
alg->halg.statesize == 0)
return -EINVAL;
 
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index bfcf595fd8f9..d0cde541beb6 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -239,7 +239,7 @@ static int hash_accept(struct socket *sock, struct socket 
*newsock, int flags,
struct alg_sock *ask = alg_sk(sk);
struct hash_ctx *ctx = ask->private;
struct ahash_request *req = >req;
-   char state[crypto_ahash_statesize(crypto_ahash_reqtfm(req)) ? : 1];
+   char state[HASH_MAX_STATESIZE];
struct sock *sk2;
struct alg_sock *ask2;
struct hash_ctx *ctx2;
diff --git a/crypto/shash.c b/crypto/shash.c
index 5d732c6bb4b2..86d76b5c626c 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -458,9 +458,9 @@ static int shash_prepare_alg(struct shash_alg *alg)
 {
struct crypto_alg *base = >base;
 
-   if (alg->digestsize > PAGE_SIZE / 8 ||
-   alg->descsize > PAGE_SIZE / 8 ||
-   alg->statesize > PAGE_SIZE / 8)
+   if (alg->digestsize > HASH_MAX_DIGESTSIZE ||
+   alg->descsize > HASH_MAX_DESCSIZE ||
+   alg->statesize > HASH_MAX_STATESIZE)
return -EINVAL;
 
base->cra_type = _shash_type;
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index 76e432cab75d..21587011ab0f 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -151,9 +151,13 @@ struct shash_desc {
void *__ctx[] CRYPTO_MINALIGN_ATTR;
 };
 
+#define HASH_MAX_DIGESTSIZE 64
+#define HASH_MAX_DESCSIZE  360
+#define HASH_MAX_STATESIZE 512
+
 #define SHASH_DESC_ON_STACK(shash, ctx)  \
char __##shash##_desc[sizeof(struct shash_desc) + \
-   crypto_shash_descsize(ctx)] CRYPTO_MINALIGN_ATTR; \
+   HASH_MAX_DESCSIZE] CRYPTO_MINALIGN_ATTR; \
struct shash_desc *shash = (struct shash_desc *)__##shash##_desc
 
 /**
-- 
2.17.1

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


[PATCH v6 06/18] crypto: qat: Remove VLA usage

2018-07-24 Thread Kees Cook
In the quest to remove all stack VLA usage from the kernel[1], this uses
the new upper bound for the stack buffer. Also adds a sanity check.

[1] 
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com

Signed-off-by: Kees Cook 
---
 drivers/crypto/qat/qat_common/qat_algs.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/qat_algs.c 
b/drivers/crypto/qat/qat_common/qat_algs.c
index 1138e41d6805..a28edf7b792f 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -153,8 +153,8 @@ static int qat_alg_do_precomputes(struct 
icp_qat_hw_auth_algo_blk *hash,
struct sha512_state sha512;
int block_size = crypto_shash_blocksize(ctx->hash_tfm);
int digest_size = crypto_shash_digestsize(ctx->hash_tfm);
-   char ipad[block_size];
-   char opad[block_size];
+   char ipad[MAX_ALGAPI_BLOCKSIZE];
+   char opad[MAX_ALGAPI_BLOCKSIZE];
__be32 *hash_state_out;
__be64 *hash512_state_out;
int i, offset;
@@ -164,6 +164,10 @@ static int qat_alg_do_precomputes(struct 
icp_qat_hw_auth_algo_blk *hash,
shash->tfm = ctx->hash_tfm;
shash->flags = 0x0;
 
+   if (WARN_ON(block_size > sizeof(ipad) ||
+   sizeof(ipad) != sizeof(opad)))
+   return -EINVAL;
+
if (auth_keylen > block_size) {
int ret = crypto_shash_digest(shash, auth_key,
  auth_keylen, ipad);
-- 
2.17.1

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


[PATCH v6 07/18] crypto: shash: Remove VLA usage in unaligned hashing

2018-07-24 Thread Kees Cook
In the quest to remove all stack VLA usage from the kernel[1], this uses
the newly defined max alignment to perform unaligned hashing to avoid
VLAs, and drops the helper function while adding sanity checks on the
resulting buffer sizes. Additionally, the __aligned_largest macro is
removed since this helper was the only user.

[1] 
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com

Signed-off-by: Kees Cook 
---
 crypto/shash.c   | 27 ---
 include/linux/compiler-gcc.h |  1 -
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/crypto/shash.c b/crypto/shash.c
index 86d76b5c626c..d21f04d70dce 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -73,13 +73,6 @@ int crypto_shash_setkey(struct crypto_shash *tfm, const u8 
*key,
 }
 EXPORT_SYMBOL_GPL(crypto_shash_setkey);
 
-static inline unsigned int shash_align_buffer_size(unsigned len,
-  unsigned long mask)
-{
-   typedef u8 __aligned_largest u8_aligned;
-   return len + (mask & ~(__alignof__(u8_aligned) - 1));
-}
-
 static int shash_update_unaligned(struct shash_desc *desc, const u8 *data,
  unsigned int len)
 {
@@ -88,11 +81,17 @@ static int shash_update_unaligned(struct shash_desc *desc, 
const u8 *data,
unsigned long alignmask = crypto_shash_alignmask(tfm);
unsigned int unaligned_len = alignmask + 1 -
 ((unsigned long)data & alignmask);
-   u8 ubuf[shash_align_buffer_size(unaligned_len, alignmask)]
-   __aligned_largest;
+   /*
+* We cannot count on __aligned() working for large values:
+* https://patchwork.kernel.org/patch/9507697/
+*/
+   u8 ubuf[MAX_ALGAPI_ALIGNMASK * 2];
u8 *buf = PTR_ALIGN([0], alignmask + 1);
int err;
 
+   if (WARN_ON(buf + unaligned_len > ubuf + sizeof(ubuf)))
+   return -EINVAL;
+
if (unaligned_len > len)
unaligned_len = len;
 
@@ -124,11 +123,17 @@ static int shash_final_unaligned(struct shash_desc *desc, 
u8 *out)
unsigned long alignmask = crypto_shash_alignmask(tfm);
struct shash_alg *shash = crypto_shash_alg(tfm);
unsigned int ds = crypto_shash_digestsize(tfm);
-   u8 ubuf[shash_align_buffer_size(ds, alignmask)]
-   __aligned_largest;
+   /*
+* We cannot count on __aligned() working for large values:
+* https://patchwork.kernel.org/patch/9507697/
+*/
+   u8 ubuf[MAX_ALGAPI_ALIGNMASK + HASH_MAX_DIGESTSIZE];
u8 *buf = PTR_ALIGN([0], alignmask + 1);
int err;
 
+   if (WARN_ON(buf + ds > ubuf + sizeof(ubuf)))
+   return -EINVAL;
+
err = shash->final(desc, buf);
if (err)
goto out;
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index f1a7492a5cc8..1f1cdef36a82 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -125,7 +125,6 @@
  */
 #define __pure __attribute__((pure))
 #define __aligned(x)   __attribute__((aligned(x)))
-#define __aligned_largest  __attribute__((aligned))
 #define __printf(a, b) __attribute__((format(printf, a, b)))
 #define __scanf(a, b)  __attribute__((format(scanf, a, b)))
 #define __attribute_const____attribute__((__const__))
-- 
2.17.1

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


[PATCH v6 09/18] ppp: mppe: Remove VLA usage

2018-07-24 Thread Kees Cook
In the quest to remove all stack VLA usage from the kernel[1], this
removes the discouraged use of AHASH_REQUEST_ON_STACK (and associated
VLA) by switching to shash directly and keeping the associated descriptor
allocated with the regular state on the heap.

[1] 
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com

Signed-off-by: Kees Cook 
Acked-by: Arnd Bergmann 
---
 drivers/net/ppp/ppp_mppe.c | 56 --
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index 6c7fd98cb00a..a205750b431b 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -96,7 +96,7 @@ static inline void sha_pad_init(struct sha_pad *shapad)
  */
 struct ppp_mppe_state {
struct crypto_skcipher *arc4;
-   struct crypto_ahash *sha1;
+   struct shash_desc *sha1;
unsigned char *sha1_digest;
unsigned char master_key[MPPE_MAX_KEY_LEN];
unsigned char session_key[MPPE_MAX_KEY_LEN];
@@ -136,25 +136,16 @@ struct ppp_mppe_state {
  */
 static void get_new_key_from_sha(struct ppp_mppe_state * state)
 {
-   AHASH_REQUEST_ON_STACK(req, state->sha1);
-   struct scatterlist sg[4];
-   unsigned int nbytes;
-
-   sg_init_table(sg, 4);
-
-   nbytes = setup_sg([0], state->master_key, state->keylen);
-   nbytes += setup_sg([1], sha_pad->sha_pad1,
-  sizeof(sha_pad->sha_pad1));
-   nbytes += setup_sg([2], state->session_key, state->keylen);
-   nbytes += setup_sg([3], sha_pad->sha_pad2,
-  sizeof(sha_pad->sha_pad2));
-
-   ahash_request_set_tfm(req, state->sha1);
-   ahash_request_set_callback(req, 0, NULL, NULL);
-   ahash_request_set_crypt(req, sg, state->sha1_digest, nbytes);
-
-   crypto_ahash_digest(req);
-   ahash_request_zero(req);
+   crypto_shash_init(state->sha1);
+   crypto_shash_update(state->sha1, state->master_key,
+   state->keylen);
+   crypto_shash_update(state->sha1, sha_pad->sha_pad1,
+   sizeof(sha_pad->sha_pad1));
+   crypto_shash_update(state->sha1, state->session_key,
+   state->keylen);
+   crypto_shash_update(state->sha1, sha_pad->sha_pad2,
+   sizeof(sha_pad->sha_pad2));
+   crypto_shash_final(state->sha1, state->sha1_digest);
 }
 
 /*
@@ -200,6 +191,7 @@ static void mppe_rekey(struct ppp_mppe_state * state, int 
initial_key)
 static void *mppe_alloc(unsigned char *options, int optlen)
 {
struct ppp_mppe_state *state;
+   struct crypto_shash *shash;
unsigned int digestsize;
 
if (optlen != CILEN_MPPE + sizeof(state->master_key) ||
@@ -217,13 +209,21 @@ static void *mppe_alloc(unsigned char *options, int 
optlen)
goto out_free;
}
 
-   state->sha1 = crypto_alloc_ahash("sha1", 0, CRYPTO_ALG_ASYNC);
-   if (IS_ERR(state->sha1)) {
-   state->sha1 = NULL;
+   shash = crypto_alloc_shash("sha1", 0, 0);
+   if (IS_ERR(shash))
+   goto out_free;
+
+   state->sha1 = kmalloc(sizeof(*state->sha1) +
+crypto_shash_descsize(shash),
+ GFP_KERNEL);
+   if (!state->sha1) {
+   crypto_free_shash(shash);
goto out_free;
}
+   state->sha1->tfm = shash;
+   state->sha1->flags = 0;
 
-   digestsize = crypto_ahash_digestsize(state->sha1);
+   digestsize = crypto_shash_digestsize(shash);
if (digestsize < MPPE_MAX_KEY_LEN)
goto out_free;
 
@@ -246,7 +246,10 @@ static void *mppe_alloc(unsigned char *options, int optlen)
 
 out_free:
kfree(state->sha1_digest);
-   crypto_free_ahash(state->sha1);
+   if (state->sha1) {
+   crypto_free_shash(state->sha1->tfm);
+   kzfree(state->sha1);
+   }
crypto_free_skcipher(state->arc4);
kfree(state);
 out:
@@ -261,7 +264,8 @@ static void mppe_free(void *arg)
struct ppp_mppe_state *state = (struct ppp_mppe_state *) arg;
if (state) {
kfree(state->sha1_digest);
-   crypto_free_ahash(state->sha1);
+   crypto_free_shash(state->sha1->tfm);
+   kzfree(state->sha1);
crypto_free_skcipher(state->arc4);
kfree(state);
}
-- 
2.17.1

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


[PATCH v6 10/18] x86/power/64: Remove VLA usage

2018-07-24 Thread Kees Cook
In the quest to remove all stack VLA usage from the kernel[1], this
removes the discouraged use of AHASH_REQUEST_ON_STACK by switching to
shash directly and allocating the descriptor in heap memory (which should
be fine: the tfm has already been allocated there too).

[1] 
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com

Signed-off-by: Kees Cook 
Acked-by: Pavel Machek 
---
 arch/x86/power/hibernate_64.c | 36 ---
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/arch/x86/power/hibernate_64.c b/arch/x86/power/hibernate_64.c
index 67ccf64c8bd8..f8e3b668d20b 100644
--- a/arch/x86/power/hibernate_64.c
+++ b/arch/x86/power/hibernate_64.c
@@ -233,29 +233,35 @@ struct restore_data_record {
  */
 static int get_e820_md5(struct e820_table *table, void *buf)
 {
-   struct scatterlist sg;
-   struct crypto_ahash *tfm;
+   struct crypto_shash *tfm;
+   struct shash_desc *desc;
int size;
int ret = 0;
 
-   tfm = crypto_alloc_ahash("md5", 0, CRYPTO_ALG_ASYNC);
+   tfm = crypto_alloc_shash("md5", 0, 0);
if (IS_ERR(tfm))
return -ENOMEM;
 
-   {
-   AHASH_REQUEST_ON_STACK(req, tfm);
-   size = offsetof(struct e820_table, entries) + sizeof(struct 
e820_entry) * table->nr_entries;
-   ahash_request_set_tfm(req, tfm);
-   sg_init_one(, (u8 *)table, size);
-   ahash_request_set_callback(req, 0, NULL, NULL);
-   ahash_request_set_crypt(req, , buf, size);
-
-   if (crypto_ahash_digest(req))
-   ret = -EINVAL;
-   ahash_request_zero(req);
+   desc = kmalloc(sizeof(struct shash_desc) + crypto_shash_descsize(tfm),
+  GFP_KERNEL);
+   if (!desc) {
+   ret = -ENOMEM;
+   goto free_tfm;
}
-   crypto_free_ahash(tfm);
 
+   desc->tfm = tfm;
+   desc->flags = 0;
+
+   size = offsetof(struct e820_table, entries) +
+   sizeof(struct e820_entry) * table->nr_entries;
+
+   if (crypto_shash_digest(desc, (u8 *)table, size, buf))
+   ret = -EINVAL;
+
+   kzfree(desc);
+
+free_tfm:
+   crypto_free_shash(tfm);
return ret;
 }
 
-- 
2.17.1

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


[PATCH v6 13/18] wireless/lib80211: Convert from ahash to shash

2018-07-24 Thread Kees Cook
In preparing to remove all stack VLA usage from the kernel[1], this
removes the discouraged use of AHASH_REQUEST_ON_STACK in favor of
the smaller SHASH_DESC_ON_STACK by converting from ahash-wrapped-shash
to direct shash. By removing a layer of indirection this both improves
performance and reduces stack usage. The stack allocation will be made
a fixed size in a later patch to the crypto subsystem.

[1] 
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com

Signed-off-by: Kees Cook 
---
 net/wireless/lib80211_crypt_tkip.c | 58 +++---
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/net/wireless/lib80211_crypt_tkip.c 
b/net/wireless/lib80211_crypt_tkip.c
index ba0a1f398ce5..21040aba3a81 100644
--- a/net/wireless/lib80211_crypt_tkip.c
+++ b/net/wireless/lib80211_crypt_tkip.c
@@ -65,9 +65,9 @@ struct lib80211_tkip_data {
int key_idx;
 
struct crypto_skcipher *rx_tfm_arc4;
-   struct crypto_ahash *rx_tfm_michael;
+   struct crypto_shash *rx_tfm_michael;
struct crypto_skcipher *tx_tfm_arc4;
-   struct crypto_ahash *tx_tfm_michael;
+   struct crypto_shash *tx_tfm_michael;
 
/* scratch buffers for virt_to_page() (crypto API) */
u8 rx_hdr[16], tx_hdr[16];
@@ -106,8 +106,7 @@ static void *lib80211_tkip_init(int key_idx)
goto fail;
}
 
-   priv->tx_tfm_michael = crypto_alloc_ahash("michael_mic", 0,
- CRYPTO_ALG_ASYNC);
+   priv->tx_tfm_michael = crypto_alloc_shash("michael_mic", 0, 0);
if (IS_ERR(priv->tx_tfm_michael)) {
priv->tx_tfm_michael = NULL;
goto fail;
@@ -120,8 +119,7 @@ static void *lib80211_tkip_init(int key_idx)
goto fail;
}
 
-   priv->rx_tfm_michael = crypto_alloc_ahash("michael_mic", 0,
- CRYPTO_ALG_ASYNC);
+   priv->rx_tfm_michael = crypto_alloc_shash("michael_mic", 0, 0);
if (IS_ERR(priv->rx_tfm_michael)) {
priv->rx_tfm_michael = NULL;
goto fail;
@@ -131,9 +129,9 @@ static void *lib80211_tkip_init(int key_idx)
 
   fail:
if (priv) {
-   crypto_free_ahash(priv->tx_tfm_michael);
+   crypto_free_shash(priv->tx_tfm_michael);
crypto_free_skcipher(priv->tx_tfm_arc4);
-   crypto_free_ahash(priv->rx_tfm_michael);
+   crypto_free_shash(priv->rx_tfm_michael);
crypto_free_skcipher(priv->rx_tfm_arc4);
kfree(priv);
}
@@ -145,9 +143,9 @@ static void lib80211_tkip_deinit(void *priv)
 {
struct lib80211_tkip_data *_priv = priv;
if (_priv) {
-   crypto_free_ahash(_priv->tx_tfm_michael);
+   crypto_free_shash(_priv->tx_tfm_michael);
crypto_free_skcipher(_priv->tx_tfm_arc4);
-   crypto_free_ahash(_priv->rx_tfm_michael);
+   crypto_free_shash(_priv->rx_tfm_michael);
crypto_free_skcipher(_priv->rx_tfm_arc4);
}
kfree(priv);
@@ -510,29 +508,31 @@ static int lib80211_tkip_decrypt(struct sk_buff *skb, int 
hdr_len, void *priv)
return keyidx;
 }
 
-static int michael_mic(struct crypto_ahash *tfm_michael, u8 * key, u8 * hdr,
-  u8 * data, size_t data_len, u8 * mic)
+static int michael_mic(struct crypto_shash *tfm_michael, u8 *key, u8 *hdr,
+  u8 *data, size_t data_len, u8 *mic)
 {
-   AHASH_REQUEST_ON_STACK(req, tfm_michael);
-   struct scatterlist sg[2];
+   SHASH_DESC_ON_STACK(desc, tfm_michael);
int err;
 
-   if (tfm_michael == NULL) {
-   pr_warn("%s(): tfm_michael == NULL\n", __func__);
-   return -1;
-   }
-   sg_init_table(sg, 2);
-   sg_set_buf([0], hdr, 16);
-   sg_set_buf([1], data, data_len);
+   desc->tfm = tfm_michael;
+   desc->flags = 0;
 
-   if (crypto_ahash_setkey(tfm_michael, key, 8))
+   if (crypto_shash_setkey(tfm_michael, key, 8))
return -1;
 
-   ahash_request_set_tfm(req, tfm_michael);
-   ahash_request_set_callback(req, 0, NULL, NULL);
-   ahash_request_set_crypt(req, sg, mic, data_len + 16);
-   err = crypto_ahash_digest(req);
-   ahash_request_zero(req);
+   err = crypto_shash_init(desc);
+   if (err)
+   goto out;
+   err = crypto_shash_update(desc, hdr, 16);
+   if (err)
+   goto out;
+   err = crypto_shash_update(desc, data, data_len);
+   if (err)
+   goto out;
+   err = crypto_shash_final(desc, mic);
+
+out:
+   shash_desc_zero(desc);
return err;
 }
 
@@ -654,9 +654,9 @@ static int lib80211_tkip_set_key(void *key, int len, u8 * 
seq, void *

[PATCH v6 12/18] drbd: Convert from ahash to shash

2018-07-24 Thread Kees Cook
In preparing to remove all stack VLA usage from the kernel[1], this
removes the discouraged use of AHASH_REQUEST_ON_STACK in favor of
the smaller SHASH_DESC_ON_STACK by converting from ahash-wrapped-shash
to direct shash. By removing a layer of indirection this both improves
performance and reduces stack usage. The stack allocation will be made
a fixed size in a later patch to the crypto subsystem.

[1] 
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com

Signed-off-by: Kees Cook 
---
 drivers/block/drbd/drbd_int.h  | 13 +++
 drivers/block/drbd/drbd_main.c | 14 
 drivers/block/drbd/drbd_nl.c   | 39 +++--
 drivers/block/drbd/drbd_receiver.c | 35 ++-
 drivers/block/drbd/drbd_worker.c   | 56 +-
 5 files changed, 69 insertions(+), 88 deletions(-)

diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index bc4ed2ed40a2..97d8e290c2be 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -726,10 +726,10 @@ struct drbd_connection {
struct list_head transfer_log;  /* all requests not yet fully processed 
*/
 
struct crypto_shash *cram_hmac_tfm;
-   struct crypto_ahash *integrity_tfm;  /* checksums we compute, updates 
protected by connection->data->mutex */
-   struct crypto_ahash *peer_integrity_tfm;  /* checksums we verify, only 
accessed from receiver thread  */
-   struct crypto_ahash *csums_tfm;
-   struct crypto_ahash *verify_tfm;
+   struct crypto_shash *integrity_tfm;  /* checksums we compute, updates 
protected by connection->data->mutex */
+   struct crypto_shash *peer_integrity_tfm;  /* checksums we verify, only 
accessed from receiver thread  */
+   struct crypto_shash *csums_tfm;
+   struct crypto_shash *verify_tfm;
void *int_dig_in;
void *int_dig_vv;
 
@@ -1533,8 +1533,9 @@ static inline void ov_out_of_sync_print(struct 
drbd_device *device)
 }
 
 
-extern void drbd_csum_bio(struct crypto_ahash *, struct bio *, void *);
-extern void drbd_csum_ee(struct crypto_ahash *, struct drbd_peer_request *, 
void *);
+extern void drbd_csum_bio(struct crypto_shash *, struct bio *, void *);
+extern void drbd_csum_ee(struct crypto_shash *, struct drbd_peer_request *,
+void *);
 /* worker callbacks */
 extern int w_e_end_data_req(struct drbd_work *, int);
 extern int w_e_end_rsdata_req(struct drbd_work *, int);
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index a80809bd3057..ccb54791d39c 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -1377,7 +1377,7 @@ void drbd_send_ack_dp(struct drbd_peer_device 
*peer_device, enum drbd_packet cmd
  struct p_data *dp, int data_size)
 {
if (peer_device->connection->peer_integrity_tfm)
-   data_size -= 
crypto_ahash_digestsize(peer_device->connection->peer_integrity_tfm);
+   data_size -= 
crypto_shash_digestsize(peer_device->connection->peer_integrity_tfm);
_drbd_send_ack(peer_device, cmd, dp->sector, cpu_to_be32(data_size),
   dp->block_id);
 }
@@ -1690,7 +1690,7 @@ int drbd_send_dblock(struct drbd_peer_device 
*peer_device, struct drbd_request *
sock = _device->connection->data;
p = drbd_prepare_command(peer_device, sock);
digest_size = peer_device->connection->integrity_tfm ?
- 
crypto_ahash_digestsize(peer_device->connection->integrity_tfm) : 0;
+ 
crypto_shash_digestsize(peer_device->connection->integrity_tfm) : 0;
 
if (!p)
return -EIO;
@@ -1796,7 +1796,7 @@ int drbd_send_block(struct drbd_peer_device *peer_device, 
enum drbd_packet cmd,
p = drbd_prepare_command(peer_device, sock);
 
digest_size = peer_device->connection->integrity_tfm ?
- 
crypto_ahash_digestsize(peer_device->connection->integrity_tfm) : 0;
+ 
crypto_shash_digestsize(peer_device->connection->integrity_tfm) : 0;
 
if (!p)
return -EIO;
@@ -2561,11 +2561,11 @@ void conn_free_crypto(struct drbd_connection 
*connection)
 {
drbd_free_sock(connection);
 
-   crypto_free_ahash(connection->csums_tfm);
-   crypto_free_ahash(connection->verify_tfm);
+   crypto_free_shash(connection->csums_tfm);
+   crypto_free_shash(connection->verify_tfm);
crypto_free_shash(connection->cram_hmac_tfm);
-   crypto_free_ahash(connection->integrity_tfm);
-   crypto_free_ahash(connection->peer_integrity_tfm);
+   crypto_free_shash(connection->integrity_tfm);
+   crypto_free_shash(connection->peer_integrity_tfm);
kfree(connection->int_dig_in);
kfree(connection->int_dig_vv);
 
diff --git a/drivers/

[PATCH v6 05/18] crypto alg: Introduce generic max blocksize and alignmask

2018-07-24 Thread Kees Cook
In the quest to remove all stack VLA usage from the kernel[1], this
exposes a new general upper bound on crypto blocksize and alignmask
(higher than for the existing cipher limits) for VLA removal,
and introduces new checks.

At present, the highest cra_alignmask in the kernel is 63. The highest
cra_blocksize is 144 (SHA3_224_BLOCK_SIZE, 18 8-byte words). For the
new blocksize limit, I went with 160 (20 8-byte words).

[1] 
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com

Signed-off-by: Kees Cook 
---
 crypto/algapi.c | 7 ++-
 include/crypto/algapi.h | 4 +++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/crypto/algapi.c b/crypto/algapi.c
index c0755cf4f53f..496fc51bf215 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -57,9 +57,14 @@ static int crypto_check_alg(struct crypto_alg *alg)
if (alg->cra_alignmask & (alg->cra_alignmask + 1))
return -EINVAL;
 
-   if (alg->cra_blocksize > PAGE_SIZE / 8)
+   /* General maximums for all algs. */
+   if (alg->cra_alignmask > MAX_ALGAPI_ALIGNMASK)
return -EINVAL;
 
+   if (alg->cra_blocksize > MAX_ALGAPI_BLOCKSIZE)
+   return -EINVAL;
+
+   /* Lower maximums for specific alg types. */
if (!alg->cra_type && (alg->cra_flags & CRYPTO_ALG_TYPE_MASK) ==
   CRYPTO_ALG_TYPE_CIPHER) {
if (alg->cra_alignmask > MAX_CIPHER_ALIGNMASK)
diff --git a/include/crypto/algapi.h b/include/crypto/algapi.h
index bd5e8ccf1687..21371ac8f355 100644
--- a/include/crypto/algapi.h
+++ b/include/crypto/algapi.h
@@ -20,8 +20,10 @@
 /*
  * Maximum values for blocksize and alignmask, used to allocate
  * static buffers that are big enough for any combination of
- * ciphers and architectures.
+ * algs and architectures. Ciphers have a lower maximum size.
  */
+#define MAX_ALGAPI_BLOCKSIZE   160
+#define MAX_ALGAPI_ALIGNMASK   63
 #define MAX_CIPHER_BLOCKSIZE   16
 #define MAX_CIPHER_ALIGNMASK   15
 
-- 
2.17.1

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


[PATCH] ppp: mppe: Remove VLA usage

2018-07-15 Thread Kees Cook
In the quest to remove all stack VLA usage from the kernel[1], this
removes the discouraged use of AHASH_REQUEST_ON_STACK (and associated
VLA) by switching to shash directly and keeping the associated descriptor
allocated with the regular state on the heap.

[1] 
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com

Signed-off-by: Kees Cook 
---
 drivers/net/ppp/ppp_mppe.c | 57 +-
 1 file changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index 6c7fd98cb00a..5b4b81027a75 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -96,7 +96,7 @@ static inline void sha_pad_init(struct sha_pad *shapad)
  */
 struct ppp_mppe_state {
struct crypto_skcipher *arc4;
-   struct crypto_ahash *sha1;
+   struct shash_desc *sha1;
unsigned char *sha1_digest;
unsigned char master_key[MPPE_MAX_KEY_LEN];
unsigned char session_key[MPPE_MAX_KEY_LEN];
@@ -136,25 +136,16 @@ struct ppp_mppe_state {
  */
 static void get_new_key_from_sha(struct ppp_mppe_state * state)
 {
-   AHASH_REQUEST_ON_STACK(req, state->sha1);
-   struct scatterlist sg[4];
-   unsigned int nbytes;
-
-   sg_init_table(sg, 4);
-
-   nbytes = setup_sg([0], state->master_key, state->keylen);
-   nbytes += setup_sg([1], sha_pad->sha_pad1,
-  sizeof(sha_pad->sha_pad1));
-   nbytes += setup_sg([2], state->session_key, state->keylen);
-   nbytes += setup_sg([3], sha_pad->sha_pad2,
-  sizeof(sha_pad->sha_pad2));
-
-   ahash_request_set_tfm(req, state->sha1);
-   ahash_request_set_callback(req, 0, NULL, NULL);
-   ahash_request_set_crypt(req, sg, state->sha1_digest, nbytes);
-
-   crypto_ahash_digest(req);
-   ahash_request_zero(req);
+   crypto_shash_init(state->sha1);
+   crypto_shash_update(state->sha1, state->master_key,
+   state->keylen);
+   crypto_shash_update(state->sha1, sha_pad->sha_pad1,
+   sizeof(sha_pad->sha_pad1));
+   crypto_shash_update(state->sha1, state->session_key,
+   state->keylen);
+   crypto_shash_update(state->sha1, sha_pad->sha_pad2,
+   sizeof(sha_pad->sha_pad2));
+   crypto_shash_final(state->sha1, state->sha1_digest);
 }
 
 /*
@@ -200,6 +191,7 @@ static void mppe_rekey(struct ppp_mppe_state * state, int 
initial_key)
 static void *mppe_alloc(unsigned char *options, int optlen)
 {
struct ppp_mppe_state *state;
+   struct crypto_shash *shash;
unsigned int digestsize;
 
if (optlen != CILEN_MPPE + sizeof(state->master_key) ||
@@ -217,13 +209,21 @@ static void *mppe_alloc(unsigned char *options, int 
optlen)
goto out_free;
}
 
-   state->sha1 = crypto_alloc_ahash("sha1", 0, CRYPTO_ALG_ASYNC);
-   if (IS_ERR(state->sha1)) {
-   state->sha1 = NULL;
+   shash = crypto_alloc_shash("sha1", 0, 0);
+   if (IS_ERR(shash))
+   goto out_free;
+
+   state->sha1 = kmalloc(sizeof(*state->sha1) +
+crypto_shash_descsize(shash),
+ GFP_KERNEL);
+   if (!state->sha1) {
+   crypto_free_shash(shash);
goto out_free;
}
+   state->sha1->tfm = shash;
+   state->sha1->flags = 0;
 
-   digestsize = crypto_ahash_digestsize(state->sha1);
+   digestsize = crypto_shash_digestsize(shash);
if (digestsize < MPPE_MAX_KEY_LEN)
goto out_free;
 
@@ -246,7 +246,11 @@ static void *mppe_alloc(unsigned char *options, int optlen)
 
 out_free:
kfree(state->sha1_digest);
-   crypto_free_ahash(state->sha1);
+   if (state->sha1) {
+   if (state->sha1->tfm)
+   crypto_free_shash(state->sha1->tfm);
+   kzfree(state->sha1);
+   }
crypto_free_skcipher(state->arc4);
kfree(state);
 out:
@@ -261,7 +265,8 @@ static void mppe_free(void *arg)
struct ppp_mppe_state *state = (struct ppp_mppe_state *) arg;
if (state) {
kfree(state->sha1_digest);
-   crypto_free_ahash(state->sha1);
+   crypto_free_shash(state->sha1->tfm);
+   kzfree(state->sha1);
crypto_free_skcipher(state->arc4);
kfree(state);
}
-- 
2.17.1


-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-ppp" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html