Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

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 <demelier.david@gmail.com>
Approved by:	re (kib)
  • Loading branch information...
commit fc07a03dd7725402013830e4de4644dcde753110 1 parent 83f1a02
@avg-I avg-I authored
Showing with 13 additions and 0 deletions.
  1. +13 −0 sys/dev/acpica/acpi_battery.c
View
13 sys/dev/acpica/acpi_battery.c
@@ -360,6 +360,18 @@ acpi_battery_ioctl(u_long cmd, caddr_t addr, void *arg)
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 addr, void *arg)
error = EINVAL;
}
+ mtx_unlock(&Giant);
return (error);
}
Please sign in to comment.
Something went wrong with that request. Please try again.