public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Zijun Hu <zijun_hu@icloud.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	 "Rafael J. Wysocki" <rafael@kernel.org>,
	Tejun Heo <tj@kernel.org>,  Josef Bacik <josef@toxicpanda.com>,
	Jens Axboe <axboe@kernel.dk>,  Boris Burkov <boris@bur.io>,
	Davidlohr Bueso <dave@stgolabs.net>,
	 Jonathan Cameron <jonathan.cameron@huawei.com>,
	 Dave Jiang <dave.jiang@intel.com>,
	 Alison Schofield <alison.schofield@intel.com>,
	 Vishal Verma <vishal.l.verma@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	 Dan Williams <dan.j.williams@intel.com>
Cc: Zijun Hu <zijun_hu@icloud.com>,
	linux-kernel@vger.kernel.org,  cgroups@vger.kernel.org,
	linux-block@vger.kernel.org,  linux-cxl@vger.kernel.org,
	Zijun Hu <quic_zijuhu@quicinc.com>,
	 Jonathan Cameron <Jonathan.Cameron@huawei.com>
Subject: [PATCH v5 1/8] driver core: class: Fix wild pointer dereferences in API class_dev_iter_next()
Date: Tue, 24 Dec 2024 21:37:20 +0800	[thread overview]
Message-ID: <20241224-class_fix-v5-1-9eaaf7abe843@quicinc.com> (raw)
In-Reply-To: <20241224-class_fix-v5-0-9eaaf7abe843@quicinc.com>

From: Zijun Hu <quic_zijuhu@quicinc.com>

There are a potential wild pointer dereferences issue regarding APIs
class_dev_iter_(init|next|exit)(), as explained by below typical usage:

// All members of @iter are wild pointers.
struct class_dev_iter iter;

// class_dev_iter_init(@iter, @class, ...) checks parameter @class for
// potential class_to_subsys() error, and it returns void type and does
// not initialize its output parameter @iter, so caller can not detect
// the error and continues to invoke class_dev_iter_next(@iter) even if
// @iter still contains wild pointers.
class_dev_iter_init(&iter, ...);

// Dereference these wild pointers in @iter here once suffer the error.
while (dev = class_dev_iter_next(&iter)) { ... };

// Also dereference these wild pointers here.
class_dev_iter_exit(&iter);

Actually, all callers of these APIs have such usage pattern in kernel tree.
Fix by:
- Initialize output parameter @iter by memset() in class_dev_iter_init()
  and give callers prompt by pr_crit() for the error.
- Check if @iter is valid in class_dev_iter_next().

Fixes: 7b884b7f24b4 ("driver core: class.c: convert to only use class_to_subsys")
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
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.

This issue is APIs themself issues, and regardless of how various API
users use them, and silent wild pointer dereferences are not what API
users expect for the error absolutely.
---
 drivers/base/class.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/base/class.c b/drivers/base/class.c
index 582b5a02a5c410113326601fe00eb6d7231f988f..d57f277978dc9033fba3484b4620bcf884a4029f 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -323,8 +323,12 @@ 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: class %p was not registered yet\n",
+			__func__, class);
 		return;
+	}
 
 	if (start)
 		start_knode = &start->p->knode_class;
@@ -351,6 +355,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


  reply	other threads:[~2024-12-24 13:38 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-24 13:37 [PATCH v5 0/8] driver core: class: Fix bug and code improvements for class APIs Zijun Hu
2024-12-24 13:37 ` Zijun Hu [this message]
2024-12-24 13:37 ` [PATCH v5 2/8] blk-cgroup: Fix class @block_class's subsystem refcount leakage Zijun Hu
2024-12-24 13:37 ` [PATCH v5 3/8] driver core: Move true expression out of if condition in 3 device finding APIs Zijun Hu
2024-12-24 13:37 ` [PATCH v5 4/8] driver core: Rename declaration parameter name for API device_find_child() cluster Zijun Hu
2024-12-24 13:37 ` [PATCH v5 5/8] driver core: Correct parameter check for API device_for_each_child_reverse_from() Zijun Hu
2024-12-24 16:27   ` Jonathan Cameron
2024-12-24 13:37 ` [PATCH v5 6/8] driver core: Correct API device_for_each_child_reverse_from() prototype Zijun Hu
2024-12-24 13:37 ` [PATCH v5 7/8] driver core: Introduce device_iter_t for device iterating APIs Zijun Hu
2024-12-24 13:37 ` [PATCH v5 8/8] driver core: Move two simple APIs for finding child device to header Zijun Hu
2024-12-24 16:28   ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241224-class_fix-v5-1-9eaaf7abe843@quicinc.com \
    --to=zijun_hu@icloud.com \
    --cc=alison.schofield@intel.com \
    --cc=axboe@kernel.dk \
    --cc=boris@bur.io \
    --cc=cgroups@vger.kernel.org \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=ira.weiny@intel.com \
    --cc=jonathan.cameron@huawei.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=quic_zijuhu@quicinc.com \
    --cc=rafael@kernel.org \
    --cc=tj@kernel.org \
    --cc=vishal.l.verma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox