ACPI panic

[Andriy Gapon]

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