Commit ae6b3e54 authored by Hans de Goede's avatar Hans de Goede Committed by Rafael J. Wysocki

ACPICA: Fix handling of buffer-size in acpi_ex_write_data_to_field()

Generic Serial Bus transfers use a data struct like this:

struct gsb_buffer {
        u8      status;
        u8      len;
        u8      data[0];

acpi_ex_write_data_to_field() copies the data which is to be written from
the source-buffer to a temp-buffer. This is done because the OpReg-handler
overwrites the status field and some transfers do a write + read-back.

Commit f99b89ee ("ACPICA: Update for generic_serial_bus and
attrib_raw_process_bytes protocol") acpi_ex_write_data_to_field()
introduces a number of problems with this:

 1) It drops a "length += 2" statement used to calculate the temp-buffer
 size causing the temp-buffer to only be 1/2 bytes large for byte/word
 transfers while it should be 3/4 bytes (taking the status and len field
 into account). This is already fixed in commit e324e101 ("ACPICA:
 Update for field unit access") which refactors the code.

The ACPI 6.0 spec (ACPI_6.0.pdf) " Declaring and Using a
GenericSerialBusData Buffer" (page 232) states that the GenericSerialBus
Data Buffer Length field is only valid when doing a Read/Write Block
(AttribBlock) transfer, but since the troublesome commit we unconditionally
use the len field to determine how much data to copy from the source-buffer
into the temp-buffer passed to the OpRegion.

This causes 3 further issues:

 2) This may lead to not copying enough data to the temp-buffer causing the
 OpRegion handler for the serial-bus to write garbage to the hardware.

 3) The temp-buffer passed to the OpRegion is allocated to the size
 returned by acpi_ex_get_serial_access_length(), which may be as little
 as 1, so potentially this may lead to a write overflow of the temp-buffer.

 4) Commit e324e101 ("ACPICA: Update for field unit access") drops a
 length check on the source-buffer, leading to a potential read overflow
 of the source-buffer.

This commit fixes all 3 remaining issues by not looking at the len field at
all (the interpretation of this field is left up to the OpRegion handler),
and copying the minimum of the source- and temp-buffer sizes from the
source-buffer to the temp-buffer.

This fixes e.g. an Acer S1003 no longer booting since the troublesome

Fixes: f99b89ee (ACPICA: Update for generic_serial_bus and ...)
Fixes: e324e101 (ACPICA: Update for field unit access)
Signed-off-by: default avatarHans de Goede <>
Signed-off-by: default avatarRafael J. Wysocki <>
parent 9ff01193
......@@ -244,7 +244,6 @@ acpi_ex_write_serial_bus(union acpi_operand_object *source_desc,
acpi_status status;
u32 buffer_length;
u32 data_length;
void *buffer;
union acpi_operand_object *buffer_desc;
u32 function;
......@@ -282,14 +281,12 @@ acpi_ex_write_serial_bus(union acpi_operand_object *source_desc,
buffer_length = ACPI_SMBUS_BUFFER_SIZE;
data_length = ACPI_SMBUS_DATA_SIZE;
function = ACPI_WRITE | (obj_desc->field.attribute << 16);
buffer_length = ACPI_IPMI_BUFFER_SIZE;
data_length = ACPI_IPMI_DATA_SIZE;
function = ACPI_WRITE;
......@@ -310,7 +307,6 @@ acpi_ex_write_serial_bus(union acpi_operand_object *source_desc,
/* Add header length to get the full size of the buffer */
buffer_length += ACPI_SERIAL_HEADER_SIZE;
data_length = source_desc->buffer.pointer[1];
function = ACPI_WRITE | (accessor_type << 16);
......@@ -318,20 +314,6 @@ acpi_ex_write_serial_bus(union acpi_operand_object *source_desc,
#if 0
/* Check for possible buffer overflow */
if (data_length > source_desc->buffer.length) {
"Length in buffer header (%u)(%u) is greater than "
"the physical buffer length (%u) and will overflow",
data_length, buffer_length,
/* Create the transfer/bidirectional/return buffer */
buffer_desc = acpi_ut_create_buffer_object(buffer_length);
......@@ -342,7 +324,8 @@ acpi_ex_write_serial_bus(union acpi_operand_object *source_desc,
/* Copy the input buffer data to the transfer buffer */
buffer = buffer_desc->buffer.pointer;
memcpy(buffer, source_desc->buffer.pointer, data_length);
memcpy(buffer, source_desc->buffer.pointer,
min(buffer_length, source_desc->buffer.length));
/* Lock entire transaction if requested */
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment