Re: [issue568] ACPI(?) double-free on shutdown (More K9AGM fun)

[YONETANI Tomokazu <qhwt+dfly@xxxxxxxxxx>]

On Thu, Mar 08, 2007 at 02:31:51AM -0000, Joe Floid Kanowitz wrote:
> 
> Joe "Floid" Kanowitz <jkanowitz@snet.net> added the comment:
> 
> Looks like the patch was a winner; what you've won, I'm not experienced (or
> awake) enough to figure out.
> 
> http://bugs.dragonflybsd.org/file213/Patch_Panic_1.png and
> http://bugs.dragonflybsd.org/file214/Patch_Panic_2.png ,

Can you try attached patch?  AcpiUtUpdateObjectReference() can leave
Object->CommonNotify.SystemNotify or Object->CommonNotify.DeviceNotify
non-NULL after they are (eventually) released by AcpiUtDeleteObjectDesc().
AcpiUtDeleteObjectDesc() can leak the object if it's not of type
ACPI_OPERAND_OBJECT, so it also may have to be converted to accept
(ACPI_OPERAND_OBJECT **).

Cheers.

Index: Makefile
===================================================================
RCS file: /home/source/dragonfly/cvs/src/sys/dev/acpica5/Makefile,v
retrieving revision 1.19
diff -u -p -r1.19 Makefile
--- Makefile	25 Jan 2007 15:12:06 -0000	1.19
+++ Makefile	8 Mar 2007 22:03:59 -0000
@@ -3,7 +3,8 @@
 
 CONTRIBDIR=	${SYSDIR}/${ACPICA_DIR}
 # patches to fix problems in ACPI-CA code
-PATCHES=	hardware,hwsleep.c.patch tables,tbxface.c.patch
+PATCHES=	hardware,hwsleep.c.patch tables,tbxface.c.patch \
+		utilities,utdelete.c.patch
 # patches to silence warnings
 PATCHES+=	include,acglobal.h.patch debugger,dbstats.c.patch
 
Index: utilities,utdelete.c.patch
===================================================================
RCS file: utilities,utdelete.c.patch
diff -N utilities,utdelete.c.patch
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ utilities,utdelete.c.patch	8 Mar 2007 22:35:13 -0000
@@ -0,0 +1,127 @@
+# $DragonFly$
+
+--- utilities/utdelete.c	17 Jan 2007 16:29:45 -0000	1.1.1.1
++++ utilities/utdelete.c	8 Mar 2007 22:34:44 -0000
+@@ -129,11 +129,11 @@
+ 
+ static void
+ AcpiUtDeleteInternalObj (
+-    ACPI_OPERAND_OBJECT     *Object);
++    ACPI_OPERAND_OBJECT     **ObjectPtr);
+ 
+ static void
+ AcpiUtUpdateRefCount (
+-    ACPI_OPERAND_OBJECT     *Object,
++    ACPI_OPERAND_OBJECT     **ObjectPtr,
+     UINT32                  Action);
+ 
+ 
+@@ -141,7 +141,7 @@ AcpiUtUpdateRefCount (
+  *
+  * FUNCTION:    AcpiUtDeleteInternalObj
+  *
+- * PARAMETERS:  Object         - Object to be deleted
++ * PARAMETERS:  ObjectPtr      - pointer to Object to be deleted
+  *
+  * RETURN:      None
+  *
+@@ -152,18 +152,19 @@ AcpiUtUpdateRefCount (
+ 
+ static void
+ AcpiUtDeleteInternalObj (
+-    ACPI_OPERAND_OBJECT     *Object)
++    ACPI_OPERAND_OBJECT     **ObjectPtr)
+ {
+     void                    *ObjPointer = NULL;
+     ACPI_OPERAND_OBJECT     *HandlerDesc;
+     ACPI_OPERAND_OBJECT     *SecondDesc;
+     ACPI_OPERAND_OBJECT     *NextDesc;
++    ACPI_OPERAND_OBJECT     *Object;
+ 
+ 
+-    ACPI_FUNCTION_TRACE_PTR (UtDeleteInternalObj, Object);
++    ACPI_FUNCTION_TRACE_PTR (UtDeleteInternalObj, ObjectPtr);
+ 
+ 
+-    if (!Object)
++    if (!ObjectPtr || !(Object = *ObjectPtr))
+     {
+         return_VOID;
+     }
+@@ -364,6 +365,7 @@ AcpiUtDeleteInternalObj (
+         Object, AcpiUtGetObjectTypeName (Object)));
+ 
+     AcpiUtDeleteObjectDesc (Object);
++    *ObjectPtr = NULL;
+     return_VOID;
+ }
+ 
+@@ -409,7 +411,8 @@ AcpiUtDeleteInternalObjectList (
+  *
+  * FUNCTION:    AcpiUtUpdateRefCount
+  *
+- * PARAMETERS:  Object          - Object whose ref count is to be updated
++ * PARAMETERS:  ObjectPtr       - Pointer to Object whose ref count is to be
++ *                                updated
+  *              Action          - What to do
+  *
+  * RETURN:      New ref count
+@@ -420,9 +423,10 @@ AcpiUtDeleteInternalObjectList (
+ 
+ static void
+ AcpiUtUpdateRefCount (
+-    ACPI_OPERAND_OBJECT     *Object,
++    ACPI_OPERAND_OBJECT     **ObjectPtr,
+     UINT32                  Action)
+ {
++    ACPI_OPERAND_OBJECT     *Object;
+     UINT16                  Count;
+     UINT16                  NewCount;
+ 
+@@ -430,7 +434,7 @@ AcpiUtUpdateRefCount (
+     ACPI_FUNCTION_NAME (UtUpdateRefCount);
+ 
+ 
+-    if (!Object)
++    if (!ObjectPtr || !(Object = *ObjectPtr))
+     {
+         return;
+     }
+@@ -481,7 +485,7 @@ AcpiUtUpdateRefCount (
+         Object->Common.ReferenceCount = NewCount;
+         if (NewCount == 0)
+         {
+-            AcpiUtDeleteInternalObj (Object);
++            AcpiUtDeleteInternalObj (&Object);
+         }
+         break;
+ 
+@@ -492,7 +496,7 @@ AcpiUtUpdateRefCount (
+ 
+         NewCount = 0;
+         Object->Common.ReferenceCount = NewCount;
+-        AcpiUtDeleteInternalObj (Object);
++        AcpiUtDeleteInternalObj (&Object);
+         break;
+ 
+     default:
+@@ -574,8 +578,8 @@ AcpiUtUpdateObjectReference (
+ 
+             /* Update the notify objects for these types (if present) */
+ 
+-            AcpiUtUpdateRefCount (Object->CommonNotify.SystemNotify, Action);
+-            AcpiUtUpdateRefCount (Object->CommonNotify.DeviceNotify, Action);
++            AcpiUtUpdateRefCount (&Object->CommonNotify.SystemNotify, Action);
++            AcpiUtUpdateRefCount (&Object->CommonNotify.DeviceNotify, Action);
+             break;
+ 
+         case ACPI_TYPE_PACKAGE:
+@@ -652,7 +656,7 @@ AcpiUtUpdateObjectReference (
+          * happen after we update the sub-objects in case this causes the
+          * main object to be deleted.
+          */
+-        AcpiUtUpdateRefCount (Object, Action);
++        AcpiUtUpdateRefCount (&Object, Action);
+         Object = NULL;
+ 
+         /* Move on to the next object to be updated */