Hei hei, On Mon, Oct 05, 2020 at 05:35:38PM +0200, ultracoolguy@xxxxxxxxxxxx wrote: > 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. Note: your patch, especially the commit message, still needs a Signed-off-by line. Please read [1] (again?) and resend. Greets Alex [1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html > Oct 5, 2020, 14:41 by dmurphy@xxxxxx: > > > Gabriel > > > > On 10/5/20 9:38 AM, ultracoolguy@xxxxxxxxxxxx wrote: > > > >> I understand. So I should leave it like it was and do the rename in another patch? > >> > > > > You should do the fix in one patch and leave the structure name alone. > > > > The structure naming if fine and has no benefit and actually will make it more difficult for others to backport future fixes. > > > > Unless Pavel finds benefit in accepting the structure rename. > > > > Dan > > > > >From ee004d26bb2f91491141aa06f5518cc411711ff0 Mon Sep 17 00:00:00 2001 > From: Ultracoolguy <ultracoolguy@xxxxxxxxxxxx> > Date: Fri, 2 Oct 2020 18:27:00 -0400 > Subject: [PATCH] leds:lm3697:Fix out-of-bound access > > If both led banks aren't used in device tree, > an out-of-bounds condition in lm3697_init occurs > because of the for loop assuming that all the banks are used. > Fix it by adding a variable that contains the number of used banks. > --- > drivers/leds/leds-lm3697.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > 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 > -- /"\ ASCII RIBBON | »With the first link, the chain is forged. The first \ / CAMPAIGN | speech censured, the first thought forbidden, the X AGAINST | first freedom denied, chains us all irrevocably.« / \ HTML MAIL | (Jean-Luc Picard, quoting Judge Aaron Satie)
Attachment:
signature.asc
Description: PGP signature
