From owner-svn-src-all@FreeBSD.ORG Thu Jul 25 08:05:26 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 0EAE7687; Thu, 25 Jul 2013 08:05:26 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id D581B23E9; Thu, 25 Jul 2013 08:05:25 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.7/8.14.7) with ESMTP id r6P85P0G031086; Thu, 25 Jul 2013 08:05:25 GMT (envelope-from avg@svn.freebsd.org) Received: (from avg@localhost) by svn.freebsd.org (8.14.7/8.14.5/Submit) id r6P85PZn031085; Thu, 25 Jul 2013 08:05:25 GMT (envelope-from avg@svn.freebsd.org) Message-Id: <201307250805.r6P85PZn031085@svn.freebsd.org> From: Andriy Gapon Date: Thu, 25 Jul 2013 08:05:25 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r253642 - stable/9/sys/dev/acpica X-SVN-Group: stable-9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Jul 2013 08:05:26 -0000 Author: avg Date: Thu Jul 25 08:05:25 2013 New Revision: 253642 URL: http://svnweb.freebsd.org/changeset/base/253642 Log: protect acpi_battery_ioctl with Giant This is a direct commit to stable/9. There is a bug in the ACPICA version 20110527 that is used in stable/9. The bug can lead to unprotected reference counting on ACPI objects and eventually to a crash or a memory corruption. The bug has been fixed upstream and imported to head as of ACPICA version 20130328. Unfortunately, ACPICA version in stable has not been updated, so merging all past ACPICA versions or cherry-picking parts of 20130328 would be a big change with a risk of potential regressions. During debugging it was determined that the most probable vector for the bug was through concurrent calls to ACPI battery sysctls and ioctls. The sysctls are already guarded by Giant (not MPSAFE), but ioctls could execute in parallel to a sysctl call and to each other. All the calls go through acpi_battery_ioctl function, which makes the actual calls into ACPICA and those are the calls that lack necessary protection. Thus preventing concurrency in acpi_battery_ioctl should prevent the conditions that triggered the ACPICA bug. Some additional details can be found in this thread: http://thread.gmane.org/gmane.os.freebsd.devel.acpi/7707/focus=7774 Tested by: kron24@gmail.com, David Demelier Approved by: re (kib) Modified: stable/9/sys/dev/acpica/acpi_battery.c Modified: stable/9/sys/dev/acpica/acpi_battery.c ============================================================================== --- stable/9/sys/dev/acpica/acpi_battery.c Thu Jul 25 08:03:03 2013 (r253641) +++ stable/9/sys/dev/acpica/acpi_battery.c Thu Jul 25 08:05:25 2013 (r253642) @@ -360,6 +360,18 @@ acpi_battery_ioctl(u_long cmd, caddr_t a int error, unit; device_t dev; + + /* + * Giant is acquired to work around a reference counting bug in ACPICA + * versions prior to 20130328. If not for that bug this function could + * be executed concurrently without any problems. + * The bug is in acpi_BatteryIsPresent -> AcpiGetObjectInfo call tree, + * where AcpiUtExecute_HID, AcpiUtExecute_UID, etc are executed without + * protection of any ACPICA lock and may concurrently call + * AcpiUtRemoveReference on a battery object. + */ + mtx_lock(&Giant); + /* For commands that use the ioctl_arg struct, validate it first. */ error = ENXIO; unit = 0; @@ -417,6 +429,7 @@ acpi_battery_ioctl(u_long cmd, caddr_t a error = EINVAL; } + mtx_unlock(&Giant); return (error); }