[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c16715ff-1e47-4a73-8fcb-87462069b5ca@embeddedor.com>
Date: Tue, 16 Jul 2024 15:50:04 -0600
From: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To: Kees Cook <kees@...nel.org>, Lee Jones <lee@...nel.org>
Cc: Pavel Machek <pavel@....cz>, linux-leds@...r.kernel.org,
"Gustavo A. R. Silva" <gustavoars@...nel.org>, linux-kernel@...r.kernel.org,
linux-hardening@...r.kernel.org
Subject: Re: [PATCH] leds: gpio: Set num_leds after allocation
On 16/07/24 15:24, Kees Cook wrote:
> With the new __counted_by annotation, the "num_leds" variable needs to
> valid for accesses to the "leds" array. This requirement is not met in
> gpio_leds_create(), since "num_leds" starts at "0", so "leds" index "0"
> will not be considered valid (num_leds would need to be "1" to access
> index "0").
>
> Fix this by setting the allocation size after allocation, and then update
> the final count based on how many were actually added to the array.
>
> Fixes: 52cd75108a42 ("leds: gpio: Annotate struct gpio_leds_priv with __counted_by")
> Signed-off-by: Kees Cook <kees@...nel.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@...nel.org>
Thanks
--
Gustavo
> ---
> Cc: Lee Jones <lee@...nel.org>
> Cc: Pavel Machek <pavel@....cz>
> Cc: linux-leds@...r.kernel.org
> ---
> drivers/leds/leds-gpio.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index 83fcd7b6afff..4d1612d557c8 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -150,7 +150,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct device *dev)
> {
> struct fwnode_handle *child;
> struct gpio_leds_priv *priv;
> - int count, ret;
> + int count, used, ret;
>
> count = device_get_child_node_count(dev);
> if (!count)
> @@ -159,9 +159,11 @@ static struct gpio_leds_priv *gpio_leds_create(struct device *dev)
> priv = devm_kzalloc(dev, struct_size(priv, leds, count), GFP_KERNEL);
> if (!priv)
> return ERR_PTR(-ENOMEM);
> + priv->num_leds = count;
> + used = 0;
>
> device_for_each_child_node(dev, child) {
> - struct gpio_led_data *led_dat = &priv->leds[priv->num_leds];
> + struct gpio_led_data *led_dat = &priv->leds[used];
> struct gpio_led led = {};
>
> /*
> @@ -197,8 +199,9 @@ static struct gpio_leds_priv *gpio_leds_create(struct device *dev)
> /* Set gpiod label to match the corresponding LED name. */
> gpiod_set_consumer_name(led_dat->gpiod,
> led_dat->cdev.dev->kobj.name);
> - priv->num_leds++;
> + used++;
> }
> + priv->num_leds = used;
>
> return priv;
> }
Powered by blists - more mailing lists