linux-kernel - [PATCH] ipmi: Fix use-after-free and list corruption on sender error

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20260127-ipmi-v1-1-ba5cc90f516f@debian.org>
Date: Tue, 27 Jan 2026 01:57:59 -0800
From: Breno Leitao <leitao@...ian.org>
To: Corey Minyard <corey@...yard.net>, 
 Nathan Chancellor <nathan@...nel.org>, 
 Nick Desaulniers <nick.desaulniers+lkml@...il.com>, 
 Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>
Cc: openipmi-developer@...ts.sourceforge.net, linux-kernel@...r.kernel.org, 
 llvm@...ts.linux.dev, Breno Leitao <leitao@...ian.org>, 
 kernel-team@...a.com
Subject: [PATCH] ipmi: Fix use-after-free and list corruption on sender
 error

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 by introducing a send_failed flag that signals when sender()
returns an error. At the restart label, inside the existing spinlock
critical section, check this flag and clear curr_msg if set. This
ensures:

- The smi_msg is always freed after sender error
- curr_msg is cleared so the next iteration pulls a new message
- No stale pointers remain that could cause use-after-free
- Only one lock acquisition per iteration (avoids extra lock/unlock
  in the error path)

Fixes: 9cf93a8fa9513 ("ipmi: Allow an SMI sender to return an error")
Signed-off-by: Breno Leitao <leitao@...ian.org>
---
 drivers/char/ipmi/ipmi_msghandler.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 3f48fc6ab596d..81259c93261fb 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -4816,6 +4816,7 @@ static void smi_work(struct work_struct *t)
 	int run_to_completion = READ_ONCE(intf->run_to_completion);
 	struct ipmi_smi_msg *newmsg = NULL;
 	struct ipmi_recv_msg *msg, *msg2;
+	bool send_failed = false;
 	int cc;
 
 	/*
@@ -4828,6 +4829,16 @@ static void smi_work(struct work_struct *t)
 restart:
 	if (!run_to_completion)
 		spin_lock_irqsave(&intf->xmit_msgs_lock, flags);
+	if (send_failed) {
+		/*
+		 * Sender failed, clear curr_msg so we can pull a new
+		 * message. Do not clear it unconditionally as a message
+		 * may be in flight from a previous run.
+		 */
+		intf->curr_msg = NULL;
+		send_failed = false;
+	}
+	newmsg = NULL;
 	if (intf->curr_msg == NULL && !intf->in_shutdown) {
 		struct list_head *entry = NULL;
 
@@ -4852,8 +4863,14 @@ static void smi_work(struct work_struct *t)
 			if (newmsg->recv_msg)
 				deliver_err_response(intf,
 						     newmsg->recv_msg, cc);
-			else
-				ipmi_free_smi_msg(newmsg);
+			/*
+			 * Sender returned error, the lower layer did not
+			 * take ownership of the message. The transaction
+			 * is abandoned - the user has been notified via
+			 * deliver_err_response() above.
+			 */
+			ipmi_free_smi_msg(newmsg);
+			send_failed = true;
 			goto restart;
 		}
 	}

-- 
2.47.3