linux-kernel - [PATCH stable] ipmi:msghandler: Fix potential memory corruption in ipmi_create

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-next>] [day] [month] [year] [list]

Message-ID: <20250630-ipmi-fix-v1-1-2d496de3c856@google.com>
Date: Mon, 30 Jun 2025 17:09:02 +0000
From: Brendan Jackman <jackmanb@...gle.com>
To: stable@...r.kernel.org, Corey Minyard <minyard@....org>
Cc: Corey Minyard <cminyard@...sta.com>, openipmi-developer@...ts.sourceforge.net, 
	linux-kernel@...r.kernel.org, Dan Carpenter <dan.carpenter@...aro.org>, 
	Corey Minyard <corey@...yard.net>, Brendan Jackman <jackmanb@...gle.com>
Subject: [PATCH stable] ipmi:msghandler: Fix potential memory corruption in ipmi_create_user()

From: Dan Carpenter <dan.carpenter@...aro.org>

commit fa332f5dc6fc662ad7d3200048772c96b861cf6b upstream

The "intf" list iterator is an invalid pointer if the correct
"intf->intf_num" is not found.  Calling atomic_dec(&intf->nr_users) on
and invalid pointer will lead to memory corruption.

We don't really need to call atomic_dec() if we haven't called
atomic_add_return() so update the if (intf->in_shutdown) path as well.

Fixes: 8e76741c3d8b ("ipmi: Add a limit on the number of users that may use IPMI")
Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>
Message-ID: <aBjMZ8RYrOt6NOgi@...nley.mountain>
Signed-off-by: Corey Minyard <corey@...yard.net>
Signed-off-by: Brendan Jackman <jackmanb@...gle.com>
---
I have tested this in 6.12 with Google's platform drivers added to
reproduce the bug.  The bug causes the panic notifier chain to get
corrupted leading to a crash. With the fix this goes away.

Applies to 6.6 too but I haven't tested it there.

Backport changes:

- Dropped change to the `if (intf->in_shutdown)` block since that logic
  doesn't exist yet.
- Modified out_unlock to release the srcu lock instead of the mutex
  since we don't have the mutex here yet.
---
 drivers/char/ipmi/ipmi_msghandler.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index e12b531f5c2f338008a42dc2c35b0a62395c9f3c..6a4a8ecd0edd02793eda70f9f9ae578e37da477f 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -1241,7 +1241,7 @@ int ipmi_create_user(unsigned int          if_num,
 	}
 	/* Not found, return an error */
 	rv = -EINVAL;
-	goto out_kfree;
+	goto out_unlock;
 
  found:
 	if (atomic_add_return(1, &intf->nr_users) > max_users) {
@@ -1283,6 +1283,7 @@ int ipmi_create_user(unsigned int          if_num,
 
 out_kfree:
 	atomic_dec(&intf->nr_users);
+out_unlock:
 	srcu_read_unlock(&ipmi_interfaces_srcu, index);
 	vfree(new_user);
 	return rv;

---
base-commit: 783cd2c3dca8b6c434e955b84c20c8940588dc68
change-id: 20250630-ipmi-fix-c565f7098afd

Best regards,
-- 
Brendan Jackman <jackmanb@...gle.com>