Date: Thu, 25 Jul 2013 08:05:25 +0000 (UTC) From: Andriy Gapon <avg@FreeBSD.org> 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 Message-ID: <201307250805.r6P85PZn031085@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
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 <demelier.david@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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201307250805.r6P85PZn031085>