re: acer-wmi: fix obj is NULL but dereferenced

re: acer-wmi: fix obj is NULL but dereferenced

[Dan Carpenter]

Hello Lee, Chun-Yi,

The patch 727651bf738b: "acer-wmi: fix obj is NULL but dereferenced" 
from Dec 14, 2012, leads to the following warning:
drivers/platform/x86/acer-wmi.c:1227 WMID_set_capabilities()
	 warn: calling kfree() when 'out.pointer' is always NULL.

  1217          obj = (union acpi_object *) out.pointer;
  1218          if (obj) {
  1219                  if (obj->type == ACPI_TYPE_BUFFER &&
  1220                          (obj->buffer.length == sizeof(u32) ||
  1221                          obj->buffer.length == sizeof(u64))) {
  1222                          devices = *((u32 *) obj->buffer.pointer);
  1223                  } else if (obj->type == ACPI_TYPE_INTEGER) {
  1224                          devices = (u32) obj->integer.value;
  1225                  }
  1226          } else {
  1227                  kfree(out.pointer);
  1228                  return AE_ERROR;
  1229          }

I was going to remove the kfree() but looking the patch which introduced
it causes GCC warnings and so there is some other stuff which needs to
be fixed as well.

regards,
dan carpenter

re: acer-wmi: fix obj is NULL but dereferenced

[joeyli]

Hi Dan, 

Sorry for I am late to reply you!

於 一，2013-01-07 於 14:32 +0300，Dan Carpenter 提到：
> Hello Lee, Chun-Yi,
> 
> The patch 727651bf738b: "acer-wmi: fix obj is NULL but dereferenced" 
> from Dec 14, 2012, leads to the following warning:
> drivers/platform/x86/acer-wmi.c:1227 WMID_set_capabilities()
> 	 warn: calling kfree() when 'out.pointer' is always NULL.
> 
>   1217          obj = (union acpi_object *) out.pointer;
>   1218          if (obj) {
>   1219                  if (obj->type == ACPI_TYPE_BUFFER &&
>   1220                          (obj->buffer.length == sizeof(u32) ||
>   1221                          obj->buffer.length == sizeof(u64))) {
>   1222                          devices = *((u32 *) obj->buffer.pointer);
>   1223                  } else if (obj->type == ACPI_TYPE_INTEGER) {
>   1224                          devices = (u32) obj->integer.value;
>   1225                  }
>   1226          } else {
>   1227                  kfree(out.pointer);
>   1228                  return AE_ERROR;
>   1229          }
> 
> I was going to remove the kfree() but looking the patch which introduced
> it causes GCC warnings and so there is some other stuff which needs to
> be fixed as well.
> 
> regards,
> dan carpenter
> 
> 

The warning will be fixed by Zhang Rui or Paul Bolle's patch:

http://www.mail-archive.com/platform-driver-x86@vger.kernel.org/msg04016.html
http://lkml.org/lkml/2013/1/24/215

With the above patch, kfree() will called when:
 + obj is not NULL but the type is not BUFFER or INTEGER
or
 + obj is NULL. The kfree() check obj is NULL then direct return.

Still appreciate for your review and suggestion!


Thanks a lot!
Joey Lee