[PATCH v1 2/2] platform/x86: toshiba_haps: Fix memory leaks in add/remove routines

Rafael J. Wysocki posted 1 patch 2 weeks, 3 days ago
drivers/platform/x86/toshiba_haps.c |   13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
[PATCH v1 2/2] platform/x86: toshiba_haps: Fix memory leaks in add/remove routines
Posted by Rafael J. Wysocki 2 weeks, 3 days ago
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

toshiba_haps_add() leaks the haps object allocated by it if it returns
an error after allocating that object successfully.

toshiba_haps_remove() does not free the object pointed to by
toshiba_haps before clearing that pointer, so it becomes unreachable
allocated memory.

Address these memory leaks by freeing the memory in question as
appropriate.

Fixes: 23d0ba0c908a ("platform/x86: Toshiba HDD Active Protection Sensor")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/toshiba_haps.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

--- a/drivers/platform/x86/toshiba_haps.c
+++ b/drivers/platform/x86/toshiba_haps.c
@@ -142,8 +142,8 @@ static void toshiba_haps_remove(struct a
 {
 	sysfs_remove_group(&device->dev.kobj, &haps_attr_group);
 
-	if (toshiba_haps)
-		toshiba_haps = NULL;
+	kfree(toshiba_haps);
+	toshiba_haps = NULL;
 }
 
 /* Helper function */
@@ -195,15 +195,20 @@ static int toshiba_haps_add(struct acpi_
 	/* Set the protection level, currently at level 2 (Medium) */
 	ret = toshiba_haps_protection_level(acpi_dev->handle, 2);
 	if (ret != 0)
-		return ret;
+		goto err;
 
 	ret = sysfs_create_group(&acpi_dev->dev.kobj, &haps_attr_group);
 	if (ret)
-		return ret;
+		goto err;
 
 	toshiba_haps = haps;
 
 	return 0;
+
+err:
+	kfree(haps);
+
+	return ret;
 }
 
 #ifdef CONFIG_PM_SLEEP
[PATCH v2 2/2] platform/x86: toshiba_haps: Fix memory leaks in add/remove routines
Posted by Rafael J. Wysocki 2 weeks, 3 days ago
On Tuesday, January 20, 2026 4:44:23 PM CET Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> toshiba_haps_add() leaks the haps object allocated by it if it returns
> an error after allocating that object successfully.
> 
> toshiba_haps_remove() does not free the object pointed to by
> toshiba_haps before clearing that pointer, so it becomes unreachable
> allocated memory.
> 
> Address these memory leaks by freeing the memory in question as
> appropriate.

Actually, this one can use devm_kzalloc() for allocating memory instead,
so here's a v2.

---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH v2 2/2] platform/x86: toshiba_haps: Fix memory leaks in add/remove routines

toshiba_haps_add() leaks the haps object allocated by it if it returns
an error after allocating that object successfully.

toshiba_haps_remove() does not free the object pointed to by
toshiba_haps before clearing that pointer, so it becomes unreachable
allocated memory.

Address these memory leaks by using devm_kzalloc() for allocating
the memory in question.

Fixes: 23d0ba0c908a ("platform/x86: Toshiba HDD Active Protection Sensor")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2:
   * Use devm_kzalloc() for memory allocation and drop the rest of the changes.

---
 drivers/platform/x86/toshiba_haps.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/platform/x86/toshiba_haps.c
+++ b/drivers/platform/x86/toshiba_haps.c
@@ -183,7 +183,7 @@ static int toshiba_haps_add(struct acpi_
 
 	pr_info("Toshiba HDD Active Protection Sensor device\n");
 
-	haps = kzalloc(sizeof(struct toshiba_haps_dev), GFP_KERNEL);
+	haps = devm_kzalloc(&acpi_dev->dev, sizeof(*haps), GFP_KERNEL);
 	if (!haps)
 		return -ENOMEM;