All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
To: netdev@vger.kernel.org
Cc: vadim.fedorenko@linux.dev, jiri@resnulli.us, davem@davemloft.net,
	rrameshbabu@nvidia.com, linux-kernel@vger.kernel.org,
	pabeni@redhat.com, kuba@kernel.org, mschmidt@redhat.com,
	Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>
Subject: [PATCH net] dpll: fix dpll_pin_registration missing refcount
Date: Fri, 19 Apr 2024 21:47:11 +0200	[thread overview]
Message-ID: <20240419194711.1075349-1-arkadiusz.kubalewski@intel.com> (raw)

In scenario where pin is registered with multiple parent pins via
dpll_pin_on_pin_register(..), belonging to the same dpll device,
and each time with the same set of ops/priv data, a reference
between a pin and dpll is created once and then refcounted, at the same
time the dpll_pin_registration is only checked for existence and created
if does not exist. This is wrong, as for the same ops/priv data a
registration shall be also refcounted, a child pin is also registered
with dpll device, until each child is unregistered the registration data
shall exist.

Add refcount and check if all registrations are dropped before releasing
dpll_pin_registration resources.

Currently, the following crash/call trace is produced when ice driver is
removed on the system with installed NIC which includes dpll device:

WARNING: CPU: 51 PID: 9155 at drivers/dpll/dpll_core.c:809 dpll_pin_ops+0x20/0x30
Call Trace:
 dpll_msg_add_pin_freq+0x37/0x1d0
 dpll_cmd_pin_get_one+0x1c0/0x400
 ? __nlmsg_put+0x63/0x80
 dpll_pin_event_send+0x93/0x140
 dpll_pin_on_pin_unregister+0x3f/0x100
 ice_dpll_deinit_pins+0xa1/0x230 [ice]
 ice_remove+0xf1/0x210 [ice]

Fixes: b446631f355e ("dpll: fix dpll_xa_ref_*_del() for multiple registrations")
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
 drivers/dpll/dpll_core.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index 64eaca80d736..7ababa327c0c 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -40,6 +40,7 @@ struct dpll_device_registration {
 
 struct dpll_pin_registration {
 	struct list_head list;
+	refcount_t refcount;
 	const struct dpll_pin_ops *ops;
 	void *priv;
 };
@@ -81,6 +82,7 @@ dpll_xa_ref_pin_add(struct xarray *xa_pins, struct dpll_pin *pin,
 		reg = dpll_pin_registration_find(ref, ops, priv);
 		if (reg) {
 			refcount_inc(&ref->refcount);
+			refcount_inc(&reg->refcount);
 			return 0;
 		}
 		ref_exists = true;
@@ -113,6 +115,7 @@ dpll_xa_ref_pin_add(struct xarray *xa_pins, struct dpll_pin *pin,
 	reg->priv = priv;
 	if (ref_exists)
 		refcount_inc(&ref->refcount);
+	refcount_set(&reg->refcount, 1);
 	list_add_tail(&reg->list, &ref->registration_list);
 
 	return 0;
@@ -131,8 +134,10 @@ static int dpll_xa_ref_pin_del(struct xarray *xa_pins, struct dpll_pin *pin,
 		reg = dpll_pin_registration_find(ref, ops, priv);
 		if (WARN_ON(!reg))
 			return -EINVAL;
-		list_del(&reg->list);
-		kfree(reg);
+		if (refcount_dec_and_test(&reg->refcount)) {
+			list_del(&reg->list);
+			kfree(reg);
+		}
 		if (refcount_dec_and_test(&ref->refcount)) {
 			xa_erase(xa_pins, i);
 			WARN_ON(!list_empty(&ref->registration_list));
@@ -160,6 +165,7 @@ dpll_xa_ref_dpll_add(struct xarray *xa_dplls, struct dpll_device *dpll,
 		reg = dpll_pin_registration_find(ref, ops, priv);
 		if (reg) {
 			refcount_inc(&ref->refcount);
+			refcount_inc(&reg->refcount);
 			return 0;
 		}
 		ref_exists = true;
@@ -192,6 +198,7 @@ dpll_xa_ref_dpll_add(struct xarray *xa_dplls, struct dpll_device *dpll,
 	reg->priv = priv;
 	if (ref_exists)
 		refcount_inc(&ref->refcount);
+	refcount_set(&reg->refcount, 1);
 	list_add_tail(&reg->list, &ref->registration_list);
 
 	return 0;
@@ -211,8 +218,10 @@ dpll_xa_ref_dpll_del(struct xarray *xa_dplls, struct dpll_device *dpll,
 		reg = dpll_pin_registration_find(ref, ops, priv);
 		if (WARN_ON(!reg))
 			return;
-		list_del(&reg->list);
-		kfree(reg);
+		if (refcount_dec_and_test(&reg->refcount)) {
+			list_del(&reg->list);
+			kfree(reg);
+		}
 		if (refcount_dec_and_test(&ref->refcount)) {
 			xa_erase(xa_dplls, i);
 			WARN_ON(!list_empty(&ref->registration_list));

base-commit: ac1a21db32eda8a09076bad025d7b848dd086d28
-- 
2.38.1


             reply	other threads:[~2024-04-19 19:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-19 19:47 Arkadiusz Kubalewski [this message]
2024-04-22 13:31 ` [PATCH net] dpll: fix dpll_pin_registration missing refcount Jiri Pirko
2024-04-23 11:04   ` Kubalewski, Arkadiusz
2024-04-23 11:16     ` Jiri Pirko
2024-04-24 10:26       ` Kubalewski, Arkadiusz

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=20240419194711.1075349-1-arkadiusz.kubalewski@intel.com \
    --to=arkadiusz.kubalewski@intel.com \
    --cc=davem@davemloft.net \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mschmidt@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=rrameshbabu@nvidia.com \
    --cc=vadim.fedorenko@linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.