Re: [PATCH 03/13] ACPI/IPMI: Fix race caused by the unprotected ACPIIPMI transfers

From: Corey Minyard
Date: Thu Jul 25 2013 - 20:48:21 EST

On 07/25/2013 07:16 PM, Zheng, Lv wrote:

If I understand this correctly, the problem would be if:

rem_time = wait_for_completion_timeout(&tx_msg->tx_complete,

returns on a timeout, then checks msg_done and races with something setting
msg_done. If that is the case, you would need the smp_rmb() before checking

However, the timeout above is unnecessary. You are using
ipmi_request_settime(), so you can set the timeout when the IPMI command
fails and returns a failure message. The driver guarantees a return message
for each request. Just remove the timeout from the completion, set the
timeout and retries in the ipmi request, and the completion should handle the
barrier issues.
It's just difficult for me to determine retry count and timeout value, maybe retry=0, timeout=IPMI_TIMEOUT is OK.
The code of the timeout completion is already there, I think the quick fix code should not introduce this logic.
I'll add a new patch to apply your comment.

Since it is a local BMC, I doubt a retry is required. That is probably fine. Or you could set retry=1 and timeout=IPMI_TIMEOUT/2 if you wanted to be more sure, but I doubt it would make a difference. The only time you really need to worry about retries is if you are resetting the BMC or it is being overloaded.

Plus, from a quick glance at the code, it doesn't look like it will properly handle a
situation where the timeout occurs and is handled then the response comes in
PATCH 07 fixed this issue.
Here we just need the smp_rmb() or holding tx_msg_lock() around the acpi_format_ipmi_response().

If you apply the fix like I suggest, then the race goes away. If there's no timeout and it just waits for the completion, things get a lot simpler.

Thanks for commenting.

No problem, thanks for working on this.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at