Date: Thu, 29 Nov 2012 23:43:51 +0200 From: Andriy Gapon <avg@FreeBSD.org> To: Stefan Farfeleder <stefanf@FreeBSD.org> Cc: freebsd-acpi@FreeBSD.org Subject: Re: ACPI panic Message-ID: <50B7D717.4030402@FreeBSD.org> In-Reply-To: <20121129084627.GA1450@mole.fafoe.narf.at> References: <50ADFFB2.1000108@FreeBSD.org> <50AE057D.8060808@FreeBSD.org> <20121125140008.GA1497@mole.fafoe.narf.at> <50B244A1.1040800@FreeBSD.org> <20121126091101.GA1469@mole.fafoe.narf.at> <50B33693.2060000@FreeBSD.org> <20121126093704.GB1469@mole.fafoe.narf.at> <50B34484.1090807@FreeBSD.org> <20121126104737.GC1469@mole.fafoe.narf.at> <50B34EEA.4000209@FreeBSD.org> <20121129084627.GA1450@mole.fafoe.narf.at>
next in thread | previous in thread | raw e-mail | index | archive | help
on 29/11/2012 10:46 Stefan Farfeleder said the following:
> On Mon, Nov 26, 2012 at 01:13:46PM +0200, Andriy Gapon wrote:
>>
>> Also, I've just realized that the check is racy...
>> Could you please move the whole check block (between and excluding
>> AcpiUtAcquireMutex and AcpiUtReleaseMutex) down right below the following lines:
>>
>> Status = AcpiUtAcquireMutex (ACPI_MTX_CACHES);
>> if (ACPI_FAILURE (Status))
>> {
>> return (Status);
>> }
>
> Sorry for the delay. I'm now running the patch below. I still got the
> cycle panic, this time with a 4-objects cycle. It looks like an object
> gets released twice but I don't understand why the "freeing a free
> object" check fails to trigger.
Hmmm...
Another bug-catching patch before I start questioning my ability to understand
the code.
index 59ecf21..1687c75b 100644
--- a/sys/contrib/dev/acpica/components/utilities/utcache.c
+++ b/sys/contrib/dev/acpica/components/utilities/utcache.c
@@ -238,6 +238,8 @@ AcpiOsReleaseObject (
else
{
+ if (AcpiGbl_MutexInfo[ACPI_MTX_CACHES].ThreadId == AcpiOsGetThreadId ())
+ panic("ACPI_MTX_CACHES acquired recursively");
Status = AcpiUtAcquireMutex (ACPI_MTX_CACHES);
if (ACPI_FAILURE (Status))
{
@@ -311,6 +313,8 @@ AcpiOsAcquireObject (
return (NULL);
}
+ if (AcpiGbl_MutexInfo[ACPI_MTX_CACHES].ThreadId == AcpiOsGetThreadId ())
+ panic("ACPI_MTX_CACHES acquired recursively");
Status = AcpiUtAcquireMutex (ACPI_MTX_CACHES);
if (ACPI_FAILURE (Status))
{
> Stefan
>
> Index: components/utilities/utcache.c
> ===================================================================
> --- components/utilities/utcache.c (revision 243234)
> +++ components/utilities/utcache.c (working copy)
> @@ -244,6 +244,28 @@
> return (Status);
> }
>
> + char *Curr;
> + char *Next;
> + int Depth;
> + Depth = Cache->CurrentDepth;
> + Next = Cache->ListHead;
> + while (Next)
> + {
> + Curr = Next;
> + Next = *(ACPI_CAST_INDIRECT_PTR (char,
> + &(((char *) Curr)[Cache->LinkOffset])));
> + if (*(const unsigned char *) Curr != 0xCA) {
> + panic("detected use after free %p\n", Curr);
> + }
> + if (Object == Curr) {
> + panic("freeing a free object %p", Object);
> + }
> + Depth--;
> + if (Depth < 0) {
> + panic("cycle in a cache list");
> + }
> + }
> +
> /* Mark the object as cached */
>
> ACPI_MEMSET (Object, 0xCA, Cache->ObjectSize);
> @@ -312,6 +334,10 @@
>
> Cache->CurrentDepth--;
>
> + if (*(const unsigned char *) Object != 0xCA) {
> + panic("detected use after free %p\n", Object);
> + }
> +
> ACPI_MEM_TRACKING (Cache->Hits++);
> ACPI_DEBUG_PRINT ((ACPI_DB_EXEC,
> "Object %p from %s cache\n", Object, Cache->ListName));
>
Just in case: this is exactly what I had in mind.
--
Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50B7D717.4030402>