mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tzung-Bi Shih <tzungbi@kernel.org>
To: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>,
	linux-kernel@vger.kernel.org, tzungbi@kernel.org
Subject: [PATCH v2 1/2] reset: Fix reset_control leak when put after controller unregister
Date: Thu,  5 Nov 2026 00:20:37 +0800	[thread overview]
Message-ID: <20261104162038.48622-2-tzungbi@kernel.org> (raw)
In-Reply-To: <20261104162038.48622-1-tzungbi@kernel.org>

After commit 8c91302a29bc ("reset: protect struct reset_control with its
own mutex"), reset_control_put_internal() returns early if `rcdev` is
NULL.

When a reset controller is unregistered while consumers still hold
reset_control handles, reset_controller_unregister() sets `rstc->rcdev`
to NULL.  When consumers later release their handles via
reset_control_put(), the early return skips kref_put(), resulting in
leaking resources.

Call kref_put() in reset_control_put_internal() even if `rcdev` is NULL.

Fixes: 8c91302a29bc ("reset: protect struct reset_control with its own mutex")
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- Factor out __reset_control_free().
- Call kref_put() directly in reset_controller_unregister() instead of
  calling reset_control_put_internal().
- Drop redundant lockdep_assert_held() and add comment for
  __reset_control_release().

v1: https://lore.kernel.org/all/20261102155840.20376-2-tzungbi@kernel.org
---
 drivers/reset/core.c | 47 ++++++++++++++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 38e189d04d09..f0ab09795ae1 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -160,6 +160,9 @@ int reset_controller_register(struct reset_controller_dev *rcdev)
 }
 EXPORT_SYMBOL_GPL(reset_controller_register);
 
+static void __reset_control_release(struct kref *kref);
+static void __reset_control_free(struct reset_control *rstc);
+
 static void reset_controller_remove(struct reset_controller_dev *rcdev,
 				    struct reset_control *rstc)
 {
@@ -185,11 +188,18 @@ void reset_controller_unregister(struct reset_controller_dev *rcdev)
 		/*
 		 * Numb but don't free the remaining reset control handles that are
 		 * still held by consumers.
+		 *
+		 * Hold a temporary reference on rstc before clearing rstc->rcdev:
+		 * once rstc->rcdev is NULL, a concurrent reset_control_put_internal()
+		 * will not acquire rcdev->lock and could otherwise free rstc.
 		 */
 		list_for_each_entry_safe(rstc, pos, &rcdev->reset_control_head, list) {
+			kref_get(&rstc->refcnt);
 			rcu_assign_pointer(rstc->rcdev, NULL);
 			synchronize_srcu(&rstc->srcu);
 			reset_controller_remove(rcdev, rstc);
+			if (kref_put(&rstc->refcnt, __reset_control_release))
+				__reset_control_free(rstc);
 		}
 	}
 
@@ -886,14 +896,16 @@ __reset_control_get_internal(struct reset_controller_dev *rcdev,
 	return rstc;
 }
 
+/*
+ * Detaches rstc from rcdev (if still attached) when refcnt reaches 0.
+ * If rstc is still attached, the caller must hold rcdev->lock.
+ */
 static void __reset_control_release(struct kref *kref)
 {
 	struct reset_control *rstc = container_of(kref, struct reset_control,
 						  refcnt);
 	struct reset_controller_dev *rcdev;
 
-	lockdep_assert_held(&rstc->srcu);
-
 	rcdev = rcu_replace_pointer(rstc->rcdev, NULL, true);
 	if (rcdev) {
 		lockdep_assert_held(&rcdev->lock);
@@ -903,6 +915,17 @@ static void __reset_control_release(struct kref *kref)
 	mutex_destroy(&rstc->lock);
 }
 
+/*
+ * Must be called outside the rstc->srcu read-side critical section after
+ * kref_put(..., __reset_control_release) returns true.
+ */
+static void __reset_control_free(struct reset_control *rstc)
+{
+	synchronize_srcu(&rstc->srcu);
+	cleanup_srcu_struct(&rstc->srcu);
+	kfree(rstc);
+}
+
 static void reset_control_put_internal(struct reset_control *rstc)
 {
 	struct reset_controller_dev *rcdev;
@@ -913,19 +936,17 @@ static void reset_control_put_internal(struct reset_control *rstc)
 
 	scoped_guard(srcu, &rstc->srcu) {
 		rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
-		if (!rcdev)
-			/* Already released. */
-			return;
-
-		guard(mutex)(&rcdev->lock);
-		ret = kref_put(&rstc->refcnt, __reset_control_release);
+		if (rcdev) {
+			guard(mutex)(&rcdev->lock);
+			ret = kref_put(&rstc->refcnt, __reset_control_release);
+		} else {
+			/* Already detached from rcdev. */
+			ret = kref_put(&rstc->refcnt, __reset_control_release);
+		}
 	}
 
-	if (ret) {
-		synchronize_srcu(&rstc->srcu);
-		cleanup_srcu_struct(&rstc->srcu);
-		kfree(rstc);
-	}
+	if (ret)
+		__reset_control_free(rstc);
 }
 
 static void reset_gpio_aux_device_release(struct device *dev)
-- 
2.53.0


  reply	other threads:[~2026-11-04 16:21 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-04 16:20 [PATCH v2 0/2] reset: Fix issues reported by Sashiko Tzung-Bi Shih
2026-11-04 16:20 ` Tzung-Bi Shih [this message]
2026-11-04 17:38   ` [PATCH v2 1/2] reset: Fix reset_control leak when put after controller unregister Markus Elfring
2026-11-04 16:20 ` [PATCH v2 2/2] reset: Fix circular locking dependency in reset_controller_unregister() Tzung-Bi Shih

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=20261104162038.48622-2-tzungbi@kernel.org \
    --to=tzungbi@kernel.org \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    /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

all inboxes | Powered by JetHome®