| Message ID | 8d35529d-8ba9-4bdd-a3a3-00d67ab6f2d5@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Andi Shyti |
| Headers | show |
| Series | [v3] i2c: i801: Avoid potential double call to gpiod_remove_lookup_table | expand |
Hi Heiner, On Mon, Mar 04, 2024 at 09:31:06PM +0100, Heiner Kallweit wrote: > If registering the platform device fails, the lookup table is > removed in the error path. On module removal we would try to > remove the lookup table again. Fix this by setting priv->lookup > only if registering the platform device was successful. > In addition free the memory allocated for the lookup table in > the error path. > > Fixes: d308dfbf62ef ("i2c: mux/i801: Switch to use descriptor passing") > Cc: stable@vger.kernel.org > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > v2: > - cc stable > - free memory allocated for the lookup table > v3: > - cc'ed Linus this patch is a resend... > --- > drivers/i2c/busses/i2c-i801.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c > index 291c609d1..9c624f31c 100644 > --- a/drivers/i2c/busses/i2c-i801.c > +++ b/drivers/i2c/busses/i2c-i801.c > @@ -1428,7 +1428,6 @@ static void i801_add_mux(struct i801_priv *priv) > lookup->table[i] = GPIO_LOOKUP(mux_config->gpio_chip, > mux_config->gpios[i], "mux", 0); > gpiod_add_lookup_table(lookup); > - priv->lookup = lookup; if I miss something obvious in my reviews, please let me know... I spend most of my time reviewing patches and it happens quite often :-) Andi > /* > * Register the mux device, we use PLATFORM_DEVID_NONE here > @@ -1442,7 +1441,10 @@ static void i801_add_mux(struct i801_priv *priv) > sizeof(struct i2c_mux_gpio_platform_data)); > if (IS_ERR(priv->mux_pdev)) { > gpiod_remove_lookup_table(lookup); > + devm_kfree(dev, lookup); > dev_err(dev, "Failed to register i2c-mux-gpio device\n"); > + } else { > + priv->lookup = lookup; > } > } > > -- > 2.44.0 >
On 05.03.2024 00:04, Andi Shyti wrote: > Hi Heiner, > > On Mon, Mar 04, 2024 at 09:31:06PM +0100, Heiner Kallweit wrote: >> If registering the platform device fails, the lookup table is >> removed in the error path. On module removal we would try to >> remove the lookup table again. Fix this by setting priv->lookup >> only if registering the platform device was successful. >> In addition free the memory allocated for the lookup table in >> the error path. >> >> Fixes: d308dfbf62ef ("i2c: mux/i801: Switch to use descriptor passing") >> Cc: stable@vger.kernel.org >> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> v2: >> - cc stable >> - free memory allocated for the lookup table >> v3: >> - cc'ed Linus > > this patch is a resend... > Right, it's effectively a resend. >> --- >> drivers/i2c/busses/i2c-i801.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c >> index 291c609d1..9c624f31c 100644 >> --- a/drivers/i2c/busses/i2c-i801.c >> +++ b/drivers/i2c/busses/i2c-i801.c >> @@ -1428,7 +1428,6 @@ static void i801_add_mux(struct i801_priv *priv) >> lookup->table[i] = GPIO_LOOKUP(mux_config->gpio_chip, >> mux_config->gpios[i], "mux", 0); >> gpiod_add_lookup_table(lookup); >> - priv->lookup = lookup; > > if I miss something obvious in my reviews, please let me know... > I spend most of my time reviewing patches and it happens quite > often :-) > > Andi > >> /* >> * Register the mux device, we use PLATFORM_DEVID_NONE here >> @@ -1442,7 +1441,10 @@ static void i801_add_mux(struct i801_priv *priv) >> sizeof(struct i2c_mux_gpio_platform_data)); >> if (IS_ERR(priv->mux_pdev)) { >> gpiod_remove_lookup_table(lookup); >> + devm_kfree(dev, lookup); >> dev_err(dev, "Failed to register i2c-mux-gpio device\n"); >> + } else { >> + priv->lookup = lookup; >> } >> } >> >> -- >> 2.44.0 >>
On Mon, Mar 4, 2024 at 9:31 PM Heiner Kallweit <hkallweit1@gmail.com> wrote: > If registering the platform device fails, the lookup table is > removed in the error path. On module removal we would try to > remove the lookup table again. Fix this by setting priv->lookup > only if registering the platform device was successful. > In addition free the memory allocated for the lookup table in > the error path. > > Fixes: d308dfbf62ef ("i2c: mux/i801: Switch to use descriptor passing") > Cc: stable@vger.kernel.org > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > v2: > - cc stable > - free memory allocated for the lookup table > v3: > - cc'ed Linus My bug I guess, mea culpa. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
Hi Linus, > > If registering the platform device fails, the lookup table is > > removed in the error path. On module removal we would try to > > remove the lookup table again. Fix this by setting priv->lookup > > only if registering the platform device was successful. > > In addition free the memory allocated for the lookup table in > > the error path. > > > > Fixes: d308dfbf62ef ("i2c: mux/i801: Switch to use descriptor passing") > > Cc: stable@vger.kernel.org > > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > --- > > v2: > > - cc stable > > - free memory allocated for the lookup table > > v3: > > - cc'ed Linus > > My bug I guess, mea culpa. > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Thanks for your review! I have pushed this patch only in my testing branch so that i can still add your r-b. Andi
Hi On Mon, 04 Mar 2024 21:31:06 +0100, Heiner Kallweit wrote: > If registering the platform device fails, the lookup table is > removed in the error path. On module removal we would try to > remove the lookup table again. Fix this by setting priv->lookup > only if registering the platform device was successful. > In addition free the memory allocated for the lookup table in > the error path. > > [...] Applied to i2c/i2c-host-fixes on git://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git Thank you, Andi Patches applied =============== [1/1] i2c: i801: Avoid potential double call to gpiod_remove_lookup_table commit: 7c16cfd69d519ba14c9e9d474facd22d075ad14d
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 291c609d1..9c624f31c 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -1428,7 +1428,6 @@ static void i801_add_mux(struct i801_priv *priv) lookup->table[i] = GPIO_LOOKUP(mux_config->gpio_chip, mux_config->gpios[i], "mux", 0); gpiod_add_lookup_table(lookup); - priv->lookup = lookup; /* * Register the mux device, we use PLATFORM_DEVID_NONE here @@ -1442,7 +1441,10 @@ static void i801_add_mux(struct i801_priv *priv) sizeof(struct i2c_mux_gpio_platform_data)); if (IS_ERR(priv->mux_pdev)) { gpiod_remove_lookup_table(lookup); + devm_kfree(dev, lookup); dev_err(dev, "Failed to register i2c-mux-gpio device\n"); + } else { + priv->lookup = lookup; } }
If registering the platform device fails, the lookup table is removed in the error path. On module removal we would try to remove the lookup table again. Fix this by setting priv->lookup only if registering the platform device was successful. In addition free the memory allocated for the lookup table in the error path. Fixes: d308dfbf62ef ("i2c: mux/i801: Switch to use descriptor passing") Cc: stable@vger.kernel.org Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- v2: - cc stable - free memory allocated for the lookup table v3: - cc'ed Linus --- drivers/i2c/busses/i2c-i801.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)