linux-kernel - [PATCH v2 1/2] driver core: class: Fix wild pointer dereference in API class_dev_iter

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

Message-Id: <20241112-class_fix-v2-1-73d198d0a0d5@quicinc.com>
Date: Tue, 12 Nov 2024 23:15:12 +0800
From: Zijun Hu <zijun_hu@...oud.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
 "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Zijun Hu <zijun_hu@...oud.com>, linux-kernel@...r.kernel.org, 
 Zijun Hu <quic_zijuhu@...cinc.com>
Subject: [PATCH v2 1/2] driver core: class: Fix wild pointer dereference in
 API class_dev_iter_next()

From: Zijun Hu <quic_zijuhu@...cinc.com>

class_dev_iter_init(struct class_dev_iter *iter, struct class *class, ...)
has return type void, but it does not initialize its output parameter @iter
when suffers class_to_subsys(@class) error, so caller can not detect the
error and call API class_dev_iter_next(@iter) which will dereference wild
pointers of @iter's members as shown by below typical usage:

// @iter's members are wild pointers
struct class_dev_iter iter;

// No change in @iter when the error happens.
class_dev_iter_init(&iter, ...);

// dereference these wild member pointers here.
while (dev = class_dev_iter_next(&iter)) { ... }.

Actually, all callers of the API have such usage pattern in kernel tree.
Fix by memset() @iter in API *_init() and error checking @iter in *_next().

Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>

---
Alternative fix solutions ever thought about:

1) Use BUG_ON(!sp) instead of error return in class_dev_iter_init().
2) Change class_dev_iter_init()'s type to int, lots of jobs to do.
---
 drivers/base/class.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/base/class.c b/drivers/base/class.c
index cb5359235c70..b331dda002e3 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -323,8 +323,11 @@ void class_dev_iter_init(struct class_dev_iter *iter, const struct class *class,
 	struct subsys_private *sp = class_to_subsys(class);
 	struct klist_node *start_knode = NULL;
 
-	if (!sp)
+	memset(iter, 0, sizeof(*iter));
+	if (!sp) {
+		pr_crit("%s: the class was not registered yet\n", __func__);
 		return;
+	}
 
 	if (start)
 		start_knode = &start->p->knode_class;
@@ -351,6 +354,9 @@ struct device *class_dev_iter_next(struct class_dev_iter *iter)
 	struct klist_node *knode;
 	struct device *dev;
 
+	if (!iter->sp)
+		return NULL;
+
 	while (1) {
 		knode = klist_next(&iter->ki);
 		if (!knode)

-- 
2.34.1