[v1] i2c: i801: Avoid memory leak in check_acpi_smo88xx_device()
diff mbox series

Message ID 20190816131705.77750-1-andriy.shevchenko@linux.intel.com
State Accepted
Headers show
Series
  • [v1] i2c: i801: Avoid memory leak in check_acpi_smo88xx_device()
Related show

Commit Message

Andy Shevchenko Aug. 16, 2019, 1:17 p.m. UTC
check_acpi_smo88xx_device() utilizes acpi_get_object_info() which in its turn
allocates a buffer. User is responsible to clean allocated resources. The last
has been missed in the original code. Fix it here.

While here, replace !ACPI_SUCCESS() with ACPI_FAILURE().

Fixes: 19b07cb4a187 ("i2c: i801: Register optional lis3lv02d I2C device on Dell machines")
Cc: Pali Rohár <pali.rohar@gmail.com>
Cc: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-i801.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Pali Rohár Aug. 16, 2019, 1:23 p.m. UTC | #1
On Friday 16 August 2019 16:17:05 Andy Shevchenko wrote:
> check_acpi_smo88xx_device() utilizes acpi_get_object_info() which in its turn
> allocates a buffer. User is responsible to clean allocated resources. The last
> has been missed in the original code. Fix it here.
> 
> While here, replace !ACPI_SUCCESS() with ACPI_FAILURE().
> 
> Fixes: 19b07cb4a187 ("i2c: i801: Register optional lis3lv02d I2C device on Dell machines")
> Cc: Pali Rohár <pali.rohar@gmail.com>
> Cc: Jean Delvare <jdelvare@suse.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Nice catch! Thank you.

Reviewed-by: Pali Rohár <pali.rohar@gmail.com>

> ---
>  drivers/i2c/busses/i2c-i801.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 09b7e27ca97b..1e4cde64293b 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -1194,19 +1194,28 @@ static acpi_status check_acpi_smo88xx_device(acpi_handle obj_handle,
>  	int i;
>  
>  	status = acpi_get_object_info(obj_handle, &info);
> -	if (!ACPI_SUCCESS(status) || !(info->valid & ACPI_VALID_HID))
> +	if (ACPI_FAILURE(status))
>  		return AE_OK;
>  
> +	if (!(info->valid & ACPI_VALID_HID))
> +		goto smo88xx_not_found;
> +
>  	hid = info->hardware_id.string;
>  	if (!hid)
> -		return AE_OK;
> +		goto smo88xx_not_found;
>  
>  	i = match_string(acpi_smo8800_ids, ARRAY_SIZE(acpi_smo8800_ids), hid);
>  	if (i < 0)
> -		return AE_OK;
> +		goto smo88xx_not_found;
> +
> +	kfree(info);
>  
>  	*((bool *)return_value) = true;
>  	return AE_CTRL_TERMINATE;
> +
> +smo88xx_not_found:
> +	kfree(info);
> +	return AE_OK;
>  }
>  
>  static bool is_dell_system_with_lis3lv02d(void)
Jean Delvare Aug. 28, 2019, 12:16 p.m. UTC | #2
Hi Andy,

On Fri, 16 Aug 2019 16:17:05 +0300, Andy Shevchenko wrote:
> check_acpi_smo88xx_device() utilizes acpi_get_object_info() which in its turn
> allocates a buffer. User is responsible to clean allocated resources. The last
> has been missed in the original code. Fix it here.

FYI checkpatch.pl recommends not exceeding 75 columns for patch
descriptions (presumably because you get 4 blanks added in front by git
later).

> 
> While here, replace !ACPI_SUCCESS() with ACPI_FAILURE().
> 
> Fixes: 19b07cb4a187 ("i2c: i801: Register optional lis3lv02d I2C device on Dell machines")
> Cc: Pali Rohár <pali.rohar@gmail.com>
> Cc: Jean Delvare <jdelvare@suse.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/i2c/busses/i2c-i801.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> (...)

Good catch.

Reviewed-by: Jean Delvare <jdelvare@suse.de>
Wolfram Sang Aug. 29, 2019, 6:47 p.m. UTC | #3
On Fri, Aug 16, 2019 at 04:17:05PM +0300, Andy Shevchenko wrote:
> check_acpi_smo88xx_device() utilizes acpi_get_object_info() which in its turn
> allocates a buffer. User is responsible to clean allocated resources. The last
> has been missed in the original code. Fix it here.
> 
> While here, replace !ACPI_SUCCESS() with ACPI_FAILURE().
> 
> Fixes: 19b07cb4a187 ("i2c: i801: Register optional lis3lv02d I2C device on Dell machines")
> Cc: Pali Rohár <pali.rohar@gmail.com>
> Cc: Jean Delvare <jdelvare@suse.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to for-current, thanks!

Patch
diff mbox series

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 09b7e27ca97b..1e4cde64293b 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1194,19 +1194,28 @@  static acpi_status check_acpi_smo88xx_device(acpi_handle obj_handle,
 	int i;
 
 	status = acpi_get_object_info(obj_handle, &info);
-	if (!ACPI_SUCCESS(status) || !(info->valid & ACPI_VALID_HID))
+	if (ACPI_FAILURE(status))
 		return AE_OK;
 
+	if (!(info->valid & ACPI_VALID_HID))
+		goto smo88xx_not_found;
+
 	hid = info->hardware_id.string;
 	if (!hid)
-		return AE_OK;
+		goto smo88xx_not_found;
 
 	i = match_string(acpi_smo8800_ids, ARRAY_SIZE(acpi_smo8800_ids), hid);
 	if (i < 0)
-		return AE_OK;
+		goto smo88xx_not_found;
+
+	kfree(info);
 
 	*((bool *)return_value) = true;
 	return AE_CTRL_TERMINATE;
+
+smo88xx_not_found:
+	kfree(info);
+	return AE_OK;
 }
 
 static bool is_dell_system_with_lis3lv02d(void)