[Skiboot] [PATCH] hw/bt.c: Fix message response search

[Alistair Popple]

bt_get_resp() uses the sequence number to locate the outstanding
message by searching the queue with list_for_each(...). The check to
see if a message was found in the queue is:

    if (!bt_msg || (bt_msg->seq != seq))

However this check is incorrect. list_for_each(...) does not set
bt_msg to NULL at the end of the loop, nor does it set it to the last
element in the list. Rather it ends up pointing at an offset from the
list_head. Therefore bt_msg is never equal to NULL and the first half
of this condition is always false.

However the second condition is almost always true as sequence numbers
are constantly increasing and unlikely to match whatever bt_msg->seq
ends up pointing to, but in rare circumstances it is possible for
bt_msg->seq to equal seq and hence the overall expression can evaluate
to false.

This leads to bt_msg being used as a valid message pointer even though
it points to an incorrect address, typically causing a xstop due to an
invalid address access when ipmi_cmd_done(...) attempts to call the
callbacks.

Signed-off-by: Alistair Popple <alistair at popple.id.au>
---
 hw/bt.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/bt.c b/hw/bt.c
index 8a9e935..da59b14 100644
--- a/hw/bt.c
+++ b/hw/bt.c
@@ -207,7 +207,7 @@ static void bt_flush_msg(void)
 static void bt_get_resp(void)
 {
 	int i;
-	struct bt_msg *bt_msg;
+	struct bt_msg *tmp_bt_msg, *bt_msg = NULL;
 	struct ipmi_msg *ipmi_msg;
 	uint8_t resp_len, netfn, seq, cmd;
 	uint8_t cc = IPMI_CC_NO_ERROR;
@@ -236,13 +236,14 @@ static void bt_get_resp(void)
 	cc = bt_inb(BT_HOST2BMC);
 
 	/* Find the corresponding messasge */
-	list_for_each(&bt.msgq, bt_msg, link) {
-		if (bt_msg->seq == seq) {
+	list_for_each(&bt.msgq, tmp_bt_msg, link) {
+		if (tmp_bt_msg->seq == seq) {
+			bt_msg = tmp_bt_msg;
 			break;
 		}
 
 	}
-	if (!bt_msg || (bt_msg->seq != seq)) {
+	if (!bt_msg) {
 		/* A response to a message we no longer care about. */
 		prlog(PR_INFO, "BT: Nobody cared about a response to an BT/IPMI message\n");
 		bt_flush_msg();
-- 
1.8.3.2