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(®->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(®->refcount, 1);
list_add_tail(®->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(®->list);
- kfree(reg);
+ if (refcount_dec_and_test(®->refcount)) {
+ list_del(®->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(®->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(®->refcount, 1);
list_add_tail(®->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(®->list);
- kfree(reg);
+ if (refcount_dec_and_test(®->refcount)) {
+ list_del(®->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
next 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox