* [PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
@ 2017-02-03 20:56 João Paulo Rechi Vita
2017-02-04 4:19 ` Moore, Robert
0 siblings, 1 reply; 4+ messages in thread
From: João Paulo Rechi Vita @ 2017-02-03 20:56 UTC (permalink / raw)
To: Robert Moore, Lv Zheng, Rafael J . Wysocki, Len Brown, Lin Ming
Cc: linux-acpi, devel, linux-kernel, Daniel Drake, linux,
João Paulo Rechi Vita
When acpi_ns_repair_CID() is called for a _CID which returns a package
of strings, it calls acpi_ns_repair_HID() for each of the package
elements. acpi_ns_repair_HID() calls acpi_ut_remove_reference() on the
original object, but acpi_ns_repair_CID() calls it again on return,
leading to a double free.
This problem was seen on a Acer TravelMate P449-G2-MG.
Thanks to Daniel Drake for helping investigating this problem.
Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
drivers/acpi/acpica/nsrepair2.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/acpi/acpica/nsrepair2.c b/drivers/acpi/acpica/nsrepair2.c
index d5336122486b..c429c8eca476 100644
--- a/drivers/acpi/acpica/nsrepair2.c
+++ b/drivers/acpi/acpica/nsrepair2.c
@@ -411,8 +411,6 @@ acpi_ns_repair_CID(struct acpi_evaluate_info *info,
(*element_ptr)->common.reference_count =
original_ref_count;
-
- acpi_ut_remove_reference(original_element);
}
element_ptr++;
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
2017-02-03 20:56 [PATCH] acpica: Fix double-free in acpi_ns_repair_CID() João Paulo Rechi Vita
@ 2017-02-04 4:19 ` Moore, Robert
2017-02-06 1:44 ` Zheng, Lv
0 siblings, 1 reply; 4+ messages in thread
From: Moore, Robert @ 2017-02-04 4:19 UTC (permalink / raw)
To: João Paulo Rechi Vita, Zheng, Lv, Wysocki, Rafael J,
Len Brown, Lin Ming
Cc: linux-acpi, devel, linux-kernel, Daniel Drake, linux,
João Paulo Rechi Vita, Box, David E, Schmauss, Erik
Here's the sequence of events as I see it:
Repair_HID is a standalone function that removes one reference on the incoming object. For simple _HID objects, this in fact deletes the object.
For _CID, all elements of the package are examined. If a repair was made on a _HID within the _CID function, one reference on the original object was removed by Repair_HID. However, since the object is part of a package, it has an extra reference to reflect this fact. Thus, in the case in question, the elements of the package all have at least two references. Repair_HID removes one reference, thus the extra RemoveReference is needed in Repair_CID to bring the reference count down to zero actually delete the object (in the typical case where the object had two references).
Bob
> -----Original Message-----
> From: João Paulo Rechi Vita [mailto:jprvita@gmail.com]
> Sent: Friday, February 03, 2017 12:57 PM
> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len Brown; Lin Ming
> Cc: linux-acpi@vger.kernel.org; devel@acpica.org; linux-
> kernel@vger.kernel.org; Daniel Drake; linux@endlessm.com; João Paulo Rechi
> Vita
> Subject: [PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
>
> When acpi_ns_repair_CID() is called for a _CID which returns a package of
> strings, it calls acpi_ns_repair_HID() for each of the package elements.
> acpi_ns_repair_HID() calls acpi_ut_remove_reference() on the original
> object, but acpi_ns_repair_CID() calls it again on return, leading to a
> double free.
>
> This problem was seen on a Acer TravelMate P449-G2-MG.
>
> Thanks to Daniel Drake for helping investigating this problem.
>
> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
> ---
> drivers/acpi/acpica/nsrepair2.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/acpi/acpica/nsrepair2.c
> b/drivers/acpi/acpica/nsrepair2.c index d5336122486b..c429c8eca476 100644
> --- a/drivers/acpi/acpica/nsrepair2.c
> +++ b/drivers/acpi/acpica/nsrepair2.c
> @@ -411,8 +411,6 @@ acpi_ns_repair_CID(struct acpi_evaluate_info *info,
>
> (*element_ptr)->common.reference_count =
> original_ref_count;
> -
> - acpi_ut_remove_reference(original_element);
> }
>
> element_ptr++;
> --
> 2.11.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
2017-02-04 4:19 ` Moore, Robert
@ 2017-02-06 1:44 ` Zheng, Lv
[not found] ` <CA+A7VXXNk58unA+k6743VV7=cHTTbW8YHVf_5GAYdv1EPA5dPw@mail.gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Zheng, Lv @ 2017-02-06 1:44 UTC (permalink / raw)
To: Moore, Robert, Jo?o Paulo Rechi Vita, Wysocki, Rafael J,
Len Brown, Lin Ming
Cc: linux-acpi, devel, linux-kernel, Daniel Drake, linux,
Jo?o Paulo Rechi Vita, Box, David E, Schmauss, Erik
Hi,
So if a real problem related to package reference counting is triggered, the problem should be fixed elsewhere IMO.
See this bug for reference:
https://bugs.acpica.org/show_bug.cgi?id=1336
Thanks and best regards
Lv
> From: Moore, Robert
> Subject: RE: [PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
>
> Here's the sequence of events as I see it:
>
> Repair_HID is a standalone function that removes one reference on the incoming object. For simple _HID
> objects, this in fact deletes the object.
>
> For _CID, all elements of the package are examined. If a repair was made on a _HID within the _CID
> function, one reference on the original object was removed by Repair_HID. However, since the object is
> part of a package, it has an extra reference to reflect this fact. Thus, in the case in question, the
> elements of the package all have at least two references. Repair_HID removes one reference, thus the
> extra RemoveReference is needed in Repair_CID to bring the reference count down to zero actually
> delete the object (in the typical case where the object had two references).
>
> Bob
>
>
> > From: João Paulo Rechi Vita [mailto:jprvita@gmail.com]
> > Subject: [PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
> >
> > When acpi_ns_repair_CID() is called for a _CID which returns a package of
> > strings, it calls acpi_ns_repair_HID() for each of the package elements.
> > acpi_ns_repair_HID() calls acpi_ut_remove_reference() on the original
> > object, but acpi_ns_repair_CID() calls it again on return, leading to a
> > double free.
> >
> > This problem was seen on a Acer TravelMate P449-G2-MG.
> >
> > Thanks to Daniel Drake for helping investigating this problem.
> >
> > Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
> > ---
> > drivers/acpi/acpica/nsrepair2.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/nsrepair2.c
> > b/drivers/acpi/acpica/nsrepair2.c index d5336122486b..c429c8eca476 100644
> > --- a/drivers/acpi/acpica/nsrepair2.c
> > +++ b/drivers/acpi/acpica/nsrepair2.c
> > @@ -411,8 +411,6 @@ acpi_ns_repair_CID(struct acpi_evaluate_info *info,
> >
> > (*element_ptr)->common.reference_count =
> > original_ref_count;
> > -
> > - acpi_ut_remove_reference(original_element);
> > }
> >
> > element_ptr++;
> > --
> > 2.11.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
[not found] ` <CA+A7VXXNk58unA+k6743VV7=cHTTbW8YHVf_5GAYdv1EPA5dPw@mail.gmail.com>
@ 2017-02-06 15:06 ` Daniel Drake
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Drake @ 2017-02-06 15:06 UTC (permalink / raw)
To: João Paulo Rechi Vita
Cc: Zheng, Lv, Moore, Robert, Wysocki, Rafael J, Len Brown, Lin Ming,
linux-acpi, devel, linux-kernel, linux, Jo?o Paulo Rechi Vita,
Box, David E, Schmauss, Erik
On Mon, Feb 6, 2017 at 8:46 AM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
> Yes, the real problem is i2c_hid not being probed for the touchpad
> device on this platform
The lack of probe was because when i2c_acpi_add_device() called
acpi_bus_get_device() on the handle for the touchpad device (TPL1), it
was returning the acpi_device for something totally unrelated.
We checked that the original handle-to-device mapping was set up
correctly before that point, hence this looks like memory corruption,
and the problem went away as soon as we fixed the double free.
> [ 0.317002] ACPI Warning: Obj ffffa00472a445e8, Reference Count is
> already zero, cannot decrement
> [ 0.317178] (20160422/utdelete-442)
>
>> See this bug for reference:
>> https://bugs.acpica.org/show_bug.cgi?id=1336
>>
>
> Looks like it could be the same problem, indeed. I'm attaching a
> kernel log with acpi.trace_debug_layer=0x10091
> acpi.trace_debug_level=0x107FFF
> acpi.trace_method_name=_SB.PCI0.I2C1.TPL1._CID
> acpi.trace_state=opcode, which is what I was using to investigate the
> problem
The debug messages in that log stop when the method execution
completes, so it may also be useful to share a part of an earlier log
where you can see the double freeing happen in a bit more detail. For
each of the 2 objects double-freed below we confirmed that the first
unreference is the one in acpi_ns_repair_HID and the second
unreference is the one removed in JP's patch.
nsprepkg-0105 ns_check_package : \_SB.PCI0.I2C1.TPL1._CID
Validating return Package of Type 2, Count 2
utobject-0404 ut_allocate_object_des: ffff940f6ba36480 Size 48
utdelete-0750 ut_remove_reference : Obj ffff940f6ba36cf0 Current
Refs=1 [To Be Decremented]
utdelete-0447 ut_update_ref_count : Obj ffff940f6ba36cf0 Type 02
Refs 00 [Decremented]
utdelete-0096 ut_delete_internal_obj: **** String ffff940f6ba36cf0,
ptr ffff9f654004e2a8
utdelete-0336 ut_delete_internal_obj: Deleting Object ffff940f6ba36cf0 [String]
utdelete-0750 ut_remove_reference : Obj ffff940f6ba36cf0 Current
Refs=0 [To Be Decremented]
ACPI Warning: Obj ffff940f6ba36cf0, Reference Count is already zero,
cannot decrement
(20160422/utdelete-442)
utdelete-0447 ut_update_ref_count : Obj ffff940f6ba36cf0 Type 02
Refs 00 [Decremented]
utdelete-0096 ut_delete_internal_obj: **** String ffff940f6ba36cf0,
ptr ffff9f654004e2a8
utdelete-0336 ut_delete_internal_obj: Deleting Object ffff940f6ba36cf0 [String]
utobject-0404 ut_allocate_object_des: ffff940f6ba36cf0 Size 48
utdelete-0750 ut_remove_reference : Obj ffff940f6ba36c60 Current
Refs=1 [To Be Decremented]
utdelete-0447 ut_update_ref_count : Obj ffff940f6ba36c60 Type 02
Refs 00 [Decremented]
utdelete-0096 ut_delete_internal_obj: **** String ffff940f6ba36c60,
ptr ffff9f654004e2b2
utdelete-0336 ut_delete_internal_obj: Deleting Object ffff940f6ba36c60 [String]
utdelete-0750 ut_remove_reference : Obj ffff940f6ba36c60 Current
Refs=0 [To Be Decremented]
ACPI Warning: Obj ffff940f6ba36c60, Reference Count is already zero,
cannot decrement
(20160422/utdelete-442)
utdelete-0447 ut_update_ref_count : Obj ffff940f6ba36c60 Type 02
Refs 00 [Decremented]
utdelete-0096 ut_delete_internal_obj: **** String ffff940f6ba36c60,
ptr ffff9f654004e2b2
utdelete-0336 ut_delete_internal_obj: Deleting Object ffff940f6ba36c60 [String]
nseval-0315 ns_evaluate : *** Completed evaluation of
object _CID ***
Thanks,
Daniel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-02-06 15:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-03 20:56 [PATCH] acpica: Fix double-free in acpi_ns_repair_CID() João Paulo Rechi Vita
2017-02-04 4:19 ` Moore, Robert
2017-02-06 1:44 ` Zheng, Lv
[not found] ` <CA+A7VXXNk58unA+k6743VV7=cHTTbW8YHVf_5GAYdv1EPA5dPw@mail.gmail.com>
2017-02-06 15:06 ` Daniel Drake
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).