From owner-freebsd-acpi@FreeBSD.ORG Fri Jan 25 08:22:34 2013 Return-Path: Delivered-To: freebsd-acpi@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 863F696C; Fri, 25 Jan 2013 08:22:34 +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 A043D6B3; Fri, 25 Jan 2013 08:22:33 +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 KAA29490; Fri, 25 Jan 2013 10:22:26 +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 1TyeYL-000F3y-Od; Fri, 25 Jan 2013 10:22:25 +0200 Message-ID: <510240BF.9070301@FreeBSD.org> Date: Fri, 25 Jan 2013 10:22:23 +0200 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130121 Thunderbird/17.0.2 MIME-Version: 1.0 To: Jung-uk Kim Subject: Re: uma for acpi object cache References: <20130122175629.GA1714@garage.freebsd.pl> <51008661.4060006@FreeBSD.org> <510101B4.4030409@FreeBSD.org> <51017D79.6060202@FreeBSD.org> <51018223.4030702@FreeBSD.org> <51019AAE.10501@FreeBSD.org> In-Reply-To: <51019AAE.10501@FreeBSD.org> 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: Fri, 25 Jan 2013 08:22:34 -0000 on 24/01/2013 22:33 Jung-uk Kim said the following: > If I am not completely mistaken, this is what's happening: > > https://github.com/otcshare/acpica/pull/3 > > Please see ACPI_OBJECT_COMMON_HEADER macro change in the commit I > mentioned the pull request. > > Before the commit: > UINT8 Descriptor; > UINT8 Type; > UINT16 ReferenceCount; > union acpi_operand_object *NextObject; > UINT8 Flags; > > After the commit: > union acpi_operand_object *NextObject; > UINT8 DescriptorType; > UINT8 Type; > UINT16 ReferenceCount; > UINT8 Flags; > > It may not look obvious but LinkOffset was used to keep offset of > NextObject (and it was only "safe" for certain compilers & platforms). > > Alas, it is completely bogus now because the pointer became the first > element of any object. Although it was the right decision, the author > forgot to change the LinkOffset with this commit, I guess. Now, > modifying DescriptorType, Type, ReferenceCount, or Flags silently > corrupts the linked list. Similarly, modifying any of these before > setting the pointer gets silently clobbered. For example, > ACPI_SET_DESCRIPTOR_TYPE() in AcpiOsReleaseObject() is effectively > no-op because it's overwritten later. > > Does it make sense to you? Hmm, not sure... Are you trying to improve behavior in use-after-free scenario or some such abnormal situation? My understanding is this. An object is either in use or it's free/cached. When it is in use, then LinkOffset has nothing to do with object (cache linking is not applied to the object). When it is cached, then it is not accessed as the real object (via ACPI_OBJECT_COMMON_HEADER fields), it is accessed only as a cached entity via the LinkOffset hackery. So, I do not see any bug in the current code. Your change makes it look a little bit less ugly, though. But I do not see any functional change. -- Andriy Gapon