From patchwork Fri Apr 27 02:57:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 10367389 X-Patchwork-Delegate: herbert@gondor.apana.org.au Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1817E6016C for ; Fri, 27 Apr 2018 02:57:46 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0669F28A26 for ; Fri, 27 Apr 2018 02:57:46 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id EE40728A80; Fri, 27 Apr 2018 02:57:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 58A1928A26 for ; Fri, 27 Apr 2018 02:57:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757268AbeD0C5c (ORCPT ); Thu, 26 Apr 2018 22:57:32 -0400 Received: from mail-pg0-f65.google.com ([74.125.83.65]:37403 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757220AbeD0C5b (ORCPT ); Thu, 26 Apr 2018 22:57:31 -0400 Received: by mail-pg0-f65.google.com with SMTP id a13-v6so404676pgu.4 for ; Thu, 26 Apr 2018 19:57:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:mime-version:content-disposition; bh=N0kG9KCiRQec54/zUcv1ScG68H4AQjbCE73EBp3vOxs=; b=NB1q/BHW5hOOq3CLGTNxY6N/5QEfCvDyj19wLCqTdtLy4RWlrgboRIEyCU3StnI3ba toXl0gKLmveZzW5sznNUIZY2NCUZO83Cmh+r2lK47n1bZM7LKgEwvvnp8Zs+fOVmy3fn ZjS+MjN3QXcaopyqWFQtAFnssUL0lZwUy1uaY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition; bh=N0kG9KCiRQec54/zUcv1ScG68H4AQjbCE73EBp3vOxs=; b=QEp++255uKbeNJerS6uS74mazbbn799Suh1w22HDyt7p/1/dBok+gPyypOjeyrFzeE N0F+zrY+yTR6QeyDE+3UlCBqgYbhhRrNQzlDjB0cdGOl01THOIhYe2tCXL+sq1kfz91m pOzcgMWKvJryq4drMfeaNVJtZpPlEBL7nCv5nHfIc0MVM2KRGECfIP60h24wUHuXgUaq 8MbEF2xu+qZMUZheUoNod+yPRLRCKvIHoNSj9+DqKKeZJ8T1TZQ15KjPnreyohqT0F3F wzzq+flxYn56qXcjuM8YOtBxfweqeUegoiK8Hz0AFwZrLRjmakqhQhoaLds0ANR9hUE9 w6UA== X-Gm-Message-State: ALQs6tB4/FvtEa2Wu03WcobCoPYZ3w22euDmqKynZKRlR4JW8ocyVILo x6q/jcYXqrV9JuvXy9YwbOY7Qon+wsM= X-Google-Smtp-Source: AB8JxZpty39qfZFbBmox2cGtjgOWDAS/w0Apl9elmKPmhy3yArVf6szswzeaCfbvfQVgtP3ZL8cfmA== X-Received: by 2002:a63:5f4f:: with SMTP id t76-v6mr567164pgb.108.1524797850863; Thu, 26 Apr 2018 19:57:30 -0700 (PDT) Received: from www.outflux.net (173-164-112-133-Oregon.hfc.comcastbusiness.net. [173.164.112.133]) by smtp.gmail.com with ESMTPSA id b26sm364688pfn.73.2018.04.26.19.57.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 26 Apr 2018 19:57:29 -0700 (PDT) Date: Thu, 26 Apr 2018 19:57:28 -0700 From: Kees Cook To: Herbert Xu Cc: Gilad Ben-Yossef , linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] crypto: tcrypt: Remove VLA usage Message-ID: <20180427025728.GA38607@beast> MIME-Version: 1.0 Content-Disposition: inline Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP In the quest to remove all stack VLA usage from the kernel[1], this allocates the return code buffers before starting jiffie timers, rather than using stack space for the array. Additionally cleans up some exit paths and make sure that the num_mb module_param() is used only once per execution to avoid possible races in the value changing. [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com Signed-off-by: Kees Cook --- crypto/tcrypt.c | 118 +++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 79 insertions(+), 39 deletions(-) diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c index 51fe7c8744ae..e721faab6fc8 100644 --- a/crypto/tcrypt.c +++ b/crypto/tcrypt.c @@ -158,9 +158,9 @@ struct test_mb_aead_data { }; static int do_mult_aead_op(struct test_mb_aead_data *data, int enc, - u32 num_mb) + u32 num_mb, int *rc) { - int i, rc[num_mb], err = 0; + int i, err = 0; /* Fire up a bunch of concurrent requests */ for (i = 0; i < num_mb; i++) { @@ -188,18 +188,26 @@ static int test_mb_aead_jiffies(struct test_mb_aead_data *data, int enc, { unsigned long start, end; int bcount; - int ret; + int ret = 0; + int *rc; + + rc = kcalloc(num_mb, sizeof(*rc), GFP_KERNEL); + if (!rc) + return -ENOMEM; for (start = jiffies, end = start + secs * HZ, bcount = 0; time_before(jiffies, end); bcount++) { - ret = do_mult_aead_op(data, enc, num_mb); + ret = do_mult_aead_op(data, enc, num_mb, rc); if (ret) - return ret; + goto out; } pr_cont("%d operations in %d seconds (%ld bytes)\n", bcount * num_mb, secs, (long)bcount * blen * num_mb); - return 0; + +out: + kfree(rc); + return ret; } static int test_mb_aead_cycles(struct test_mb_aead_data *data, int enc, @@ -208,10 +216,15 @@ static int test_mb_aead_cycles(struct test_mb_aead_data *data, int enc, unsigned long cycles = 0; int ret = 0; int i; + int *rc; + + rc = kcalloc(num_mb, sizeof(*rc), GFP_KERNEL); + if (!rc) + return -ENOMEM; /* Warm-up run. */ for (i = 0; i < 4; i++) { - ret = do_mult_aead_op(data, enc, num_mb); + ret = do_mult_aead_op(data, enc, num_mb, rc); if (ret) goto out; } @@ -221,7 +234,7 @@ static int test_mb_aead_cycles(struct test_mb_aead_data *data, int enc, cycles_t start, end; start = get_cycles(); - ret = do_mult_aead_op(data, enc, num_mb); + ret = do_mult_aead_op(data, enc, num_mb, rc); end = get_cycles(); if (ret) @@ -230,11 +243,11 @@ static int test_mb_aead_cycles(struct test_mb_aead_data *data, int enc, cycles += end - start; } -out: - if (ret == 0) - pr_cont("1 operation in %lu cycles (%d bytes)\n", - (cycles + 4) / (8 * num_mb), blen); + pr_cont("1 operation in %lu cycles (%d bytes)\n", + (cycles + 4) / (8 * num_mb), blen); +out: + kfree(rc); return ret; } @@ -705,9 +718,10 @@ struct test_mb_ahash_data { char *xbuf[XBUFSIZE]; }; -static inline int do_mult_ahash_op(struct test_mb_ahash_data *data, u32 num_mb) +static inline int do_mult_ahash_op(struct test_mb_ahash_data *data, u32 num_mb, + int *rc) { - int i, rc[num_mb], err = 0; + int i, err = 0; /* Fire up a bunch of concurrent requests */ for (i = 0; i < num_mb; i++) @@ -731,18 +745,26 @@ static int test_mb_ahash_jiffies(struct test_mb_ahash_data *data, int blen, { unsigned long start, end; int bcount; - int ret; + int ret = 0; + int *rc; + + rc = kcalloc(num_mb, sizeof(*rc), GFP_KERNEL); + if (!rc) + return -ENOMEM; for (start = jiffies, end = start + secs * HZ, bcount = 0; time_before(jiffies, end); bcount++) { - ret = do_mult_ahash_op(data, num_mb); + ret = do_mult_ahash_op(data, num_mb, rc); if (ret) - return ret; + goto out; } pr_cont("%d operations in %d seconds (%ld bytes)\n", bcount * num_mb, secs, (long)bcount * blen * num_mb); - return 0; + +out: + kfree(rc); + return ret; } static int test_mb_ahash_cycles(struct test_mb_ahash_data *data, int blen, @@ -751,10 +773,15 @@ static int test_mb_ahash_cycles(struct test_mb_ahash_data *data, int blen, unsigned long cycles = 0; int ret = 0; int i; + int *rc; + + rc = kcalloc(num_mb, sizeof(*rc), GFP_KERNEL); + if (!rc) + return -ENOMEM; /* Warm-up run. */ for (i = 0; i < 4; i++) { - ret = do_mult_ahash_op(data, num_mb); + ret = do_mult_ahash_op(data, num_mb, rc); if (ret) goto out; } @@ -764,7 +791,7 @@ static int test_mb_ahash_cycles(struct test_mb_ahash_data *data, int blen, cycles_t start, end; start = get_cycles(); - ret = do_mult_ahash_op(data, num_mb); + ret = do_mult_ahash_op(data, num_mb, rc); end = get_cycles(); if (ret) @@ -773,11 +800,11 @@ static int test_mb_ahash_cycles(struct test_mb_ahash_data *data, int blen, cycles += end - start; } -out: - if (ret == 0) - pr_cont("1 operation in %lu cycles (%d bytes)\n", - (cycles + 4) / (8 * num_mb), blen); + pr_cont("1 operation in %lu cycles (%d bytes)\n", + (cycles + 4) / (8 * num_mb), blen); +out: + kfree(rc); return ret; } @@ -1118,9 +1145,9 @@ struct test_mb_skcipher_data { }; static int do_mult_acipher_op(struct test_mb_skcipher_data *data, int enc, - u32 num_mb) + u32 num_mb, int *rc) { - int i, rc[num_mb], err = 0; + int i, err = 0; /* Fire up a bunch of concurrent requests */ for (i = 0; i < num_mb; i++) { @@ -1148,18 +1175,26 @@ static int test_mb_acipher_jiffies(struct test_mb_skcipher_data *data, int enc, { unsigned long start, end; int bcount; - int ret; + int ret = 0; + int *rc; + + rc = kcalloc(num_mb, sizeof(*rc), GFP_KERNEL); + if (!rc) + return -ENOMEM; for (start = jiffies, end = start + secs * HZ, bcount = 0; time_before(jiffies, end); bcount++) { - ret = do_mult_acipher_op(data, enc, num_mb); + ret = do_mult_acipher_op(data, enc, num_mb, rc); if (ret) - return ret; + goto out; } pr_cont("%d operations in %d seconds (%ld bytes)\n", bcount * num_mb, secs, (long)bcount * blen * num_mb); - return 0; + +out: + kfree(rc); + return ret; } static int test_mb_acipher_cycles(struct test_mb_skcipher_data *data, int enc, @@ -1168,10 +1203,15 @@ static int test_mb_acipher_cycles(struct test_mb_skcipher_data *data, int enc, unsigned long cycles = 0; int ret = 0; int i; + int *rc; + + rc = kcalloc(num_mb, sizeof(*rc), GFP_KERNEL); + if (!rc) + return -ENOMEM; /* Warm-up run. */ for (i = 0; i < 4; i++) { - ret = do_mult_acipher_op(data, enc, num_mb); + ret = do_mult_acipher_op(data, enc, num_mb, rc); if (ret) goto out; } @@ -1181,7 +1221,7 @@ static int test_mb_acipher_cycles(struct test_mb_skcipher_data *data, int enc, cycles_t start, end; start = get_cycles(); - ret = do_mult_acipher_op(data, enc, num_mb); + ret = do_mult_acipher_op(data, enc, num_mb, rc); end = get_cycles(); if (ret) @@ -1190,11 +1230,11 @@ static int test_mb_acipher_cycles(struct test_mb_skcipher_data *data, int enc, cycles += end - start; } -out: - if (ret == 0) - pr_cont("1 operation in %lu cycles (%d bytes)\n", - (cycles + 4) / (8 * num_mb), blen); + pr_cont("1 operation in %lu cycles (%d bytes)\n", + (cycles + 4) / (8 * num_mb), blen); +out: + kfree(rc); return ret; } @@ -1606,7 +1646,7 @@ static inline int tcrypt_test(const char *alg) return ret; } -static int do_test(const char *alg, u32 type, u32 mask, int m) +static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb) { int i; int ret = 0; @@ -1621,7 +1661,7 @@ static int do_test(const char *alg, u32 type, u32 mask, int m) } for (i = 1; i < 200; i++) - ret += do_test(NULL, 0, 0, i); + ret += do_test(NULL, 0, 0, i, num_mb); break; case 1: @@ -2903,7 +2943,7 @@ static int __init tcrypt_mod_init(void) goto err_free_tv; } - err = do_test(alg, type, mask, mode); + err = do_test(alg, type, mask, mode, num_mb); if (err) { printk(KERN_ERR "tcrypt: one or more tests failed!\n");