linux-kernel - [PATCH 03/13] ACPI/IPMI: Fix race caused by the unprotected ACPI IPMI transfers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 23 Jul 2013 16:09:15 +0800
From:	Lv Zheng <lv.zheng@...el.com>
To:	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	Len Brown <len.brown@...el.com>,
	Corey Minyard <minyard@....org>,
	Zhao Yakui <yakui.zhao@...el.com>
Cc:	Lv Zheng <lv.zheng@...el.com>, <linux-kernel@...r.kernel.org>,
	<stable@...r.kernel.org>, linux-acpi@...r.kernel.org,
	openipmi-developer@...ts.sourceforge.net
Subject: [PATCH 03/13] ACPI/IPMI: Fix race caused by the unprotected ACPI IPMI transfers

This patch fixes races caused by unprotected ACPI IPMI transfers.

We can see the following crashes may occur:
1. There is no tx_msg_lock held for iterating tx_msg_list in
   ipmi_flush_tx_msg() while it is parellel unlinked on failure in
   acpi_ipmi_space_handler() under protection of tx_msg_lock.
2. There is no lock held for freeing tx_msg in acpi_ipmi_space_handler()
   while it is parellel accessed in ipmi_flush_tx_msg() and
   ipmi_msg_handler().

This patch enhances tx_msg_lock to protect all tx_msg accesses to solve
this issue.  Then tx_msg_lock is always held around complete() and tx_msg
accesses.
Calling smp_wmb() before setting msg_done flag so that messages completed
due to flushing will not be handled as 'done' messages while their contents
are not vaild.

Signed-off-by: Lv Zheng <lv.zheng@...el.com>
Cc: Zhao Yakui <yakui.zhao@...el.com>
Reviewed-by: Huang Ying <ying.huang@...el.com>
---
 drivers/acpi/acpi_ipmi.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c
index b37c189..527ee43 100644
--- a/drivers/acpi/acpi_ipmi.c
+++ b/drivers/acpi/acpi_ipmi.c
@@ -230,11 +230,14 @@ static void ipmi_flush_tx_msg(struct acpi_ipmi_device *ipmi)
 	struct acpi_ipmi_msg *tx_msg, *temp;
 	int count = HZ / 10;
 	struct pnp_dev *pnp_dev = ipmi->pnp_dev;
+	unsigned long flags;
 
+	spin_lock_irqsave(&ipmi->tx_msg_lock, flags);
 	list_for_each_entry_safe(tx_msg, temp, &ipmi->tx_msg_list, head) {
 		/* wake up the sleep thread on the Tx msg */
 		complete(&tx_msg->tx_complete);
 	}
+	spin_unlock_irqrestore(&ipmi->tx_msg_lock, flags);
 
 	/* wait for about 100ms to flush the tx message list */
 	while (count--) {
@@ -268,13 +271,12 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
 			break;
 		}
 	}
-	spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags);
 
 	if (!msg_found) {
 		dev_warn(&pnp_dev->dev,
 			 "Unexpected response (msg id %ld) is returned.\n",
 			 msg->msgid);
-		goto out_msg;
+		goto out_lock;
 	}
 
 	/* copy the response data to Rx_data buffer */
@@ -286,10 +288,14 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
 	}
 	tx_msg->rx_len = msg->msg.data_len;
 	memcpy(tx_msg->data, msg->msg.data, tx_msg->rx_len);
+	/* tx_msg content must be valid before setting msg_done flag */
+	smp_wmb();
 	tx_msg->msg_done = 1;
 
 out_comp:
 	complete(&tx_msg->tx_complete);
+out_lock:
+	spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags);
 out_msg:
 	ipmi_free_recv_msg(msg);
 }
-- 
1.7.10

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/