svn commit: r253642 - stable/9/sys/dev/acpica

[Andriy Gapon]

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 at gmail.com,
  		David Demelier <demelier.david at gmail.com>
  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);
 }