All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] platform/x86: Fix memory leaks in build_tokens_sysfs()
@ 2018-03-16 12:10 Dan Carpenter
  2018-03-16 18:35 ` Andy Shevchenko
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Dan Carpenter @ 2018-03-16 12:10 UTC (permalink / raw)
  To: kernel-janitors

We're freeing "value_name" which is NULL, so that's a no-op, but we
intended to free "location_name" instead.  And then we don't free the
names in token_location_attrs[0] and token_value_attrs[0].

Fixes: 33b9ca1e53b4 ("platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: style changes (move from a for loop to a while loop) and this driver
    has been renamed.

diff --git a/drivers/platform/x86/dell-smbios-base.c b/drivers/platform/x86/dell-smbios-base.c
index 2485c80a9fdd..ec31d149f82e 100644
--- a/drivers/platform/x86/dell-smbios-base.c
+++ b/drivers/platform/x86/dell-smbios-base.c
@@ -514,7 +514,7 @@ static int build_tokens_sysfs(struct platform_device *dev)
 		continue;
 
 loop_fail_create_value:
-		kfree(value_name);
+		kfree(location_name);
 		goto out_unwind_strings;
 	}
 	smbios_attribute_group.attrs = token_attrs;
@@ -525,7 +525,7 @@ static int build_tokens_sysfs(struct platform_device *dev)
 	return 0;
 
 out_unwind_strings:
-	for (i = i-1; i > 0; i--) {
+	while (--i >= 0) {
 		kfree(token_location_attrs[i].attr.name);
 		kfree(token_value_attrs[i].attr.name);
 	}

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] platform/x86: Fix memory leaks in build_tokens_sysfs()
  2018-03-16 12:10 [PATCH v2] platform/x86: Fix memory leaks in build_tokens_sysfs() Dan Carpenter
@ 2018-03-16 18:35 ` Andy Shevchenko
  2018-03-16 21:52 ` Darren Hart
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2018-03-16 18:35 UTC (permalink / raw)
  To: kernel-janitors

On Fri, Mar 16, 2018 at 2:10 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> We're freeing "value_name" which is NULL, so that's a no-op, but we
> intended to free "location_name" instead.  And then we don't free the
> names in token_location_attrs[0] and token_value_attrs[0].
>
> Fixes: 33b9ca1e53b4 ("platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: style changes (move from a for loop to a while loop) and this driver
>     has been renamed.
>
> diff --git a/drivers/platform/x86/dell-smbios-base.c b/drivers/platform/x86/dell-smbios-base.c
> index 2485c80a9fdd..ec31d149f82e 100644
> --- a/drivers/platform/x86/dell-smbios-base.c
> +++ b/drivers/platform/x86/dell-smbios-base.c
> @@ -514,7 +514,7 @@ static int build_tokens_sysfs(struct platform_device *dev)
>                 continue;
>
>  loop_fail_create_value:
> -               kfree(value_name);
> +               kfree(location_name);
>                 goto out_unwind_strings;
>         }
>         smbios_attribute_group.attrs = token_attrs;
> @@ -525,7 +525,7 @@ static int build_tokens_sysfs(struct platform_device *dev)
>         return 0;
>
>  out_unwind_strings:
> -       for (i = i-1; i > 0; i--) {
> +       while (--i >= 0) {

Simple

while (i--) {

please.

Darren or me can fix this when apply, if you agree on the change.

>                 kfree(token_location_attrs[i].attr.name);
>                 kfree(token_value_attrs[i].attr.name);
>         }



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] platform/x86: Fix memory leaks in build_tokens_sysfs()
  2018-03-16 12:10 [PATCH v2] platform/x86: Fix memory leaks in build_tokens_sysfs() Dan Carpenter
  2018-03-16 18:35 ` Andy Shevchenko
@ 2018-03-16 21:52 ` Darren Hart
  2018-03-17  4:33 ` Dan Carpenter
  2018-04-02 15:30 ` Andy Shevchenko
  3 siblings, 0 replies; 5+ messages in thread
From: Darren Hart @ 2018-03-16 21:52 UTC (permalink / raw)
  To: kernel-janitors

On Fri, Mar 16, 2018 at 08:35:44PM +0200, Andy Shevchenko wrote:
> On Fri, Mar 16, 2018 at 2:10 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > We're freeing "value_name" which is NULL, so that's a no-op, but we
> > intended to free "location_name" instead.  And then we don't free the
> > names in token_location_attrs[0] and token_value_attrs[0].
> >
> > Fixes: 33b9ca1e53b4 ("platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: style changes (move from a for loop to a while loop) and this driver
> >     has been renamed.
> >
> > diff --git a/drivers/platform/x86/dell-smbios-base.c b/drivers/platform/x86/dell-smbios-base.c
> > index 2485c80a9fdd..ec31d149f82e 100644
> > --- a/drivers/platform/x86/dell-smbios-base.c
> > +++ b/drivers/platform/x86/dell-smbios-base.c
> > @@ -514,7 +514,7 @@ static int build_tokens_sysfs(struct platform_device *dev)
> >                 continue;
> >
> >  loop_fail_create_value:
> > -               kfree(value_name);
> > +               kfree(location_name);
> >                 goto out_unwind_strings;
> >         }
> >         smbios_attribute_group.attrs = token_attrs;
> > @@ -525,7 +525,7 @@ static int build_tokens_sysfs(struct platform_device *dev)
> >         return 0;
> >
> >  out_unwind_strings:
> > -       for (i = i-1; i > 0; i--) {
> > +       while (--i >= 0) {
> 
> Simple
> 
> while (i--) {

Yeah, i starts at 0 and is not decremented anywhere but here, so i--
seems sufficient to me.

-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] platform/x86: Fix memory leaks in build_tokens_sysfs()
  2018-03-16 12:10 [PATCH v2] platform/x86: Fix memory leaks in build_tokens_sysfs() Dan Carpenter
  2018-03-16 18:35 ` Andy Shevchenko
  2018-03-16 21:52 ` Darren Hart
@ 2018-03-17  4:33 ` Dan Carpenter
  2018-04-02 15:30 ` Andy Shevchenko
  3 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2018-03-17  4:33 UTC (permalink / raw)
  To: kernel-janitors

On Fri, Mar 16, 2018 at 08:35:44PM +0200, Andy Shevchenko wrote:
> On Fri, Mar 16, 2018 at 2:10 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > We're freeing "value_name" which is NULL, so that's a no-op, but we
> > intended to free "location_name" instead.  And then we don't free the
> > names in token_location_attrs[0] and token_value_attrs[0].
> >
> > Fixes: 33b9ca1e53b4 ("platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: style changes (move from a for loop to a while loop) and this driver
> >     has been renamed.
> >
> > diff --git a/drivers/platform/x86/dell-smbios-base.c b/drivers/platform/x86/dell-smbios-base.c
> > index 2485c80a9fdd..ec31d149f82e 100644
> > --- a/drivers/platform/x86/dell-smbios-base.c
> > +++ b/drivers/platform/x86/dell-smbios-base.c
> > @@ -514,7 +514,7 @@ static int build_tokens_sysfs(struct platform_device *dev)
> >                 continue;
> >
> >  loop_fail_create_value:
> > -               kfree(value_name);
> > +               kfree(location_name);
> >                 goto out_unwind_strings;
> >         }
> >         smbios_attribute_group.attrs = token_attrs;
> > @@ -525,7 +525,7 @@ static int build_tokens_sysfs(struct platform_device *dev)
> >         return 0;
> >
> >  out_unwind_strings:
> > -       for (i = i-1; i > 0; i--) {
> > +       while (--i >= 0) {
> 
> Simple
> 
> while (i--) {
> 
> please.
> 
> Darren or me can fix this when apply, if you agree on the change.
> 

Yes, please fix it yourself when you apply it.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] platform/x86: Fix memory leaks in build_tokens_sysfs()
  2018-03-16 12:10 [PATCH v2] platform/x86: Fix memory leaks in build_tokens_sysfs() Dan Carpenter
                   ` (2 preceding siblings ...)
  2018-03-17  4:33 ` Dan Carpenter
@ 2018-04-02 15:30 ` Andy Shevchenko
  3 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2018-04-02 15:30 UTC (permalink / raw)
  To: kernel-janitors

On Fri, Mar 16, 2018 at 2:10 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> We're freeing "value_name" which is NULL, so that's a no-op, but we
> intended to free "location_name" instead.  And then we don't free the
> names in token_location_attrs[0] and token_value_attrs[0].
>

Pushed to my reviewing and testing queue, thanks!

> Fixes: 33b9ca1e53b4 ("platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: style changes (move from a for loop to a while loop) and this driver
>     has been renamed.
>
> diff --git a/drivers/platform/x86/dell-smbios-base.c b/drivers/platform/x86/dell-smbios-base.c
> index 2485c80a9fdd..ec31d149f82e 100644
> --- a/drivers/platform/x86/dell-smbios-base.c
> +++ b/drivers/platform/x86/dell-smbios-base.c
> @@ -514,7 +514,7 @@ static int build_tokens_sysfs(struct platform_device *dev)
>                 continue;
>
>  loop_fail_create_value:
> -               kfree(value_name);
> +               kfree(location_name);
>                 goto out_unwind_strings;
>         }
>         smbios_attribute_group.attrs = token_attrs;
> @@ -525,7 +525,7 @@ static int build_tokens_sysfs(struct platform_device *dev)
>         return 0;
>
>  out_unwind_strings:
> -       for (i = i-1; i > 0; i--) {
> +       while (--i >= 0) {
>                 kfree(token_location_attrs[i].attr.name);
>                 kfree(token_value_attrs[i].attr.name);
>         }



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-04-02 15:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-16 12:10 [PATCH v2] platform/x86: Fix memory leaks in build_tokens_sysfs() Dan Carpenter
2018-03-16 18:35 ` Andy Shevchenko
2018-03-16 21:52 ` Darren Hart
2018-03-17  4:33 ` Dan Carpenter
2018-04-02 15:30 ` Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.