diff options
author | Hans de Goede <hdegoede@redhat.com> | 2021-02-18 15:17:07 -0800 |
---|---|---|
committer | Rafael J. Wysocki <rafael.j.wysocki@intel.com> | 2021-02-19 16:03:25 +0100 |
commit | c27f3d011b08540e68233cf56274fdc34bebb9b5 (patch) | |
tree | 47798aebe1d925d6ed6259872da5dffaf9350306 /drivers/acpi/acpica/evhandler.c | |
parent | 938bdd1d7dad75299201d1cc47c1dcf4d49f9274 (diff) |
ACPICA: Fix race in generic_serial_bus (I2C) and GPIO op_region parameter handling
ACPICA commit c9e0116952363b0fa815143dca7e9a2eb4fefa61
The handling of the generic_serial_bus (I2C) and GPIO op_regions in
acpi_ev_address_space_dispatch() passes a number of extra parameters
to the address-space handler through the address-space Context pointer
(instead of using more function parameters).
The Context is shared between threads, so if multiple threads try to
call the handler for the same address-space at the same time, then
a second thread could change the parameters of a first thread while
the handler is running for the first thread.
An example of this race hitting is the Lenovo Yoga Tablet2 1015L,
where there are both attrib_bytes accesses and attrib_byte accesses
to the same address-space. The attrib_bytes access stores the number
of bytes to transfer in Context->access_length. Where as for the
attrib_byte access the number of bytes to transfer is always 1 and
field_obj->Field.access_length is unused (so 0). Both types of
accesses racing from different threads leads to the following problem:
1. Thread a. starts an attrib_bytes access, stores a non 0 value
from field_obj->Field.access_length in Context->access_length
2. Thread b. starts an attrib_byte access, stores 0 in
Context->access_length
3. Thread a. calls i2c_acpi_space_handler() (under Linux). Which
sees that the access-type is ACPI_GSB_ACCESS_ATTRIB_MULTIBYTE
and calls acpi_gsb_i2c_read_bytes(..., Context->access_length)
4. At this point Context->access_length is 0 (set by thread b.)
rather then the field_obj->Field.access_length value from thread a.
This 0 length reads leads to the following errors being logged:
i2c i2c-0: adapter quirk: no zero length (addr 0x0078, size 0, read)
i2c i2c-0: i2c read 0 bytes from client@0x78 starting at reg 0x0 failed, error: -95
Note this is just an example of the problems which this race can cause.
There are likely many more (sporadic) problems caused by this race.
This commit adds a new context_mutex to struct acpi_object_addr_handler
and makes acpi_ev_address_space_dispatch() take that mutex when
using the shared Context to pass extra parameters to an address-space
handler, fixing this race.
Note the new mutex must be taken *after* exiting the interpreter,
therefor the existing acpi_ex_exit_interpreter() call is moved to above
the code which stores the extra parameters in the Context.
Link: https://github.com/acpica/acpica/commit/c9e01169
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
Signed-off-by: Erik Kaneda <erik.kaneda@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Diffstat (limited to 'drivers/acpi/acpica/evhandler.c')
-rw-r--r-- | drivers/acpi/acpica/evhandler.c | 7 |
1 files changed, 7 insertions, 0 deletions
diff --git a/drivers/acpi/acpica/evhandler.c b/drivers/acpi/acpica/evhandler.c index ea9485e6a475..c0cd7147a5a3 100644 --- a/drivers/acpi/acpica/evhandler.c +++ b/drivers/acpi/acpica/evhandler.c @@ -489,6 +489,13 @@ acpi_ev_install_space_handler(struct acpi_namespace_node *node, /* Init handler obj */ + status = + acpi_os_create_mutex(&handler_obj->address_space.context_mutex); + if (ACPI_FAILURE(status)) { + acpi_ut_remove_reference(handler_obj); + goto unlock_and_exit; + } + handler_obj->address_space.space_id = (u8)space_id; handler_obj->address_space.handler_flags = flags; handler_obj->address_space.region_list = NULL; |