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() | expand |
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)
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>
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!
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)
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(-)