* [PATCH] platform/x86: alienware-wmi: fix kfree on potentially uninitialized pointer
@ 2019-03-30 0:17 Colin King
2019-04-03 22:02 ` Darren Hart
0 siblings, 1 reply; 5+ messages in thread
From: Colin King @ 2019-03-30 0:17 UTC (permalink / raw)
To: Darren Hart, Andy Shevchenko, platform-driver-x86
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently the kfree of output.pointer can be potentially freeing
an uninitalized pointer in the case where out_data is NULL. Fix this
by reworking the case where out_data is not-null to perform the
ACPI status check and also the kfree of outpoint.pointer in one block
and hence ensuring the pointer is only freed when it has been used.
Also replace the if (ptr != NULL) idiom with just if (ptr).
Fixes: ff0e9f26288d ("platform/x86: alienware-wmi: Correct a memory leak")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/platform/x86/alienware-wmi.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/platform/x86/alienware-wmi.c b/drivers/platform/x86/alienware-wmi.c
index f10af5c383c5..c0d1555735cd 100644
--- a/drivers/platform/x86/alienware-wmi.c
+++ b/drivers/platform/x86/alienware-wmi.c
@@ -522,23 +522,22 @@ static acpi_status alienware_wmax_command(struct wmax_basic_args *in_args,
input.length = (acpi_size) sizeof(*in_args);
input.pointer = in_args;
- if (out_data != NULL) {
+ if (out_data) {
output.length = ACPI_ALLOCATE_BUFFER;
output.pointer = NULL;
status = wmi_evaluate_method(WMAX_CONTROL_GUID, 0,
command, &input, &output);
- } else
+ if (ACPI_SUCCESS(status)) {
+ obj = (union acpi_object *)output.pointer;
+ if (obj && obj->type == ACPI_TYPE_INTEGER)
+ *out_data = (u32)obj->integer.value;
+ }
+ kfree(output.pointer);
+ } else {
status = wmi_evaluate_method(WMAX_CONTROL_GUID, 0,
command, &input, NULL);
-
- if (ACPI_SUCCESS(status) && out_data != NULL) {
- obj = (union acpi_object *)output.pointer;
- if (obj && obj->type == ACPI_TYPE_INTEGER)
- *out_data = (u32) obj->integer.value;
}
- kfree(output.pointer);
return status;
-
}
/*
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] platform/x86: alienware-wmi: fix kfree on potentially uninitialized pointer
2019-03-30 0:17 [PATCH] platform/x86: alienware-wmi: fix kfree on potentially uninitialized pointer Colin King
@ 2019-04-03 22:02 ` Darren Hart
2019-04-03 22:05 ` Colin Ian King
0 siblings, 1 reply; 5+ messages in thread
From: Darren Hart @ 2019-04-03 22:02 UTC (permalink / raw)
To: Colin King
Cc: Andy Shevchenko, platform-driver-x86, kernel-janitors,
linux-kernel
On Sat, Mar 30, 2019 at 12:17:12AM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently the kfree of output.pointer can be potentially freeing
> an uninitalized pointer in the case where out_data is NULL. Fix this
> by reworking the case where out_data is not-null to perform the
> ACPI status check and also the kfree of outpoint.pointer in one block
> and hence ensuring the pointer is only freed when it has been used.
>
> Also replace the if (ptr != NULL) idiom with just if (ptr).
>
> Fixes: ff0e9f26288d ("platform/x86: alienware-wmi: Correct a memory leak")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Thanks for the catch Colin, queued for testing.
Did you trigger this error or detect it via review or static analysis?
--
Darren Hart
VMware Open Source Technology Center
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] platform/x86: alienware-wmi: fix kfree on potentially uninitialized pointer
2019-04-03 22:02 ` Darren Hart
@ 2019-04-03 22:05 ` Colin Ian King
2019-04-03 22:26 ` Darren Hart
0 siblings, 1 reply; 5+ messages in thread
From: Colin Ian King @ 2019-04-03 22:05 UTC (permalink / raw)
To: Darren Hart
Cc: Andy Shevchenko, platform-driver-x86, kernel-janitors,
linux-kernel
On 03/04/2019 23:02, Darren Hart wrote:
> On Sat, Mar 30, 2019 at 12:17:12AM +0000, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Currently the kfree of output.pointer can be potentially freeing
>> an uninitalized pointer in the case where out_data is NULL. Fix this
>> by reworking the case where out_data is not-null to perform the
>> ACPI status check and also the kfree of outpoint.pointer in one block
>> and hence ensuring the pointer is only freed when it has been used.
>>
>> Also replace the if (ptr != NULL) idiom with just if (ptr).
>>
>> Fixes: ff0e9f26288d ("platform/x86: alienware-wmi: Correct a memory leak")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>
> Thanks for the catch Colin, queued for testing.
>
> Did you trigger this error or detect it via review or static analysis?
>
Static analysis, I'm now running a licensed version of Coverity on one
of our servers.
Colin
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] platform/x86: alienware-wmi: fix kfree on potentially uninitialized pointer
2019-04-03 22:05 ` Colin Ian King
@ 2019-04-03 22:26 ` Darren Hart
2019-04-03 22:27 ` Colin Ian King
0 siblings, 1 reply; 5+ messages in thread
From: Darren Hart @ 2019-04-03 22:26 UTC (permalink / raw)
To: Colin Ian King
Cc: Andy Shevchenko, platform-driver-x86, kernel-janitors,
linux-kernel
On Wed, Apr 03, 2019 at 11:05:12PM +0100, Colin Ian King wrote:
> On 03/04/2019 23:02, Darren Hart wrote:
> > On Sat, Mar 30, 2019 at 12:17:12AM +0000, Colin King wrote:
> >> From: Colin Ian King <colin.king@canonical.com>
> >>
> >> Currently the kfree of output.pointer can be potentially freeing
> >> an uninitalized pointer in the case where out_data is NULL. Fix this
> >> by reworking the case where out_data is not-null to perform the
> >> ACPI status check and also the kfree of outpoint.pointer in one block
> >> and hence ensuring the pointer is only freed when it has been used.
> >>
> >> Also replace the if (ptr != NULL) idiom with just if (ptr).
> >>
> >> Fixes: ff0e9f26288d ("platform/x86: alienware-wmi: Correct a memory leak")
> >> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >
> > Thanks for the catch Colin, queued for testing.
> >
> > Did you trigger this error or detect it via review or static analysis?
> >
> Static analysis, I'm now running a licensed version of Coverity on one
> of our servers.
We typically include the tool used to identify such bugs, and I see several such
tags for Coverity in the logs. Was there a reason not to include that tag? If
just an oversight, can you provide that tag and I'll amend the commit.
--
Darren Hart
VMware Open Source Technology Center
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] platform/x86: alienware-wmi: fix kfree on potentially uninitialized pointer
2019-04-03 22:26 ` Darren Hart
@ 2019-04-03 22:27 ` Colin Ian King
0 siblings, 0 replies; 5+ messages in thread
From: Colin Ian King @ 2019-04-03 22:27 UTC (permalink / raw)
To: Darren Hart
Cc: Andy Shevchenko, platform-driver-x86, kernel-janitors,
linux-kernel
On 03/04/2019 23:26, Darren Hart wrote:
> On Wed, Apr 03, 2019 at 11:05:12PM +0100, Colin Ian King wrote:
>> On 03/04/2019 23:02, Darren Hart wrote:
>>> On Sat, Mar 30, 2019 at 12:17:12AM +0000, Colin King wrote:
>>>> From: Colin Ian King <colin.king@canonical.com>
>>>>
>>>> Currently the kfree of output.pointer can be potentially freeing
>>>> an uninitalized pointer in the case where out_data is NULL. Fix this
>>>> by reworking the case where out_data is not-null to perform the
>>>> ACPI status check and also the kfree of outpoint.pointer in one block
>>>> and hence ensuring the pointer is only freed when it has been used.
>>>>
>>>> Also replace the if (ptr != NULL) idiom with just if (ptr).
>>>>
>>>> Fixes: ff0e9f26288d ("platform/x86: alienware-wmi: Correct a memory leak")
>>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>>
>>> Thanks for the catch Colin, queued for testing.
>>>
>>> Did you trigger this error or detect it via review or static analysis?
>>>
>> Static analysis, I'm now running a licensed version of Coverity on one
>> of our servers.
>
> We typically include the tool used to identify such bugs, and I see several such
> tags for Coverity in the logs. Was there a reason not to include that tag? If
> just an oversight, can you provide that tag and I'll amend the commit.
>
I didn't have an external coverity CID# number so I omitted it this time.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-03 22:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-30 0:17 [PATCH] platform/x86: alienware-wmi: fix kfree on potentially uninitialized pointer Colin King
2019-04-03 22:02 ` Darren Hart
2019-04-03 22:05 ` Colin Ian King
2019-04-03 22:26 ` Darren Hart
2019-04-03 22:27 ` Colin Ian King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox