Skip to content

Commit

Permalink
mmc: pwrseq: Use kmalloc_array instead of stack VLA
Browse files Browse the repository at this point in the history
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 <me@tobin.cc>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
  • Loading branch information
tcharding authored and storulf committed May 2, 2018
1 parent 57aac33 commit 486e666
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions drivers/mmc/core/pwrseq_simple.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 486e666

Please sign in to comment.