From owner-freebsd-acpi@FreeBSD.ORG Thu Nov 29 21:43:55 2012 Return-Path: Delivered-To: freebsd-acpi@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 8C6939A4; Thu, 29 Nov 2012 21:43:55 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 811848FC0C; Thu, 29 Nov 2012 21:43:53 +0000 (UTC) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id XAA14483; Thu, 29 Nov 2012 23:43:52 +0200 (EET) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1TeBtg-0001jA-KX; Thu, 29 Nov 2012 23:43:52 +0200 Message-ID: <50B7D717.4030402@FreeBSD.org> Date: Thu, 29 Nov 2012 23:43:51 +0200 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Stefan Farfeleder Subject: Re: ACPI panic 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> In-Reply-To: <20121129084627.GA1450@mole.fafoe.narf.at> X-Enigmail-Version: 1.4.6 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: freebsd-acpi@FreeBSD.org X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Nov 2012 21:43:55 -0000 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