misc/143432: Two bugs in acpica in AcpiExReleaseMutex

[Vladislav Shabanov]

>Number:         143432
>Category:       misc
>Synopsis:       Two bugs in acpica in AcpiExReleaseMutex
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Mon Feb 01 15:40:03 UTC 2010
>Closed-Date:
>Last-Modified:
>Originator:     Vladislav Shabanov
>Release:        Freebsd-8-stable
>Organization:
Mail.ru
>Environment:
FreeBSD vsnotebook 8.0-STABLE FreeBSD 8.0-STABLE #15: Mon Feb  1 16:24:33 MSK 2010     root at vsNotebook:/usr/src/sys/amd64/compile/VS-FERRARI  amd64
>Description:
>From time to time computer crashed during boot inside AcpiExReleaseMutex.
When I turned on ACPI_DEBUG, it crashes always.

Probjem 1:
line 

if ((ObjDesc->Mutex.OwnerThread->ThreadId != WalkState->Thread->ThreadId) &&
        (ObjDesc != AcpiGbl_GlobalLockMutex))

goes earlier than
if (!WalkState->Thread)

if WalkState->Thread is NULL, it crashed before check. In my case it was not happened, in my case crash was because problem 2.

Problem 2:
my computer crashed when WalkState->Thread->AcquiredMutexList was NULL. 
May be this is because Mutex was acquired in one thread and released in another?


>How-To-Repeat:

>Fix:
--- exmutex.c-ORIG      2010-02-01 15:22:05.000000000 +0300                                         
+++ exmutex.c   2010-02-01 16:16:43.000000000 +0300                                                 
@@ -490,6 +490,15 @@                                                                                
         return_ACPI_STATUS (AE_AML_MUTEX_NOT_ACQUIRED);                                            
     }                                                                                              
                                                                                                    
+    /* Must have a valid thread ID */                                                              
+                                                                                                   
+    if (!WalkState->Thread)                                                                        
+    {                                                                                              
+        ACPI_ERROR ((AE_INFO, "Cannot release Mutex [%4.4s], null thread info",
+            AcpiUtGetNodeName (ObjDesc->Mutex.Node)));
+        return_ACPI_STATUS (AE_AML_INTERNAL);
+    }
+
     /*
      * The Mutex is owned, but this thread must be the owner.
      * Special case for Global Lock, any thread can release
@@ -505,15 +514,6 @@
         return_ACPI_STATUS (AE_AML_NOT_OWNER);
     }

-    /* Must have a valid thread ID */
-
-    if (!WalkState->Thread)
-    {
-        ACPI_ERROR ((AE_INFO, "Cannot release Mutex [%4.4s], null thread info",
-            AcpiUtGetNodeName (ObjDesc->Mutex.Node)));
-        return_ACPI_STATUS (AE_AML_INTERNAL);
-    }
-
     /*
      * The sync level of the mutex must be equal to the current sync level. In
      * other words, the current level means that at least one mutex at that
@@ -535,8 +535,16 @@
      * This handles the case where several mutexes at the same level have been
      * acquired, but are not released in reverse order.
      */
-    PreviousSyncLevel =
-        WalkState->Thread->AcquiredMutexList->Mutex.OriginalSyncLevel;
+    if (!WalkState->Thread->AcquiredMutexList)
+    {
+        ACPI_ERROR ((AE_INFO, "Thread AcquiredMutexList empty while releasing mutex Mutex [%4.4s]",
+            AcpiUtGetNodeName (ObjDesc->Mutex.Node)));
+        /* return_ACPI_STATUS (AE_AML_INTERNAL); */
+        PreviousSyncLevel = WalkState->Thread->CurrentSyncLevel;
+    }
+    else
+        PreviousSyncLevel =
+            WalkState->Thread->AcquiredMutexList->Mutex.OriginalSyncLevel;

     Status = AcpiExReleaseMutexObject (ObjDesc);
     if (ACPI_FAILURE (Status))


Patch attached with submission follows:

--- exmutex.c-ORIG	2010-02-01 15:22:05.000000000 +0300
+++ exmutex.c	2010-02-01 16:16:43.000000000 +0300
@@ -490,6 +490,15 @@
         return_ACPI_STATUS (AE_AML_MUTEX_NOT_ACQUIRED);
     }
 
+    /* Must have a valid thread ID */
+
+    if (!WalkState->Thread)
+    {
+        ACPI_ERROR ((AE_INFO, "Cannot release Mutex [%4.4s], null thread info",
+            AcpiUtGetNodeName (ObjDesc->Mutex.Node)));
+        return_ACPI_STATUS (AE_AML_INTERNAL);
+    }
+
     /*
      * The Mutex is owned, but this thread must be the owner.
      * Special case for Global Lock, any thread can release
@@ -505,15 +514,6 @@
         return_ACPI_STATUS (AE_AML_NOT_OWNER);
     }
 
-    /* Must have a valid thread ID */
-
-    if (!WalkState->Thread)
-    {
-        ACPI_ERROR ((AE_INFO, "Cannot release Mutex [%4.4s], null thread info",
-            AcpiUtGetNodeName (ObjDesc->Mutex.Node)));
-        return_ACPI_STATUS (AE_AML_INTERNAL);
-    }
-
     /*
      * The sync level of the mutex must be equal to the current sync level. In
      * other words, the current level means that at least one mutex at that
@@ -535,8 +535,16 @@
      * This handles the case where several mutexes at the same level have been
      * acquired, but are not released in reverse order.
      */
-    PreviousSyncLevel =
-        WalkState->Thread->AcquiredMutexList->Mutex.OriginalSyncLevel;
+    if (!WalkState->Thread->AcquiredMutexList)
+    {
+        ACPI_ERROR ((AE_INFO, "Thread AcquiredMutexList empty while releasing mutex Mutex [%4.4s]",
+            AcpiUtGetNodeName (ObjDesc->Mutex.Node)));
+        /* return_ACPI_STATUS (AE_AML_INTERNAL); */
+        PreviousSyncLevel = WalkState->Thread->CurrentSyncLevel;
+    }
+    else
+        PreviousSyncLevel =
+            WalkState->Thread->AcquiredMutexList->Mutex.OriginalSyncLevel;
 
     Status = AcpiExReleaseMutexObject (ObjDesc);
     if (ACPI_FAILURE (Status))


>Release-Note:
>Audit-Trail:
>Unformatted: