Bug 143432 - [acpi] [patch] Two bugs in acpica in AcpiExReleaseMutex
Summary: [acpi] [patch] Two bugs in acpica in AcpiExReleaseMutex
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: Andriy Gapon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-01 15:40 UTC by Vladislav Shabanov
Modified: 2010-03-09 07:15 UTC (History)
0 users

See Also:


Attachments
file.diff (1.86 KB, patch)
2010-02-01 15:40 UTC, Vladislav Shabanov
no flags Details | Diff
mutex.patch (2.68 KB, patch)
2010-02-02 02:40 UTC, ming.m.lin
no flags Details | Diff
file.dat (212 bytes, text/plain; charset="us-ascii")
2010-02-02 02:40 UTC, ming.m.lin
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vladislav Shabanov 2010-02-01 15:40:03 UTC
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?

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:
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2010-02-02 00:07:42 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-acpi

Over to maintainer(s).
Comment 2 ming.m.lin 2010-02-02 02:40:23 UTC
Problem 1 was already fixed in release 20091214, see 
http://git.moblin.org/cgit.cgi/acpica/commit/?id=93324dd734d70c4aa451c3aa24dfe91b7b8ef7f9

Problem 2 is same with http://www.freebsd.org/cgi/query-pr.cgi?pr=140979
and fixed by the attached patch. We will merge it into next release.

Lin Ming

> -----Original Message-----
> From: owner-freebsd-acpi@freebsd.org [mailto:owner-freebsd-acpi@freebsd.org] On Behalf Of linimon@FreeBSD.org
> Sent: Monday, February 01, 2010 4:08 PM
> To: linimon@FreeBSD.org; freebsd-bugs@FreeBSD.org; freebsd-acpi@FreeBSD.org
> Subject: Re: kern/143432: [acpi] [patch] Two bugs in acpica in AcpiExReleaseMutex
> 
> Old Synopsis: Two bugs in acpica in AcpiExReleaseMutex
> New Synopsis: [acpi] [patch] Two bugs in acpica in AcpiExReleaseMutex
> 
> Responsible-Changed-From-To: freebsd-bugs->freebsd-acpi
> Responsible-Changed-By: linimon
> Responsible-Changed-When: Tue Feb 2 00:07:42 UTC 2010
> Responsible-Changed-Why: 
> Over to maintainer(s).
> 
> http://www.freebsd.org/cgi/query-pr.cgi?pr=143432
> _______________________________________________
> freebsd-acpi@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
> To unsubscribe, send any mail to "freebsd-acpi-unsubscribe@freebsd.org"
Comment 3 Andriy Gapon freebsd_committer freebsd_triage 2010-02-02 11:49:10 UTC
Responsible Changed
From-To: freebsd-acpi->avg

I will take this as I've been working on these issues.
Comment 4 Andriy Gapon freebsd_committer freebsd_triage 2010-02-02 14:25:38 UTC
State Changed
From-To: open->patched

The first issue is already fixed in head by vendor import 
of ACPICA 20091214 (presently at 20100121). 
The second issue is a duplicate of kern/140979 and will 
not be tracked here.
Comment 5 Andriy Gapon freebsd_committer freebsd_triage 2010-02-10 18:07:08 UTC
Vlad,

could you please test the patches provided by Lin Ming and let us know what you
get with them?

-- 
Andriy Gapon
Comment 6 Bruce Cran freebsd_committer freebsd_triage 2010-03-09 07:14:47 UTC
State Changed
From-To: patched->closed

ACPICA 20100121 has been merged to stable/8.