* [PATCHv2 0/4] VLA removal from the gpio subsystem @ 2018-03-15 18:00 Laura Abbott 2018-03-15 18:00 ` [PATCHv2 1/4] gpio: Remove VLA from gpiolib Laura Abbott ` (3 more replies) 0 siblings, 4 replies; 11+ messages in thread From: Laura Abbott @ 2018-03-15 18:00 UTC (permalink / raw) To: Linus Walleij, Kees Cook, Lukas Wunner, Nandor Han, Patrice Chotard Cc: Laura Abbott, linux-gpio, linux-kernel, kernel-hardening, Mathias Duckeck Hi, This is v2 of the series to remove VLAs from the kernel. This is mostly to collect some of the feedback I've gotten so far, more detailed descriptions are in the individual patches. Thanks, Laura Laura Abbott (4): gpio: Remove VLA from gpiolib gpio: Remove VLA from MAX3191X driver gpio: Remove VLA from xra1403 driver gpio: Remove VLA from stmpe driver drivers/gpio/gpio-max3191x.c | 7 +++- drivers/gpio/gpio-stmpe.c | 7 +++- drivers/gpio/gpio-xra1403.c | 8 ++++- drivers/gpio/gpiolib.c | 78 ++++++++++++++++++++++++++++++++++--------- drivers/gpio/gpiolib.h | 2 +- include/linux/gpio/consumer.h | 8 +++-- 6 files changed, 87 insertions(+), 23 deletions(-) -- 2.14.3 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv2 1/4] gpio: Remove VLA from gpiolib 2018-03-15 18:00 [PATCHv2 0/4] VLA removal from the gpio subsystem Laura Abbott @ 2018-03-15 18:00 ` Laura Abbott 2018-03-17 15:36 ` kbuild test robot 2018-03-15 18:00 ` [PATCHv2 2/4] gpio: Remove VLA from MAX3191X driver Laura Abbott ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Laura Abbott @ 2018-03-15 18:00 UTC (permalink / raw) To: Linus Walleij, Kees Cook Cc: Laura Abbott, linux-gpio, linux-kernel, kernel-hardening, Lukas Wunner, Mathias Duckeck, Nandor Han, Patrice Chotard The new challenge is to remove VLAs from the kernel (see https://lkml.org/lkml/2018/3/7/621) to eventually turn on -Wvla. This patch replaces several VLAs with an appropriate call to kmalloc_array. Signed-off-by: Laura Abbott <labbott@redhat.com> --- v2: Switched to kcalloc. Adjusted return types to propegate error code. --- drivers/gpio/gpiolib.c | 76 ++++++++++++++++++++++++++++++++++--------- drivers/gpio/gpiolib.h | 2 +- include/linux/gpio/consumer.h | 8 +++-- 3 files changed, 66 insertions(+), 20 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index d66de67ef307..c81e5d0aea42 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -390,6 +390,7 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd, return 0; } else if (cmd == GPIOHANDLE_SET_LINE_VALUES_IOCTL) { + int ret; /* TODO: check if descriptors are really output */ if (copy_from_user(&ghd, ip, sizeof(ghd))) return -EFAULT; @@ -399,11 +400,14 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd, vals[i] = !!ghd.values[i]; /* Reuse the array setting function */ - gpiod_set_array_value_complex(false, + ret = gpiod_set_array_value_complex(false, true, lh->numdescs, lh->descs, vals); + if (ret) + return ret; + return 0; } return -EINVAL; @@ -2662,16 +2666,32 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, while (i < array_size) { struct gpio_chip *chip = desc_array[i]->gdev->chip; - unsigned long mask[BITS_TO_LONGS(chip->ngpio)]; - unsigned long bits[BITS_TO_LONGS(chip->ngpio)]; + unsigned long *mask; + unsigned long *bits; int first, j, ret; + mask = kcalloc(BITS_TO_LONGS(chip->ngpio), + sizeof(*mask), + can_sleep ? GFP_KERNEL : GFP_ATOMIC); + + if (!mask) + return -ENOMEM; + + bits = kcalloc(BITS_TO_LONGS(chip->ngpio), + sizeof(*bits), + can_sleep ? GFP_KERNEL : GFP_ATOMIC); + + if (!bits) { + kfree(mask); + return -ENOMEM; + } + + if (!can_sleep) WARN_ON(chip->can_sleep); /* collect all inputs belonging to the same chip */ first = i; - memset(mask, 0, sizeof(mask)); do { const struct gpio_desc *desc = desc_array[i]; int hwgpio = gpio_chip_hwgpio(desc); @@ -2682,8 +2702,11 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, (desc_array[i]->gdev->chip == chip)); ret = gpio_chip_get_multiple(chip, mask, bits); - if (ret) + if (ret) { + kfree(bits); + kfree(mask); return ret; + } for (j = first; j < i; j++) { const struct gpio_desc *desc = desc_array[j]; @@ -2695,6 +2718,8 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, value_array[j] = value; trace_gpio_value(desc_to_gpio(desc), 1, value); } + kfree(bits); + kfree(mask); } return 0; } @@ -2878,7 +2903,7 @@ static void gpio_chip_set_multiple(struct gpio_chip *chip, } } -void gpiod_set_array_value_complex(bool raw, bool can_sleep, +int gpiod_set_array_value_complex(bool raw, bool can_sleep, unsigned int array_size, struct gpio_desc **desc_array, int *value_array) @@ -2887,14 +2912,29 @@ void gpiod_set_array_value_complex(bool raw, bool can_sleep, while (i < array_size) { struct gpio_chip *chip = desc_array[i]->gdev->chip; - unsigned long mask[BITS_TO_LONGS(chip->ngpio)]; - unsigned long bits[BITS_TO_LONGS(chip->ngpio)]; + unsigned long *mask; + unsigned long *bits; int count = 0; + mask = kcalloc(BITS_TO_LONGS(chip->ngpio), + sizeof(*mask), + can_sleep ? GFP_KERNEL : GFP_ATOMIC); + + if (!mask) + return -ENOMEM; + + bits = kcalloc(BITS_TO_LONGS(chip->ngpio), + sizeof(*bits), + can_sleep ? GFP_KERNEL : GFP_ATOMIC); + + if (!bits) { + kfree(mask); + return -ENOMEM; + } + if (!can_sleep) WARN_ON(chip->can_sleep); - memset(mask, 0, sizeof(mask)); do { struct gpio_desc *desc = desc_array[i]; int hwgpio = gpio_chip_hwgpio(desc); @@ -2925,7 +2965,11 @@ void gpiod_set_array_value_complex(bool raw, bool can_sleep, /* push collected bits to outputs */ if (count != 0) gpio_chip_set_multiple(chip, mask, bits); + + kfree(mask); + kfree(bits); } + return 0; } /** @@ -3000,13 +3044,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_value); * This function should be called from contexts where we cannot sleep, and will * complain if the GPIO chip functions potentially sleep. */ -void gpiod_set_raw_array_value(unsigned int array_size, +int gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, int *value_array) { if (!desc_array) - return; - gpiod_set_array_value_complex(true, false, array_size, desc_array, - value_array); + return -EINVAL; + return gpiod_set_array_value_complex(true, false, array_size, + desc_array, value_array); } EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value); @@ -3326,14 +3370,14 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep); * * This function is to be called from contexts that can sleep. */ -void gpiod_set_raw_array_value_cansleep(unsigned int array_size, +int gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, int *value_array) { might_sleep_if(extra_checks); if (!desc_array) - return; - gpiod_set_array_value_complex(true, true, array_size, desc_array, + return -EINVAL; + return gpiod_set_array_value_complex(true, true, array_size, desc_array, value_array); } EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value_cansleep); diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index b17ec6795c81..b64813e3876e 100644 --- a/drivers/gpio/gpiolib.h +++ b/drivers/gpio/gpiolib.h @@ -188,7 +188,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, unsigned int array_size, struct gpio_desc **desc_array, int *value_array); -void gpiod_set_array_value_complex(bool raw, bool can_sleep, +int gpiod_set_array_value_complex(bool raw, bool can_sleep, unsigned int array_size, struct gpio_desc **desc_array, int *value_array); diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index dbd065963296..da52610cc3bd 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -116,7 +116,7 @@ int gpiod_get_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, int *value_array); void gpiod_set_raw_value(struct gpio_desc *desc, int value); -void gpiod_set_raw_array_value(unsigned int array_size, +int gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, int *value_array); @@ -134,7 +134,7 @@ int gpiod_get_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, int *value_array); void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value); -void gpiod_set_raw_array_value_cansleep(unsigned int array_size, +int gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, int *value_array); @@ -369,12 +369,13 @@ static inline void gpiod_set_raw_value(struct gpio_desc *desc, int value) /* GPIO can never have been requested */ WARN_ON(1); } -static inline void gpiod_set_raw_array_value(unsigned int array_size, +static inline int gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, int *value_array) { /* GPIO can never have been requested */ WARN_ON(1); + return 0; } static inline int gpiod_get_value_cansleep(const struct gpio_desc *desc) @@ -429,6 +430,7 @@ static inline void gpiod_set_raw_array_value_cansleep(unsigned int array_size, { /* GPIO can never have been requested */ WARN_ON(1); + return 0; } static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce) -- 2.14.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCHv2 1/4] gpio: Remove VLA from gpiolib 2018-03-15 18:00 ` [PATCHv2 1/4] gpio: Remove VLA from gpiolib Laura Abbott @ 2018-03-17 15:36 ` kbuild test robot 0 siblings, 0 replies; 11+ messages in thread From: kbuild test robot @ 2018-03-17 15:36 UTC (permalink / raw) To: Laura Abbott Cc: kbuild-all, Linus Walleij, Kees Cook, Laura Abbott, linux-gpio, linux-kernel, kernel-hardening, Lukas Wunner, Mathias Duckeck, Nandor Han, Patrice Chotard [-- Attachment #1: Type: text/plain, Size: 3066 bytes --] Hi Laura, I love your patch! Perhaps something to improve: [auto build test WARNING on v4.16-rc4] [also build test WARNING on next-20180316] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Laura-Abbott/VLA-removal-from-the-gpio-subsystem/20180317-210828 config: i386-defconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): In file included from drivers/clk/clk-gpio.c:18:0: include/linux/gpio/consumer.h: In function 'gpiod_set_raw_array_value_cansleep': >> include/linux/gpio/consumer.h:433:9: warning: 'return' with a value, in function returning void return 0; ^ include/linux/gpio/consumer.h:427:20: note: declared here static inline void gpiod_set_raw_array_value_cansleep(unsigned int array_size, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/return +433 include/linux/gpio/consumer.h 380 381 static inline int gpiod_get_value_cansleep(const struct gpio_desc *desc) 382 { 383 /* GPIO can never have been requested */ 384 WARN_ON(1); 385 return 0; 386 } 387 static inline int gpiod_get_array_value_cansleep(unsigned int array_size, 388 struct gpio_desc **desc_array, 389 int *value_array) 390 { 391 /* GPIO can never have been requested */ 392 WARN_ON(1); 393 return 0; 394 } 395 static inline void gpiod_set_value_cansleep(struct gpio_desc *desc, int value) 396 { 397 /* GPIO can never have been requested */ 398 WARN_ON(1); 399 } 400 static inline void gpiod_set_array_value_cansleep(unsigned int array_size, 401 struct gpio_desc **desc_array, 402 int *value_array) 403 { 404 /* GPIO can never have been requested */ 405 WARN_ON(1); 406 } 407 static inline int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc) 408 { 409 /* GPIO can never have been requested */ 410 WARN_ON(1); 411 return 0; 412 } 413 static inline int gpiod_get_raw_array_value_cansleep(unsigned int array_size, 414 struct gpio_desc **desc_array, 415 int *value_array) 416 { 417 /* GPIO can never have been requested */ 418 WARN_ON(1); 419 return 0; 420 } 421 static inline void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, 422 int value) 423 { 424 /* GPIO can never have been requested */ 425 WARN_ON(1); 426 } 427 static inline void gpiod_set_raw_array_value_cansleep(unsigned int array_size, 428 struct gpio_desc **desc_array, 429 int *value_array) 430 { 431 /* GPIO can never have been requested */ 432 WARN_ON(1); > 433 return 0; 434 } 435 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 26860 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv2 2/4] gpio: Remove VLA from MAX3191X driver 2018-03-15 18:00 [PATCHv2 0/4] VLA removal from the gpio subsystem Laura Abbott 2018-03-15 18:00 ` [PATCHv2 1/4] gpio: Remove VLA from gpiolib Laura Abbott @ 2018-03-15 18:00 ` Laura Abbott 2018-03-19 14:38 ` Lukas Wunner 2018-03-15 18:00 ` [PATCHv2 3/4] gpio: Remove VLA from xra1403 driver Laura Abbott 2018-03-15 18:00 ` [PATCHv2 4/4] gpio: Remove VLA from stmpe driver Laura Abbott 3 siblings, 1 reply; 11+ messages in thread From: Laura Abbott @ 2018-03-15 18:00 UTC (permalink / raw) To: Linus Walleij, Kees Cook, Lukas Wunner Cc: Laura Abbott, linux-gpio, linux-kernel, kernel-hardening, Mathias Duckeck The new challenge is to remove VLAs from the kernel (see https://lkml.org/lkml/2018/3/7/621) This patch replaces several a VLA with an appropriate call to kmalloc_array. Signed-off-by: Laura Abbott <labbott@redhat.com> --- v2: No changes --- drivers/gpio/gpio-max3191x.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-max3191x.c b/drivers/gpio/gpio-max3191x.c index f74b1072e84b..b5b9cb1fda50 100644 --- a/drivers/gpio/gpio-max3191x.c +++ b/drivers/gpio/gpio-max3191x.c @@ -315,12 +315,17 @@ static void gpiod_set_array_single_value_cansleep(unsigned int ndescs, struct gpio_desc **desc, int value) { - int i, values[ndescs]; + int i, *values; + + values = kmalloc_array(ndescs, sizeof(*values), GFP_KERNEL); + if (!values) + return; for (i = 0; i < ndescs; i++) values[i] = value; gpiod_set_array_value_cansleep(ndescs, desc, values); + kfree(values); } static struct gpio_descs *devm_gpiod_get_array_optional_count( -- 2.14.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCHv2 2/4] gpio: Remove VLA from MAX3191X driver 2018-03-15 18:00 ` [PATCHv2 2/4] gpio: Remove VLA from MAX3191X driver Laura Abbott @ 2018-03-19 14:38 ` Lukas Wunner 0 siblings, 0 replies; 11+ messages in thread From: Lukas Wunner @ 2018-03-19 14:38 UTC (permalink / raw) To: Laura Abbott Cc: Linus Walleij, Kees Cook, linux-gpio, linux-kernel, kernel-hardening, Mathias Duckeck On Thu, Mar 15, 2018 at 11:00:28AM -0700, Laura Abbott wrote: > > The new challenge is to remove VLAs from the kernel > (see https://lkml.org/lkml/2018/3/7/621) > > This patch replaces several a VLA with an appropriate call to > kmalloc_array. > > Signed-off-by: Laura Abbott <labbott@redhat.com> Reviewed-and-tested-by: Lukas Wunner <lukas@wunner.de> This one isn't a hotpath, so the kmalloc overhead is negligible. Did a quick test on a single-chip MAX31913 with no apparent issues. > --- > v2: No changes > --- > drivers/gpio/gpio-max3191x.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpio/gpio-max3191x.c b/drivers/gpio/gpio-max3191x.c > index f74b1072e84b..b5b9cb1fda50 100644 > --- a/drivers/gpio/gpio-max3191x.c > +++ b/drivers/gpio/gpio-max3191x.c > @@ -315,12 +315,17 @@ static void gpiod_set_array_single_value_cansleep(unsigned int ndescs, > struct gpio_desc **desc, > int value) > { > - int i, values[ndescs]; > + int i, *values; > + > + values = kmalloc_array(ndescs, sizeof(*values), GFP_KERNEL); > + if (!values) > + return; > > for (i = 0; i < ndescs; i++) > values[i] = value; > > gpiod_set_array_value_cansleep(ndescs, desc, values); > + kfree(values); > } > > static struct gpio_descs *devm_gpiod_get_array_optional_count( > -- > 2.14.3 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv2 3/4] gpio: Remove VLA from xra1403 driver 2018-03-15 18:00 [PATCHv2 0/4] VLA removal from the gpio subsystem Laura Abbott 2018-03-15 18:00 ` [PATCHv2 1/4] gpio: Remove VLA from gpiolib Laura Abbott 2018-03-15 18:00 ` [PATCHv2 2/4] gpio: Remove VLA from MAX3191X driver Laura Abbott @ 2018-03-15 18:00 ` Laura Abbott 2018-03-15 18:00 ` [PATCHv2 4/4] gpio: Remove VLA from stmpe driver Laura Abbott 3 siblings, 0 replies; 11+ messages in thread From: Laura Abbott @ 2018-03-15 18:00 UTC (permalink / raw) To: Linus Walleij, Kees Cook, Nandor Han Cc: Laura Abbott, linux-gpio, linux-kernel, kernel-hardening The new challenge is to remove VLAs from the kernel (see https://lkml.org/lkml/2018/3/7/621) This patch replaces a VLA with an appropriate call to kmalloc_array. Reviewed-by: Nandor Han <nandor.han@ge.com> Signed-off-by: Laura Abbott <labbott@redhat.com> --- v3: Add reviwed-by --- drivers/gpio/gpio-xra1403.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-xra1403.c b/drivers/gpio/gpio-xra1403.c index 0230e4b7a2fb..8d4c8e99b251 100644 --- a/drivers/gpio/gpio-xra1403.c +++ b/drivers/gpio/gpio-xra1403.c @@ -126,11 +126,16 @@ static void xra1403_dbg_show(struct seq_file *s, struct gpio_chip *chip) { int reg; struct xra1403 *xra = gpiochip_get_data(chip); - int value[xra1403_regmap_cfg.max_register]; + int *value; int i; unsigned int gcr; unsigned int gsr; + value = kmalloc_array(xra1403_regmap_cfg.max_register, sizeof(*value), + GFP_KERNEL); + if (!value) + return; + seq_puts(s, "xra reg:"); for (reg = 0; reg <= xra1403_regmap_cfg.max_register; reg++) seq_printf(s, " %2.2x", reg); @@ -154,6 +159,7 @@ static void xra1403_dbg_show(struct seq_file *s, struct gpio_chip *chip) (gcr & BIT(i)) ? "in" : "out", (gsr & BIT(i)) ? "hi" : "lo"); } + kfree(value); } #else #define xra1403_dbg_show NULL -- 2.14.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCHv2 4/4] gpio: Remove VLA from stmpe driver 2018-03-15 18:00 [PATCHv2 0/4] VLA removal from the gpio subsystem Laura Abbott ` (2 preceding siblings ...) 2018-03-15 18:00 ` [PATCHv2 3/4] gpio: Remove VLA from xra1403 driver Laura Abbott @ 2018-03-15 18:00 ` Laura Abbott 2018-03-19 1:29 ` Phil Reid 3 siblings, 1 reply; 11+ messages in thread From: Laura Abbott @ 2018-03-15 18:00 UTC (permalink / raw) To: Linus Walleij, Kees Cook, Patrice Chotard Cc: Laura Abbott, linux-gpio, linux-kernel, kernel-hardening The new challenge is to remove VLAs from the kernel (see https://lkml.org/lkml/2018/3/7/621) This patch replaces a VLA with an appropriate call to kmalloc_array. Signed-off-by: Laura Abbott <labbott@redhat.com> --- v2: Switch to GFP_KERNEL. There was some discussion about if we should be doing the allocation at all but given a) the allocation is pretty small and b) we can possibly take a mutex in a called function I think this is fine. --- drivers/gpio/gpio-stmpe.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c index f8d7d1cd8488..c2bb20ace6f5 100644 --- a/drivers/gpio/gpio-stmpe.c +++ b/drivers/gpio/gpio-stmpe.c @@ -369,10 +369,14 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev) struct stmpe *stmpe = stmpe_gpio->stmpe; u8 statmsbreg; int num_banks = DIV_ROUND_UP(stmpe->num_gpios, 8); - u8 status[num_banks]; + u8 *status; int ret; int i; + status = kmalloc_array(num_banks, sizeof(*status), GFP_KERNEL); + if (!status) + return IRQ_NONE; + /* * the stmpe_block_read() call below, imposes to set statmsbreg * with the register located at the lowest address. As STMPE1600 @@ -424,6 +428,7 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev) } } + kfree(status); return IRQ_HANDLED; } -- 2.14.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCHv2 4/4] gpio: Remove VLA from stmpe driver 2018-03-15 18:00 ` [PATCHv2 4/4] gpio: Remove VLA from stmpe driver Laura Abbott @ 2018-03-19 1:29 ` Phil Reid 2018-03-22 21:43 ` Laura Abbott 0 siblings, 1 reply; 11+ messages in thread From: Phil Reid @ 2018-03-19 1:29 UTC (permalink / raw) To: Laura Abbott, Linus Walleij, Kees Cook, Patrice Chotard Cc: linux-gpio, linux-kernel, kernel-hardening On 16/03/2018 02:00, Laura Abbott wrote: > The new challenge is to remove VLAs from the kernel > (see https://lkml.org/lkml/2018/3/7/621) > > This patch replaces a VLA with an appropriate call to kmalloc_array. > > Signed-off-by: Laura Abbott <labbott@redhat.com> > --- > v2: Switch to GFP_KERNEL. There was some discussion about if we should > be doing the allocation at all but given a) the allocation is pretty > small and b) we can possibly take a mutex in a called function I think > this is fine. I still think it's a bad idea. It's simple to preallocate the buffer. But it's up to the maintainer. > --- > drivers/gpio/gpio-stmpe.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c > index f8d7d1cd8488..c2bb20ace6f5 100644 > --- a/drivers/gpio/gpio-stmpe.c > +++ b/drivers/gpio/gpio-stmpe.c > @@ -369,10 +369,14 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev) > struct stmpe *stmpe = stmpe_gpio->stmpe; > u8 statmsbreg; > int num_banks = DIV_ROUND_UP(stmpe->num_gpios, 8); > - u8 status[num_banks]; > + u8 *status; > int ret; > int i; > > + status = kmalloc_array(num_banks, sizeof(*status), GFP_KERNEL); > + if (!status) > + return IRQ_NONE; > + > /* > * the stmpe_block_read() call below, imposes to set statmsbreg > * with the register located at the lowest address. As STMPE1600 > @@ -424,6 +428,7 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev) > } > } > > + kfree(status); > return IRQ_HANDLED; > } > > -- Regards Phil Reid ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv2 4/4] gpio: Remove VLA from stmpe driver 2018-03-19 1:29 ` Phil Reid @ 2018-03-22 21:43 ` Laura Abbott 2018-03-23 1:28 ` Phil Reid 2018-03-27 13:41 ` Linus Walleij 0 siblings, 2 replies; 11+ messages in thread From: Laura Abbott @ 2018-03-22 21:43 UTC (permalink / raw) To: Phil Reid, Linus Walleij, Kees Cook, Patrice Chotard Cc: linux-gpio, linux-kernel, kernel-hardening On 03/18/2018 06:29 PM, Phil Reid wrote: > On 16/03/2018 02:00, Laura Abbott wrote: >> The new challenge is to remove VLAs from the kernel >> (see https://lkml.org/lkml/2018/3/7/621) >> >> This patch replaces a VLA with an appropriate call to kmalloc_array. >> >> Signed-off-by: Laura Abbott <labbott@redhat.com> >> --- >> v2: Switch to GFP_KERNEL. There was some discussion about if we should >> be doing the allocation at all but given a) the allocation is pretty >> small and b) we can possibly take a mutex in a called function I think >> this is fine. > > I still think it's a bad idea. It's simple to preallocate the buffer. > But it's up to the maintainer. > I'd feel a lot more confident about doing the global buffer with guidance from the maintainer. But looking at the platform data, the maximum number of GPIOs is 24, or 3 banks. Maybe we should just always stack allocate the maximum since it's fairly small. Thanks, Laura > >> --- >> drivers/gpio/gpio-stmpe.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c >> index f8d7d1cd8488..c2bb20ace6f5 100644 >> --- a/drivers/gpio/gpio-stmpe.c >> +++ b/drivers/gpio/gpio-stmpe.c >> @@ -369,10 +369,14 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev) >> struct stmpe *stmpe = stmpe_gpio->stmpe; >> u8 statmsbreg; >> int num_banks = DIV_ROUND_UP(stmpe->num_gpios, 8); >> - u8 status[num_banks]; >> + u8 *status; >> int ret; >> int i; >> + status = kmalloc_array(num_banks, sizeof(*status), GFP_KERNEL); >> + if (!status) >> + return IRQ_NONE; >> + >> /* >> * the stmpe_block_read() call below, imposes to set statmsbreg >> * with the register located at the lowest address. As STMPE1600 >> @@ -424,6 +428,7 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev) >> } >> } >> + kfree(status); >> return IRQ_HANDLED; >> } >> > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv2 4/4] gpio: Remove VLA from stmpe driver 2018-03-22 21:43 ` Laura Abbott @ 2018-03-23 1:28 ` Phil Reid 2018-03-27 13:41 ` Linus Walleij 1 sibling, 0 replies; 11+ messages in thread From: Phil Reid @ 2018-03-23 1:28 UTC (permalink / raw) To: Laura Abbott, Linus Walleij, Kees Cook, Patrice Chotard Cc: linux-gpio, linux-kernel, kernel-hardening On 23/03/2018 05:43, Laura Abbott wrote: > On 03/18/2018 06:29 PM, Phil Reid wrote: >> On 16/03/2018 02:00, Laura Abbott wrote: >>> The new challenge is to remove VLAs from the kernel >>> (see https://lkml.org/lkml/2018/3/7/621) >>> >>> This patch replaces a VLA with an appropriate call to kmalloc_array. >>> >>> Signed-off-by: Laura Abbott <labbott@redhat.com> >>> --- >>> v2: Switch to GFP_KERNEL. There was some discussion about if we should >>> be doing the allocation at all but given a) the allocation is pretty >>> small and b) we can possibly take a mutex in a called function I think >>> this is fine. >> >> I still think it's a bad idea. It's simple to preallocate the buffer. >> But it's up to the maintainer. >> > > I'd feel a lot more confident about doing the global buffer with > guidance from the maintainer. But looking at the platform data, the > maximum number of GPIOs is 24, or 3 banks. Maybe we should just always > stack allocate the maximum since it's fairly small. That's the other way to go. >>> --- >>> drivers/gpio/gpio-stmpe.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c >>> index f8d7d1cd8488..c2bb20ace6f5 100644 >>> --- a/drivers/gpio/gpio-stmpe.c >>> +++ b/drivers/gpio/gpio-stmpe.c >>> @@ -369,10 +369,14 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev) >>> struct stmpe *stmpe = stmpe_gpio->stmpe; >>> u8 statmsbreg; >>> int num_banks = DIV_ROUND_UP(stmpe->num_gpios, 8); >>> - u8 status[num_banks]; >>> + u8 *status; >>> int ret; >>> int i; >>> + status = kmalloc_array(num_banks, sizeof(*status), GFP_KERNEL); >>> + if (!status) >>> + return IRQ_NONE; >>> + >>> /* >>> * the stmpe_block_read() call below, imposes to set statmsbreg >>> * with the register located at the lowest address. As STMPE1600 >>> @@ -424,6 +428,7 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev) >>> } >>> } >>> + kfree(status); >>> return IRQ_HANDLED; >>> } >>> >> >> > > > -- Regards Phil Reid ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv2 4/4] gpio: Remove VLA from stmpe driver 2018-03-22 21:43 ` Laura Abbott 2018-03-23 1:28 ` Phil Reid @ 2018-03-27 13:41 ` Linus Walleij 1 sibling, 0 replies; 11+ messages in thread From: Linus Walleij @ 2018-03-27 13:41 UTC (permalink / raw) To: Laura Abbott Cc: Phil Reid, Kees Cook, Patrice Chotard, open list:GPIO SUBSYSTEM, linux-kernel, kernel-hardening Hi Laura, sorry for slow response :/ On Thu, Mar 22, 2018 at 10:43 PM, Laura Abbott <labbott@redhat.com> wrote: > On 03/18/2018 06:29 PM, Phil Reid wrote: >> >> On 16/03/2018 02:00, Laura Abbott wrote: >>> >>> The new challenge is to remove VLAs from the kernel >>> (see https://lkml.org/lkml/2018/3/7/621) >>> >>> This patch replaces a VLA with an appropriate call to kmalloc_array. >>> >>> Signed-off-by: Laura Abbott <labbott@redhat.com> >>> --- >>> v2: Switch to GFP_KERNEL. There was some discussion about if we should >>> be doing the allocation at all but given a) the allocation is pretty >>> small and b) we can possibly take a mutex in a called function I think >>> this is fine. >> >> >> I still think it's a bad idea. It's simple to preallocate the buffer. >> But it's up to the maintainer. >> > > I'd feel a lot more confident about doing the global buffer with > guidance from the maintainer. But looking at the platform data, the > maximum number of GPIOs is 24, or 3 banks. Maybe we should just always > stack allocate the maximum since it's fairly small. Either way works fine, global (in the state container struct stmpe_gpio) or stack allocation for 24 bits. I guess I am maintainer for this, I can test it at least. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-03-27 13:41 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-15 18:00 [PATCHv2 0/4] VLA removal from the gpio subsystem Laura Abbott 2018-03-15 18:00 ` [PATCHv2 1/4] gpio: Remove VLA from gpiolib Laura Abbott 2018-03-17 15:36 ` kbuild test robot 2018-03-15 18:00 ` [PATCHv2 2/4] gpio: Remove VLA from MAX3191X driver Laura Abbott 2018-03-19 14:38 ` Lukas Wunner 2018-03-15 18:00 ` [PATCHv2 3/4] gpio: Remove VLA from xra1403 driver Laura Abbott 2018-03-15 18:00 ` [PATCHv2 4/4] gpio: Remove VLA from stmpe driver Laura Abbott 2018-03-19 1:29 ` Phil Reid 2018-03-22 21:43 ` Laura Abbott 2018-03-23 1:28 ` Phil Reid 2018-03-27 13:41 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).