linux-hardening - Re: [PATCH] leds: gpio: Set num

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240725102240.GE501857@google.com>
Date: Thu, 25 Jul 2024 11:22:40 +0100
From: Lee Jones <lee@...nel.org>
To: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
Cc: Kees Cook <kees@...nel.org>, 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 Tue, 16 Jul 2024, Gustavo A. R. Silva wrote:

> 
> 
> 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
> -- 

Using the signature tag in the middle of an email turns the remainder of
the body into a signature block, which is odd to say the least.  By all
means sign-off in the middle of a mail, but please refrain from
converting the rest of the mail.

> 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;
> >   }

-- 
Lee Jones [李琼斯]