From: Axel Lin <axel.lin@xxxxxxxxx> When acpi_evaluate_object() is passed ACPI_ALLOCATE_BUFFER, the caller must kfree the returned buffer if AE_OK is returned. Call Trace: wmab_execute -> wmi_evaluate_method -> acpi_evaluate_object Thus if callers of wmab_execute() pass ACPI_ALLOCATE_BUFFER, the return buffer must be kfreed if wmab_execute return AE_OK. [akpm@xxxxxxxxxxxxxxxxxxxx: avoid multiple return points, remove unneeded cast, remove unneeded initialisation of `status'] Signed-off-by: Axel Lin <axel.lin@xxxxxxxxx> Acked-by: Carlos Corbacho <carlos@xxxxxxxxxxxxxxxxxxx> Cc: Matthew Garrett <mjg@xxxxxxxxxx> Cc: Thomas Renninger <trenn@xxxxxxx> Cc: Alan Jenkins <alan-jenkins@xxxxxxxxxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> --- drivers/platform/x86/acer-wmi.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff -puN drivers/platform/x86/acer-wmi.c~acer-wmi-fix-memory-leaks-in-wmab_execute-error-path drivers/platform/x86/acer-wmi.c --- a/drivers/platform/x86/acer-wmi.c~acer-wmi-fix-memory-leaks-in-wmab_execute-error-path +++ a/drivers/platform/x86/acer-wmi.c @@ -555,6 +555,7 @@ static acpi_status AMW0_find_mailled(voi obj->buffer.length == sizeof(struct wmab_ret)) { ret = *((struct wmab_ret *) obj->buffer.pointer); } else { + kfree(out.pointer); return AE_ERROR; } @@ -570,7 +571,7 @@ static acpi_status AMW0_set_capabilities { struct wmab_args args; struct wmab_ret ret; - acpi_status status = AE_OK; + acpi_status status; struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL }; union acpi_object *obj; @@ -593,12 +594,13 @@ static acpi_status AMW0_set_capabilities if (ACPI_FAILURE(status)) return status; - obj = (union acpi_object *) out.pointer; + obj = out.pointer; if (obj && obj->type == ACPI_TYPE_BUFFER && obj->buffer.length == sizeof(struct wmab_ret)) { ret = *((struct wmab_ret *) obj->buffer.pointer); } else { - return AE_ERROR; + status = AE_ERROR; + goto out; } if (ret.eax & 0x1) @@ -607,23 +609,26 @@ static acpi_status AMW0_set_capabilities args.ebx = 2 << 8; args.ebx |= ACER_AMW0_BLUETOOTH_MASK; + /* + * It's ok to use existing buffer for next wmab_execute call. + * But we need to kfree(out.pointer) if next wmab_execute fail. + */ status = wmab_execute(&args, &out); if (ACPI_FAILURE(status)) - return status; + goto out; obj = (union acpi_object *) out.pointer; if (obj && obj->type == ACPI_TYPE_BUFFER && obj->buffer.length == sizeof(struct wmab_ret)) { ret = *((struct wmab_ret *) obj->buffer.pointer); } else { - return AE_ERROR; + status = AE_ERROR; + goto out; } if (ret.eax & 0x1) interface->capability |= ACER_CAP_BLUETOOTH; - kfree(out.pointer); - /* * This appears to be safe to enable, since all Wistron based laptops * appear to use the same EC register for brightness, even if they @@ -632,7 +637,10 @@ static acpi_status AMW0_set_capabilities if (quirks->brightness >= 0) interface->capability |= ACER_CAP_BRIGHTNESS; - return AE_OK; + status = AE_OK; +out: + kfree(out.pointer); + return status; } static struct wmi_interface AMW0_interface = { _ -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html