Skip to content

Commit 594c11d

Browse files
committed
ipmi: Fix use-after-free and list corruption on sender error
The analysis from Breno: When the SMI sender returns an error, smi_work() delivers an error response but then jumps back to restart without cleaning up properly: 1. intf->curr_msg is not cleared, so no new message is pulled 2. newmsg still points to the message, causing sender() to be called again with the same message 3. If sender() fails again, deliver_err_response() is called with the same recv_msg that was already queued for delivery This causes list_add corruption ("list_add double add") because the recv_msg is added to the user_msgs list twice. Subsequently, the corrupted list leads to use-after-free when the memory is freed and reused, and eventually a NULL pointer dereference when accessing recv_msg->done. The buggy sequence: sender() fails -> deliver_err_response(recv_msg) // recv_msg queued for delivery -> goto restart // curr_msg not cleared! sender() fails again (same message!) -> deliver_err_response(recv_msg) // tries to queue same recv_msg -> LIST CORRUPTION Fix this by freeing the message and setting it to NULL on a send error. Also, always free the newmsg on a send error, otherwise it will leak. Reported-by: Breno Leitao <leitao@debian.org> Closes: https://lore.kernel.org/lkml/20260127-ipmi-v1-0-ba5cc90f516f@debian.org/ Fixes: 9cf93a8 ("ipmi: Allow an SMI sender to return an error") Cc: stable@vger.kernel.org # 4.18 Reviewed-by: Breno Leitao <leitao@debian.org> Signed-off-by: Corey Minyard <corey@minyard.net>
1 parent b71e635 commit 594c11d

1 file changed

Lines changed: 9 additions & 2 deletions

File tree

drivers/char/ipmi/ipmi_msghandler.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4852,8 +4852,15 @@ static void smi_work(struct work_struct *t)
48524852
if (newmsg->recv_msg)
48534853
deliver_err_response(intf,
48544854
newmsg->recv_msg, cc);
4855-
else
4856-
ipmi_free_smi_msg(newmsg);
4855+
if (!run_to_completion)
4856+
spin_lock_irqsave(&intf->xmit_msgs_lock,
4857+
flags);
4858+
intf->curr_msg = NULL;
4859+
if (!run_to_completion)
4860+
spin_unlock_irqrestore(&intf->xmit_msgs_lock,
4861+
flags);
4862+
ipmi_free_smi_msg(newmsg);
4863+
newmsg = NULL;
48574864
goto restart;
48584865
}
48594866
}

0 commit comments

Comments
 (0)