diff mbox

[1/9] ACPICA: Executer: Fix buffer allocation issue for generic_serial_bus region field accesses.

Message ID 1397654682-7094-2-git-send-email-tianyu.lan@intel.com
State Superseded
Headers show

Commit Message

Lan Tianyu April 16, 2014, 1:24 p.m. UTC
From: Lv Zheng <lv.zheng@intel.com>

The size of the buffer allocated for generic_serial_bus region access
is not correct.  This patch introduces acpi_ex_get_serial_access_length()
to be invoked to obtain correct data buffer length.  Reported by
Lan Tianyu, Fixed by Lv Zheng.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/acpi/acpica/exfield.c | 104 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 97 insertions(+), 7 deletions(-)

Comments

Rafael J. Wysocki April 21, 2014, 9:38 p.m. UTC | #1
Hi,

On Wednesday, April 16, 2014 09:24:34 PM Lan Tianyu wrote:
> From: Lv Zheng <lv.zheng@intel.com>
> 
> The size of the buffer allocated for generic_serial_bus region access
> is not correct.  This patch introduces acpi_ex_get_serial_access_length()
> to be invoked to obtain correct data buffer length.  Reported by
> Lan Tianyu, Fixed by Lv Zheng.
> 
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>

I'm queueing up this patch as a fix for 3.15, but can you please resend the
whole series with a CC to linux-acpi?


> ---
>  drivers/acpi/acpica/exfield.c | 104 +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 97 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/exfield.c b/drivers/acpi/acpica/exfield.c
> index 68d9744..12878e1 100644
> --- a/drivers/acpi/acpica/exfield.c
> +++ b/drivers/acpi/acpica/exfield.c
> @@ -45,10 +45,71 @@
>  #include "accommon.h"
>  #include "acdispat.h"
>  #include "acinterp.h"
> +#include "amlcode.h"
>  
>  #define _COMPONENT          ACPI_EXECUTER
>  ACPI_MODULE_NAME("exfield")
>  
> +/* Local prototypes */
> +static u32
> +acpi_ex_get_serial_access_length(u32 accessor_type, u32 access_length);
> +
> +/*******************************************************************************
> + *
> + * FUNCTION:    acpi_get_serial_access_bytes
> + *
> + * PARAMETERS:  accessor_type   - The type of the protocol indicated by region
> + *                                field access attributes
> + *              access_length   - The access length of the region field
> + *
> + * RETURN:      Decoded access length
> + *
> + * DESCRIPTION: This routine returns the length of the generic_serial_bus
> + *              protocol bytes
> + *
> + ******************************************************************************/
> +
> +static u32
> +acpi_ex_get_serial_access_length(u32 accessor_type, u32 access_length)
> +{
> +	u32 length;
> +
> +	switch (accessor_type) {
> +	case AML_FIELD_ATTRIB_QUICK:
> +
> +		length = 0;
> +		break;
> +
> +	case AML_FIELD_ATTRIB_SEND_RCV:
> +	case AML_FIELD_ATTRIB_BYTE:
> +
> +		length = 1;
> +		break;
> +
> +	case AML_FIELD_ATTRIB_WORD:
> +	case AML_FIELD_ATTRIB_WORD_CALL:
> +
> +		length = 2;
> +		break;
> +
> +	case AML_FIELD_ATTRIB_MULTIBYTE:
> +	case AML_FIELD_ATTRIB_RAW_BYTES:
> +	case AML_FIELD_ATTRIB_RAW_PROCESS:
> +
> +		length = access_length;
> +		break;
> +
> +	case AML_FIELD_ATTRIB_BLOCK:
> +	case AML_FIELD_ATTRIB_BLOCK_CALL:
> +	default:
> +
> +		length = ACPI_GSBUS_BUFFER_SIZE;
> +		break;
> +	}
> +
> +	return (length);
> +}
> +
>  /*******************************************************************************
>   *
>   * FUNCTION:    acpi_ex_read_data_from_field
> @@ -63,8 +124,9 @@ ACPI_MODULE_NAME("exfield")
>   *              Buffer, depending on the size of the field.
>   *
>   ******************************************************************************/
> +
>  acpi_status
> -acpi_ex_read_data_from_field(struct acpi_walk_state *walk_state,
> +acpi_ex_read_data_from_field(struct acpi_walk_state * walk_state,
>  			     union acpi_operand_object *obj_desc,
>  			     union acpi_operand_object **ret_buffer_desc)
>  {
> @@ -73,6 +135,7 @@ acpi_ex_read_data_from_field(struct acpi_walk_state *walk_state,
>  	acpi_size length;
>  	void *buffer;
>  	u32 function;
> +	u16 accessor_type;
>  
>  	ACPI_FUNCTION_TRACE_PTR(ex_read_data_from_field, obj_desc);
>  
> @@ -116,9 +179,22 @@ acpi_ex_read_data_from_field(struct acpi_walk_state *walk_state,
>  			    ACPI_READ | (obj_desc->field.attribute << 16);
>  		} else if (obj_desc->field.region_obj->region.space_id ==
>  			   ACPI_ADR_SPACE_GSBUS) {
> -			length = ACPI_GSBUS_BUFFER_SIZE;
> -			function =
> -			    ACPI_READ | (obj_desc->field.attribute << 16);
> +			accessor_type = obj_desc->field.attribute;
> +			length = acpi_ex_get_serial_access_length(accessor_type,
> +								  obj_desc->
> +								  field.
> +								  access_length);
> +
> +			/*
> +			 * Add additional 2 bytes for modeled generic_serial_bus data buffer:
> +			 * typedef struct {
> +			 *     BYTEStatus; // Byte 0 of the data buffer
> +			 *     BYTELength; // Byte 1 of the data buffer
> +			 *     BYTE[x-1]Data; // Bytes 2-x of the arbitrary length data buffer,
> +			 * }
> +			 */
> +			length += 2;
> +			function = ACPI_READ | (accessor_type << 16);
>  		} else {	/* IPMI */
>  
>  			length = ACPI_IPMI_BUFFER_SIZE;
> @@ -231,6 +307,7 @@ acpi_ex_write_data_to_field(union acpi_operand_object *source_desc,
>  	void *buffer;
>  	union acpi_operand_object *buffer_desc;
>  	u32 function;
> +	u16 accessor_type;
>  
>  	ACPI_FUNCTION_TRACE_PTR(ex_write_data_to_field, obj_desc);
>  
> @@ -284,9 +361,22 @@ acpi_ex_write_data_to_field(union acpi_operand_object *source_desc,
>  			    ACPI_WRITE | (obj_desc->field.attribute << 16);
>  		} else if (obj_desc->field.region_obj->region.space_id ==
>  			   ACPI_ADR_SPACE_GSBUS) {
> -			length = ACPI_GSBUS_BUFFER_SIZE;
> -			function =
> -			    ACPI_WRITE | (obj_desc->field.attribute << 16);
> +			accessor_type = obj_desc->field.attribute;
> +			length = acpi_ex_get_serial_access_length(accessor_type,
> +								  obj_desc->
> +								  field.
> +								  access_length);
> +
> +			/*
> +			 * Add additional 2 bytes for modeled generic_serial_bus data buffer:
> +			 * typedef struct {
> +			 *     BYTEStatus; // Byte 0 of the data buffer
> +			 *     BYTELength; // Byte 1 of the data buffer
> +			 *     BYTE[x-1]Data; // Bytes 2-x of the arbitrary length data buffer,
> +			 * }
> +			 */
> +			length += 2;
> +			function = ACPI_WRITE | (accessor_type << 16);
>  		} else {	/* IPMI */
>  
>  			length = ACPI_IPMI_BUFFER_SIZE;
>
Lan Tianyu April 22, 2014, 1:14 a.m. UTC | #2
On 2014年04月22日 05:38, Rafael J. Wysocki wrote:
> Hi,
> 
> On Wednesday, April 16, 2014 09:24:34 PM Lan Tianyu wrote:
>> From: Lv Zheng <lv.zheng@intel.com>
>>
>> The size of the buffer allocated for generic_serial_bus region access
>> is not correct.  This patch introduces acpi_ex_get_serial_access_length()
>> to be invoked to obtain correct data buffer length.  Reported by
>> Lan Tianyu, Fixed by Lv Zheng.
>>
>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> 
> I'm queueing up this patch as a fix for 3.15, but can you please resend the
> whole series with a CC to linux-acpi?
> 

Ok. I will do that.

> 
>> ---
>>  drivers/acpi/acpica/exfield.c | 104 +++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 97 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/acpi/acpica/exfield.c b/drivers/acpi/acpica/exfield.c
>> index 68d9744..12878e1 100644
>> --- a/drivers/acpi/acpica/exfield.c
>> +++ b/drivers/acpi/acpica/exfield.c
>> @@ -45,10 +45,71 @@
>>  #include "accommon.h"
>>  #include "acdispat.h"
>>  #include "acinterp.h"
>> +#include "amlcode.h"
>>  
>>  #define _COMPONENT          ACPI_EXECUTER
>>  ACPI_MODULE_NAME("exfield")
>>  
>> +/* Local prototypes */
>> +static u32
>> +acpi_ex_get_serial_access_length(u32 accessor_type, u32 access_length);
>> +
>> +/*******************************************************************************
>> + *
>> + * FUNCTION:    acpi_get_serial_access_bytes
>> + *
>> + * PARAMETERS:  accessor_type   - The type of the protocol indicated by region
>> + *                                field access attributes
>> + *              access_length   - The access length of the region field
>> + *
>> + * RETURN:      Decoded access length
>> + *
>> + * DESCRIPTION: This routine returns the length of the generic_serial_bus
>> + *              protocol bytes
>> + *
>> + ******************************************************************************/
>> +
>> +static u32
>> +acpi_ex_get_serial_access_length(u32 accessor_type, u32 access_length)
>> +{
>> +	u32 length;
>> +
>> +	switch (accessor_type) {
>> +	case AML_FIELD_ATTRIB_QUICK:
>> +
>> +		length = 0;
>> +		break;
>> +
>> +	case AML_FIELD_ATTRIB_SEND_RCV:
>> +	case AML_FIELD_ATTRIB_BYTE:
>> +
>> +		length = 1;
>> +		break;
>> +
>> +	case AML_FIELD_ATTRIB_WORD:
>> +	case AML_FIELD_ATTRIB_WORD_CALL:
>> +
>> +		length = 2;
>> +		break;
>> +
>> +	case AML_FIELD_ATTRIB_MULTIBYTE:
>> +	case AML_FIELD_ATTRIB_RAW_BYTES:
>> +	case AML_FIELD_ATTRIB_RAW_PROCESS:
>> +
>> +		length = access_length;
>> +		break;
>> +
>> +	case AML_FIELD_ATTRIB_BLOCK:
>> +	case AML_FIELD_ATTRIB_BLOCK_CALL:
>> +	default:
>> +
>> +		length = ACPI_GSBUS_BUFFER_SIZE;
>> +		break;
>> +	}
>> +
>> +	return (length);
>> +}
>> +
>>  /*******************************************************************************
>>   *
>>   * FUNCTION:    acpi_ex_read_data_from_field
>> @@ -63,8 +124,9 @@ ACPI_MODULE_NAME("exfield")
>>   *              Buffer, depending on the size of the field.
>>   *
>>   ******************************************************************************/
>> +
>>  acpi_status
>> -acpi_ex_read_data_from_field(struct acpi_walk_state *walk_state,
>> +acpi_ex_read_data_from_field(struct acpi_walk_state * walk_state,
>>  			     union acpi_operand_object *obj_desc,
>>  			     union acpi_operand_object **ret_buffer_desc)
>>  {
>> @@ -73,6 +135,7 @@ acpi_ex_read_data_from_field(struct acpi_walk_state *walk_state,
>>  	acpi_size length;
>>  	void *buffer;
>>  	u32 function;
>> +	u16 accessor_type;
>>  
>>  	ACPI_FUNCTION_TRACE_PTR(ex_read_data_from_field, obj_desc);
>>  
>> @@ -116,9 +179,22 @@ acpi_ex_read_data_from_field(struct acpi_walk_state *walk_state,
>>  			    ACPI_READ | (obj_desc->field.attribute << 16);
>>  		} else if (obj_desc->field.region_obj->region.space_id ==
>>  			   ACPI_ADR_SPACE_GSBUS) {
>> -			length = ACPI_GSBUS_BUFFER_SIZE;
>> -			function =
>> -			    ACPI_READ | (obj_desc->field.attribute << 16);
>> +			accessor_type = obj_desc->field.attribute;
>> +			length = acpi_ex_get_serial_access_length(accessor_type,
>> +								  obj_desc->
>> +								  field.
>> +								  access_length);
>> +
>> +			/*
>> +			 * Add additional 2 bytes for modeled generic_serial_bus data buffer:
>> +			 * typedef struct {
>> +			 *     BYTEStatus; // Byte 0 of the data buffer
>> +			 *     BYTELength; // Byte 1 of the data buffer
>> +			 *     BYTE[x-1]Data; // Bytes 2-x of the arbitrary length data buffer,
>> +			 * }
>> +			 */
>> +			length += 2;
>> +			function = ACPI_READ | (accessor_type << 16);
>>  		} else {	/* IPMI */
>>  
>>  			length = ACPI_IPMI_BUFFER_SIZE;
>> @@ -231,6 +307,7 @@ acpi_ex_write_data_to_field(union acpi_operand_object *source_desc,
>>  	void *buffer;
>>  	union acpi_operand_object *buffer_desc;
>>  	u32 function;
>> +	u16 accessor_type;
>>  
>>  	ACPI_FUNCTION_TRACE_PTR(ex_write_data_to_field, obj_desc);
>>  
>> @@ -284,9 +361,22 @@ acpi_ex_write_data_to_field(union acpi_operand_object *source_desc,
>>  			    ACPI_WRITE | (obj_desc->field.attribute << 16);
>>  		} else if (obj_desc->field.region_obj->region.space_id ==
>>  			   ACPI_ADR_SPACE_GSBUS) {
>> -			length = ACPI_GSBUS_BUFFER_SIZE;
>> -			function =
>> -			    ACPI_WRITE | (obj_desc->field.attribute << 16);
>> +			accessor_type = obj_desc->field.attribute;
>> +			length = acpi_ex_get_serial_access_length(accessor_type,
>> +								  obj_desc->
>> +								  field.
>> +								  access_length);
>> +
>> +			/*
>> +			 * Add additional 2 bytes for modeled generic_serial_bus data buffer:
>> +			 * typedef struct {
>> +			 *     BYTEStatus; // Byte 0 of the data buffer
>> +			 *     BYTELength; // Byte 1 of the data buffer
>> +			 *     BYTE[x-1]Data; // Bytes 2-x of the arbitrary length data buffer,
>> +			 * }
>> +			 */
>> +			length += 2;
>> +			function = ACPI_WRITE | (accessor_type << 16);
>>  		} else {	/* IPMI */
>>  
>>  			length = ACPI_IPMI_BUFFER_SIZE;
>>
>
diff mbox

Patch

diff --git a/drivers/acpi/acpica/exfield.c b/drivers/acpi/acpica/exfield.c
index 68d9744..12878e1 100644
--- a/drivers/acpi/acpica/exfield.c
+++ b/drivers/acpi/acpica/exfield.c
@@ -45,10 +45,71 @@ 
 #include "accommon.h"
 #include "acdispat.h"
 #include "acinterp.h"
+#include "amlcode.h"
 
 #define _COMPONENT          ACPI_EXECUTER
 ACPI_MODULE_NAME("exfield")
 
+/* Local prototypes */
+static u32
+acpi_ex_get_serial_access_length(u32 accessor_type, u32 access_length);
+
+/*******************************************************************************
+ *
+ * FUNCTION:    acpi_get_serial_access_bytes
+ *
+ * PARAMETERS:  accessor_type   - The type of the protocol indicated by region
+ *                                field access attributes
+ *              access_length   - The access length of the region field
+ *
+ * RETURN:      Decoded access length
+ *
+ * DESCRIPTION: This routine returns the length of the generic_serial_bus
+ *              protocol bytes
+ *
+ ******************************************************************************/
+
+static u32
+acpi_ex_get_serial_access_length(u32 accessor_type, u32 access_length)
+{
+	u32 length;
+
+	switch (accessor_type) {
+	case AML_FIELD_ATTRIB_QUICK:
+
+		length = 0;
+		break;
+
+	case AML_FIELD_ATTRIB_SEND_RCV:
+	case AML_FIELD_ATTRIB_BYTE:
+
+		length = 1;
+		break;
+
+	case AML_FIELD_ATTRIB_WORD:
+	case AML_FIELD_ATTRIB_WORD_CALL:
+
+		length = 2;
+		break;
+
+	case AML_FIELD_ATTRIB_MULTIBYTE:
+	case AML_FIELD_ATTRIB_RAW_BYTES:
+	case AML_FIELD_ATTRIB_RAW_PROCESS:
+
+		length = access_length;
+		break;
+
+	case AML_FIELD_ATTRIB_BLOCK:
+	case AML_FIELD_ATTRIB_BLOCK_CALL:
+	default:
+
+		length = ACPI_GSBUS_BUFFER_SIZE;
+		break;
+	}
+
+	return (length);
+}
+
 /*******************************************************************************
  *
  * FUNCTION:    acpi_ex_read_data_from_field
@@ -63,8 +124,9 @@  ACPI_MODULE_NAME("exfield")
  *              Buffer, depending on the size of the field.
  *
  ******************************************************************************/
+
 acpi_status
-acpi_ex_read_data_from_field(struct acpi_walk_state *walk_state,
+acpi_ex_read_data_from_field(struct acpi_walk_state * walk_state,
 			     union acpi_operand_object *obj_desc,
 			     union acpi_operand_object **ret_buffer_desc)
 {
@@ -73,6 +135,7 @@  acpi_ex_read_data_from_field(struct acpi_walk_state *walk_state,
 	acpi_size length;
 	void *buffer;
 	u32 function;
+	u16 accessor_type;
 
 	ACPI_FUNCTION_TRACE_PTR(ex_read_data_from_field, obj_desc);
 
@@ -116,9 +179,22 @@  acpi_ex_read_data_from_field(struct acpi_walk_state *walk_state,
 			    ACPI_READ | (obj_desc->field.attribute << 16);
 		} else if (obj_desc->field.region_obj->region.space_id ==
 			   ACPI_ADR_SPACE_GSBUS) {
-			length = ACPI_GSBUS_BUFFER_SIZE;
-			function =
-			    ACPI_READ | (obj_desc->field.attribute << 16);
+			accessor_type = obj_desc->field.attribute;
+			length = acpi_ex_get_serial_access_length(accessor_type,
+								  obj_desc->
+								  field.
+								  access_length);
+
+			/*
+			 * Add additional 2 bytes for modeled generic_serial_bus data buffer:
+			 * typedef struct {
+			 *     BYTEStatus; // Byte 0 of the data buffer
+			 *     BYTELength; // Byte 1 of the data buffer
+			 *     BYTE[x-1]Data; // Bytes 2-x of the arbitrary length data buffer,
+			 * }
+			 */
+			length += 2;
+			function = ACPI_READ | (accessor_type << 16);
 		} else {	/* IPMI */
 
 			length = ACPI_IPMI_BUFFER_SIZE;
@@ -231,6 +307,7 @@  acpi_ex_write_data_to_field(union acpi_operand_object *source_desc,
 	void *buffer;
 	union acpi_operand_object *buffer_desc;
 	u32 function;
+	u16 accessor_type;
 
 	ACPI_FUNCTION_TRACE_PTR(ex_write_data_to_field, obj_desc);
 
@@ -284,9 +361,22 @@  acpi_ex_write_data_to_field(union acpi_operand_object *source_desc,
 			    ACPI_WRITE | (obj_desc->field.attribute << 16);
 		} else if (obj_desc->field.region_obj->region.space_id ==
 			   ACPI_ADR_SPACE_GSBUS) {
-			length = ACPI_GSBUS_BUFFER_SIZE;
-			function =
-			    ACPI_WRITE | (obj_desc->field.attribute << 16);
+			accessor_type = obj_desc->field.attribute;
+			length = acpi_ex_get_serial_access_length(accessor_type,
+								  obj_desc->
+								  field.
+								  access_length);
+
+			/*
+			 * Add additional 2 bytes for modeled generic_serial_bus data buffer:
+			 * typedef struct {
+			 *     BYTEStatus; // Byte 0 of the data buffer
+			 *     BYTELength; // Byte 1 of the data buffer
+			 *     BYTE[x-1]Data; // Bytes 2-x of the arbitrary length data buffer,
+			 * }
+			 */
+			length += 2;
+			function = ACPI_WRITE | (accessor_type << 16);
 		} else {	/* IPMI */
 
 			length = ACPI_IPMI_BUFFER_SIZE;