From patchwork Mon Mar 26 06:33:14 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tobin Harding X-Patchwork-Id: 10307221 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 3CDEF60353 for ; Mon, 26 Mar 2018 06:33:43 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2A12028E3B for ; Mon, 26 Mar 2018 06:33:43 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1EEAB29502; Mon, 26 Mar 2018 06:33:43 +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=-4.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id 1CD0B28E3B for ; Mon, 26 Mar 2018 06:33:41 +0000 (UTC) Received: (qmail 21971 invoked by uid 550); 26 Mar 2018 06:33:39 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 21926 invoked from network); 26 Mar 2018 06:33:38 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tobin.cc; h=cc :date:from:message-id:subject:to:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=SkA5nm/L0ZJHx3nVkwQEdRW0Dt+mqyocDBj8FQeX/ gw=; b=daFLy5mvG3SzsWjvGBrhNWDOnZ47RatheF86271hw3Yex6D6rR9kIQ84d MBZjO6+2hmQUKyev7ebXUuT3S+mY5Wx3Am4jMsM/BnKNu7AaUK4CPXdcYOrdLT9S Wl8Z3Ngb+52KBWV9SFe5T7pr1UGK5ll444g6J7inhpdCiA81ywiOp2FvwVjH9Zpi FCe7jx8zzsoLqBGgrl/gSLMOp8CEKjY7OZ54u8yZmkaiHPhsUKT8tBhrIhb1/lv4 jf1pZ6bc8w3DdcuiUm3hQye1iKC4vjL6yn1aXR2rqZuOKtnd5Q3m/MzKZ+7jIbJe Go9U+ZNos6ED5jJ5MxrFBqTH9LRtw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:date:from:message-id:subject:to :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=SkA5nm/L0ZJHx3nVk wQEdRW0Dt+mqyocDBj8FQeX/gw=; b=ZnJvIyIelvOM4CiPdYk2x0eJ5Fbvade0H G7iCtRBBU2746o1qzppO2fWtKTH0TtYcJUIC2C+sdqo6WAD2v37bgSNQud+oXtH2 LMKcWA0qua3sl/aqMxL+5hA1Q+SV+YG4n6QvjpznMlJ52fZO4RSJA09+3RFp8M74 kwccDGYIPyzHC+PpNbpRt6/KOFK7FZ++q5eq7Gi+BSp53b5BvgTKJbFr5FdJTTu+ RxNM2PeQJJ/qrcC6mgOwcLxwopqEmG2aPPnBtWVdxltwmwxux2x3O1CyD1WVCxnd LRngvHv6mJjnH3NUcKSq3Zirmekbi3eM3ysNTV8U5jqDMBKsZvAwg== X-ME-Sender: From: "Tobin C. Harding" To: Ulf Hansson Cc: "Tobin C. Harding" , linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-hardening@lists.openwall.com, Tycho Andersen Subject: [PATCH] mmc: pwrseq: Use kmalloc_array instead of stack VLA Date: Mon, 26 Mar 2018 17:33:14 +1100 Message-Id: <1522045994-21913-1-git-send-email-me@tobin.cc> X-Mailer: git-send-email 2.7.4 X-Virus-Scanned: ClamAV using ClamSMTP The use of stack Variable Length Arrays needs to be avoided, as they can be a vector for stack exhaustion, which can be both a runtime bug (kernel Oops) or a security flaw (overwriting memory beyond the stack). Also, in general, as code evolves it is easy to lose track of how big a VLA can get. Thus, we can end up having runtime failures that are hard to debug. As part of the directive[1] to remove all VLAs from the kernel, and build with -Wvla. Currently driver is using a VLA declared using the number of descriptors. This array is used to store integer values and is later used as an argument to `gpiod_set_array_value_cansleep()` This can be avoided by using `kmalloc_array()` to allocate memory for the array of integer values. Memory is free'd before return from function. From the code it appears that it is safe to sleep so we can use GFP_KERNEL (based _cansleep() suffix of function `gpiod_set_array_value_cansleep()`. It can be expected that this patch will result in a small increase in overhead due to the use of `kmalloc_array()` [1] https://lkml.org/lkml/2018/3/7/621 Signed-off-by: Tobin C. Harding --- Patch is untested. I was not able to fully grok the call chain involved with this change so was unable to quantify the total degradation of performance. Line calling to gpiod_set_array_value_cansleep() is 83 characters long, patch attempts to leave the code as clean as possible. Open to suggestions for improvement. I tried various forms of while loop and pointer arithmetic for the setting of `values` but in the end settled for array indexing, again open to suggestions. thanks, Tobin. drivers/mmc/core/pwrseq_simple.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c index 13ef162cf066..a8b9fee4d62a 100644 --- a/drivers/mmc/core/pwrseq_simple.c +++ b/drivers/mmc/core/pwrseq_simple.c @@ -40,14 +40,18 @@ static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq, struct gpio_descs *reset_gpios = pwrseq->reset_gpios; if (!IS_ERR(reset_gpios)) { - int i; - int values[reset_gpios->ndescs]; + int i, *values; + int nvalues = reset_gpios->ndescs; - for (i = 0; i < reset_gpios->ndescs; i++) + values = kmalloc_array(nvalues, sizeof(int), GFP_KERNEL); + if (!values) + return; + + for (i = 0; i < nvalues; i++) values[i] = value; - gpiod_set_array_value_cansleep( - reset_gpios->ndescs, reset_gpios->desc, values); + gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc, values); + kfree(values); } }