[PATCH v2 0/2] driver core: class: Fix bug and code improvements for class APIs

[PATCH v2 0/2] driver core: class: Fix bug and code improvements for class APIs

[Zijun Hu]

This patch series is to

- Fix an potential wild pointer dereference bug for API:
  class_dev_iter_next()

- Improve the following APIs:
  class_for_each_device()
  class_find_device()

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
Changes in v2:
- Remove both fix and stable tag for patch 1/3
- drop patch 3/3
- Link to v1: https://lore.kernel.org/r/20241105-class_fix-v1-0-80866f9994a5@quicinc.com

---
Zijun Hu (2):
      driver core: class: Fix wild pointer dereference in API class_dev_iter_next()
      driver core: class: Correct WARN() message in APIs class_(for_each|find)_device()

 drivers/base/class.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
---
base-commit: 9bd133f05b1dca5ca4399a76d04d0f6f4d454e44
change-id: 20241104-class_fix-f176bd9eba22

Best regards,
-- 
Zijun Hu <quic_zijuhu@quicinc.com>

[PATCH v2 1/2] driver core: class: Fix wild pointer dereference in API class_dev_iter_next()

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.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@quicinc.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

[PATCH v2 2/2] driver core: class: Correct WARN() message in APIs class_(for_each|find)_device()

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

For both API class_for_each_device(const struct class *class, ...) and
class_find_device(const struct class *class, ...), their WARN() messages
prompt @class was not initialized when suffer class_to_subsys(@class)
error, but the error actually means @class was not registered, so these
warning messages are not accurate.

Fix by replacing term initialized with registered within these messages.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/base/class.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/base/class.c b/drivers/base/class.c
index b331dda002e3..e81da280af74 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -411,7 +411,7 @@ int class_for_each_device(const struct class *class, const struct device *start,
 	if (!class)
 		return -EINVAL;
 	if (!sp) {
-		WARN(1, "%s called for class '%s' before it was initialized",
+		WARN(1, "%s called for class '%s' before it was registered",
 		     __func__, class->name);
 		return -EINVAL;
 	}
@@ -459,7 +459,7 @@ struct device *class_find_device(const struct class *class, const struct device
 	if (!class)
 		return NULL;
 	if (!sp) {
-		WARN(1, "%s called for class '%s' before it was initialized",
+		WARN(1, "%s called for class '%s' before it was registered",
 		     __func__, class->name);
 		return NULL;
 	}

-- 
2.34.1