linux-kernel - Re: [PATCH] leds: lm3697: Fix out-of-bound access

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 5 Oct 2020 18:05:38 +0200
From:   Pavel Machek <pavel@....cz>
To:     ultracoolguy@...anota.com
Cc:     Dan Murphy <dmurphy@...com>, Marek Behun <kabel@...ckhole.sk>,
        Linux Kernel <linux-kernel@...r.kernel.org>,
        Linux Leds <linux-leds@...r.kernel.org>
Subject: Re: [PATCH] leds: lm3697: Fix out-of-bound access

Hi!

> Well, the major benefit I see is that it makes the driver slightly more readable. However I'm fine with whatever you guys decide.
> 
> I'll attach the patch with the struct renaming removed just in case.

Thanks for the patches. Content is pretty good, but I'd really need
From + Signed-off-by: with your real name to be able to apply it. (I'd
avoid renaming leds->banks variable in this patch, too, so we have
minimum stable patch).

Dan is maintaining this code, I suspect he'll come up with minimum
fix + followup cleanups shortly.

Best regards,
								Pavel


> diff --git a/drivers/leds/leds-lm3697.c b/drivers/leds/leds-lm3697.c
> index 024983088d59..bd53450050b2 100644
> --- a/drivers/leds/leds-lm3697.c
> +++ b/drivers/leds/leds-lm3697.c
> @@ -78,8 +78,9 @@ struct lm3697 {
>  	struct mutex lock;
>  
>  	int bank_cfg;
> +	int num_banks;
>  
> -	struct lm3697_led leds[];
> +	struct lm3697_led banks[];
>  };
>  
>  static const struct reg_default lm3697_reg_defs[] = {
> @@ -180,8 +181,8 @@ static int lm3697_init(struct lm3697 *priv)
>  	if (ret)
>  		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");
>  
> -	for (i = 0; i < LM3697_MAX_CONTROL_BANKS; i++) {
> -		led = &priv->leds[i];
> +	for (i = 0; i < priv->num_banks; i++) {
> +		led = &priv->banks[i];
>  		ret = ti_lmu_common_set_ramp(&led->lmu_data);
>  		if (ret)
>  			dev_err(&priv->client->dev, "Setting the ramp rate failed\n");
> @@ -228,7 +229,7 @@ static int lm3697_probe_dt(struct lm3697 *priv)
>  			goto child_out;
>  		}
>  
> -		led = &priv->leds[i];
> +		led = &priv->banks[i];
>  
>  		ret = ti_lmu_common_get_brt_res(&priv->client->dev,
>  						child, &led->lmu_data);
> @@ -307,16 +308,17 @@ static int lm3697_probe(struct i2c_client *client,
>  	int ret;
>  
>  	count = device_get_child_node_count(&client->dev);
> -	if (!count) {
> -		dev_err(&client->dev, "LEDs are not defined in device tree!");
> -		return -ENODEV;
> +	if (!count || count > LM3697_MAX_CONTROL_BANKS) {
> +		return -EINVAL;
>  	}
>  
> -	led = devm_kzalloc(&client->dev, struct_size(led, leds, count),
> +	led = devm_kzalloc(&client->dev, struct_size(led, banks, count),
>  			   GFP_KERNEL);
>  	if (!led)
>  		return -ENOMEM;
>  
> +	led->num_banks = count;
> +
>  	mutex_init(&led->lock);
>  	i2c_set_clientdata(client, led);
>  
> -- 
> 2.28.0
> 


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)