Skip to contentSkip to conversation

platform/x86: Fix memory leaks in build_tokens_sysfs()

PATCH In review Dan Carpenter 3 participants 5 messages Mar 16, 2018 – Apr 2, 2018

Subsystems (2)
Kernel janitors x86 platform drivers
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH v2 1/1 platform/x86: Fix memory leaks in build_tokens_sysfs()

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 <[email protected]>

---
v2: style changes (move from a for loop to a while loop) and this driver
has been renamed.

drivers/platform/x86/dell-smbios-base.c +2 -2
index 2485c80a9fdd..ec31d149f82e 100644
@@ -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);
 	}
#1 lore raw
Quoted 24 lines · Dan Carpenter

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 <[email protected]>

---
v2: style changes (move from a for loop to a while loop) and this driver
has been renamed.

drivers/platform/x86/dell-smbios-base.c +1 -1
index 2485c80a9fdd..ec31d149f82e 100644
@@ -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;

drivers/platform/x86/dell-smbios-base.c static int build_tokens_sysfs(struct platform_device *dev) · line 528

Quoted 5 lines
drivers/platform/x86/dell-smbios-base.c +1 -1
@@ -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.

Quoted 3 lines
drivers/platform/x86/dell-smbios-base.c +0 -0
                kfree(token_location_attrs[i].attr.name);
                kfree(token_value_attrs[i].attr.name);
        }
Signature · 2 lines
With Best Regards,
Andy Shevchenko
#2 lore raw
In reply to Dan Carpenter
Quoted 27 lines · Andy Shevchenko
Quoted 26 lines · Dan Carpenter

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 <[email protected]>

---
v2: style changes (move from a for loop to a while loop) and this driver
has been renamed.

drivers/platform/x86/dell-smbios-base.c +1 -1
index 2485c80a9fdd..ec31d149f82e 100644
@@ -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;
Quoted 7 lines

drivers/platform/x86/dell-smbios-base.c static int build_tokens_sysfs(struct platform_device *dev) · line 528

Quoted 3 lines
drivers/platform/x86/dell-smbios-base.c +1 -1
 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.

Signature · 2 lines
Darren Hart
VMware Open Source Technology Center
author
#3 lore raw
In reply to Dan Carpenter
Quoted 33 lines · Andy Shevchenko
Quoted 24 lines · Dan Carpenter

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 <[email protected]>

---
v2: style changes (move from a for loop to a while loop) and this driver
has been renamed.

drivers/platform/x86/dell-smbios-base.c +1 -1
index 2485c80a9fdd..ec31d149f82e 100644
@@ -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;

drivers/platform/x86/dell-smbios-base.c static int build_tokens_sysfs(struct platform_device *dev) · line 528

Quoted 5 lines
drivers/platform/x86/dell-smbios-base.c +1 -1
@@ -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

Quoted 6 lines

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

#4 lore raw
In reply to Dan Carpenter
Quoted 4 lines · Dan Carpenter

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!

Quoted 28 lines
Fixes 33b9ca1e53b4 ("platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens")
Signed-off-by Dan Carpenter <[email protected]>

---
v2: style changes (move from a for loop to a while loop) and this driver
has been renamed.

drivers/platform/x86/dell-smbios-base.c +2 -2
index 2485c80a9fdd..ec31d149f82e 100644
@@ -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);
        }
Signature · 2 lines
With Best Regards,
Andy Shevchenko