linux-kernel - [PATCH net] dpll: fix dpll_pin

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240419194711.1075349-1-arkadiusz.kubalewski@intel.com>
Date: Fri, 19 Apr 2024 21:47:11 +0200
From: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
To: netdev@...r.kernel.org
Cc: vadim.fedorenko@...ux.dev,
	jiri@...nulli.us,
	davem@...emloft.net,
	rrameshbabu@...dia.com,
	linux-kernel@...r.kernel.org,
	pabeni@...hat.com,
	kuba@...nel.org,
	mschmidt@...hat.com,
	Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
	Przemek Kitszel <przemyslaw.kitszel@...el.com>
Subject: [PATCH net] dpll: fix dpll_pin_registration missing refcount

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@...el.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.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