From 8584ec6ae9cc386de344e0d33b60f76368bb73ab Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:13:47 +0800 Subject: [PATCH] ACPI / IPMI: Fix race caused by the timed out ACPI IPMI transfers This patch fixes races caused by timed out ACPI IPMI transfers. This patch uses timeout mechanism provided by ipmi_si to avoid the race that the msg_done flag is set but without any protection, its content can be invalid. Thanks for the suggestion of Corey Minyard. Signed-off-by: Lv Zheng Reviewed-by: Huang Ying Signed-off-by: Rafael J. Wysocki --- drivers/acpi/acpi_ipmi.c | 49 ++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index 87307baeafab..9171a1a668f2 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -51,7 +51,7 @@ MODULE_LICENSE("GPL"); #define ACPI_IPMI_TIMEOUT 0x10 #define ACPI_IPMI_UNKNOWN 0x07 /* the IPMI timeout is 5s */ -#define IPMI_TIMEOUT (5 * HZ) +#define IPMI_TIMEOUT (5000) #define ACPI_IPMI_MAX_MSG_LENGTH 64 struct acpi_ipmi_device { @@ -135,6 +135,7 @@ static struct acpi_ipmi_msg *acpi_alloc_ipmi_msg(struct acpi_ipmi_device *ipmi) init_completion(&ipmi_msg->tx_complete); INIT_LIST_HEAD(&ipmi_msg->head); ipmi_msg->device = ipmi; + ipmi_msg->msg_done = ACPI_IPMI_UNKNOWN; return ipmi_msg; } @@ -192,7 +193,7 @@ static int acpi_format_ipmi_request(struct acpi_ipmi_msg *tx_msg, } static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg, - acpi_integer *value, int rem_time) + acpi_integer *value) { struct acpi_ipmi_buffer *buffer; @@ -201,24 +202,17 @@ static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg, * IPMI message returned by IPMI command. */ buffer = (struct acpi_ipmi_buffer *)value; - if (!rem_time && !msg->msg_done) { - buffer->status = ACPI_IPMI_TIMEOUT; - return; - } /* - * If the flag of msg_done is not set or the recv length is zero, it - * means that the IPMI command is not executed correctly. - * The status code will be ACPI_IPMI_UNKNOWN. + * If the flag of msg_done is not set, it means that the IPMI command is + * not executed correctly. */ - if (!msg->msg_done || !msg->rx_len) { - buffer->status = ACPI_IPMI_UNKNOWN; + buffer->status = msg->msg_done; + if (msg->msg_done != ACPI_IPMI_OK) return; - } /* * If the IPMI response message is obtained correctly, the status code * will be ACPI_IPMI_OK */ - buffer->status = ACPI_IPMI_OK; buffer->length = msg->rx_len; memcpy(buffer->data, msg->data, msg->rx_len); } @@ -280,11 +274,23 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) dev_WARN_ONCE(&pnp_dev->dev, true, "Unexpected response (msg len %d).\n", msg->msg.data_len); - } else { - tx_msg->rx_len = msg->msg.data_len; - memcpy(tx_msg->data, msg->msg.data, tx_msg->rx_len); - tx_msg->msg_done = 1; + goto out_comp; } + /* response msg is an error msg */ + msg->recv_type = IPMI_RESPONSE_RECV_TYPE; + if (msg->recv_type == IPMI_RESPONSE_RECV_TYPE && + msg->msg.data_len == 1) { + if (msg->msg.data[0] == IPMI_TIMEOUT_COMPLETION_CODE) { + dev_WARN_ONCE(&pnp_dev->dev, true, + "Unexpected response (timeout).\n"); + tx_msg->msg_done = ACPI_IPMI_TIMEOUT; + } + goto out_comp; + } + tx_msg->rx_len = msg->msg.data_len; + memcpy(tx_msg->data, msg->msg.data, tx_msg->rx_len); + tx_msg->msg_done = ACPI_IPMI_OK; +out_comp: complete(&tx_msg->tx_complete); out_lock: spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); @@ -392,7 +398,7 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, { struct acpi_ipmi_msg *tx_msg; struct acpi_ipmi_device *ipmi_device = handler_context; - int err, rem_time; + int err; acpi_status status; unsigned long flags; /* @@ -422,14 +428,13 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, &tx_msg->addr, tx_msg->tx_msgid, &tx_msg->tx_message, - NULL, 0, 0, 0); + NULL, 0, 0, IPMI_TIMEOUT); if (err) { status = AE_ERROR; goto out_list; } - rem_time = wait_for_completion_timeout(&tx_msg->tx_complete, - IPMI_TIMEOUT); - acpi_format_ipmi_response(tx_msg, value, rem_time); + wait_for_completion(&tx_msg->tx_complete); + acpi_format_ipmi_response(tx_msg, value); status = AE_OK; out_list: -- 2.39.5