mdio-mux-gpio: Remove VLA usage
Subsystems (3)
PATCH 1/1 mdio-mux-gpio: Remove VLA usage
In the quest to remove all stack VLA usage from the kernel[1], this allocates the values buffer during the callback instead of putting it on the stack.
[1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
--- drivers/net/phy/mdio-mux-gpio.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/net/phy/mdio-mux-gpio.c +6 -1
@@ -26,18 +26,23 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child, void *data) {struct mdio_mux_gpio_state *s = data; -int values[s->gpios->ndescs]; +int *values;unsigned int n;if (current_child == desired_child)return 0 ; + values =kmalloc_array (s->gpios->ndescs,sizeof (*values),GFP_KERNEL ); +if (!values) +return -ENOMEM ; +for (n =0 ; n < s->gpios->ndescs; n++) values[n] = (desired_child >> n) &1 ;gpiod_set_array_value_cansleep (s->gpios->ndescs, s->gpios->desc, values); +kfree (values);return 0 ; }
Signature · 6 lines
2.17.0 -- Kees Cook Pixel Security
Quoted 5 lines · Kees Cook
In the quest to remove all stack VLA usage from the kernel[1], this allocates the values buffer during the callback instead of putting it on the stack.
[1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
[]
drivers/net/phy/mdio-mux-gpio.c
Quoted 1 line
drivers/net/phy/mdio-mux-gpio.c +0
-0
[]
Quoted 4 lines
drivers/net/phy/mdio-mux-gpio.c +0
-0@@ -26,18 +26,23 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child, void *data) {struct mdio_mux_gpio_state *s = data;
drivers/net/phy/mdio-mux-gpio.c static int mdio_mux_gpio_switch_fn(int current_child, int desired_child, · line 37
Quoted 10 lines
drivers/net/phy/mdio-mux-gpio.c +4
-1-int values[s->gpios->ndescs]; +int *values;unsigned int n;if (current_child == desired_child)return 0 ; + values =kmalloc_array (s->gpios->ndescs,sizeof (*values),GFP_KERNEL ); +if (!values) +return -ENOMEM ;
This function previously only returned 0 and now it can return -ENOMEM.
How do you know all the indirect callers via the switch_fn stored in mdio_mux_init can handle the new return value?
If you have explored these code paths, it would be good to explain that in the commit message.
Andrew Lunn
maintainerQuoted 29 lines · Kees Cook
In the quest to remove all stack VLA usage from the kernel[1], this allocates the values buffer during the callback instead of putting it on the stack.
[1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
Signed-off-by Kees Cook <[email protected]>--- drivers/net/phy/mdio-mux-gpio.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/net/phy/mdio-mux-gpio.c +4
-1@@ -26,18 +26,23 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child, void *data) {struct mdio_mux_gpio_state *s = data; -int values[s->gpios->ndescs]; +int *values;unsigned int n;if (current_child == desired_child)return 0 ; + values =kmalloc_array (s->gpios->ndescs,sizeof (*values),GFP_KERNEL ); +if (!values) +return -ENOMEM ;
drivers/net/phy/mdio-mux-gpio.c static int mdio_mux_gpio_switch_fn(int current_child, int desired_child, · line 47
Quoted 10 lines
drivers/net/phy/mdio-mux-gpio.c +2
-0+for (n =0 ; n < s->gpios->ndescs; n++) values[n] = (desired_child >> n) &1 ;gpiod_set_array_value_cansleep (s->gpios->ndescs, s->gpios->desc, values); +kfree (values);return 0 ; }
Hi Kees
This is the 'hot path' for this driver, and you are making it more expensive.
Please allocate the memory once in mdio_mux_gpio_probe and make it part of mdio_mux_gpio_state.
Thanks
Andrew