[PATCH] usb: ucsi: fix connector partner ucsi work issue

[PATCH] usb: ucsi: fix connector partner ucsi work issue

[Linyu Yuan]

When ucsi unregister, it will destroy connector work queue, but a pending
delay work may still exist, once delay timer expire, as work queue
destroyed, it will cause system crash.

Move all partner related delay work to connector instance and cancel all
of them when ucsi unregister happen.

Fixes: b9aa02c ("usb: typec: ucsi: Add polling mechanism for partner tasks like alt mode checking")
Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
 drivers/usb/typec/ucsi/ucsi.c | 61 +++++++++++++++++++++++++------------------
 drivers/usb/typec/ucsi/ucsi.h | 11 ++++++++
 2 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index eabe519..f6b23e9 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -185,14 +185,6 @@ EXPORT_SYMBOL_GPL(ucsi_send_command);
 
 /* -------------------------------------------------------------------------- */
 
-struct ucsi_work {
-	struct delayed_work work;
-	unsigned long delay;
-	unsigned int count;
-	struct ucsi_connector *con;
-	int (*cb)(struct ucsi_connector *);
-};
-
 static void ucsi_poll_worker(struct work_struct *work)
 {
 	struct ucsi_work *uwork = container_of(work, struct ucsi_work, work.work);
@@ -203,7 +195,6 @@ static void ucsi_poll_worker(struct work_struct *work)
 
 	if (!con->partner) {
 		mutex_unlock(&con->lock);
-		kfree(uwork);
 		return;
 	}
 
@@ -211,30 +202,20 @@ static void ucsi_poll_worker(struct work_struct *work)
 
 	if (uwork->count-- && (ret == -EBUSY || ret == -ETIMEDOUT))
 		queue_delayed_work(con->wq, &uwork->work, uwork->delay);
-	else
-		kfree(uwork);
 
 	mutex_unlock(&con->lock);
 }
 
-static int ucsi_partner_task(struct ucsi_connector *con,
-			     int (*cb)(struct ucsi_connector *),
+static int ucsi_partner_task(struct ucsi_work *uwork,
 			     int retries, unsigned long delay)
 {
-	struct ucsi_work *uwork;
+	struct ucsi_connector *con = uwork->con;
 
 	if (!con->partner)
 		return 0;
 
-	uwork = kzalloc(sizeof(*uwork), GFP_KERNEL);
-	if (!uwork)
-		return -ENOMEM;
-
-	INIT_DELAYED_WORK(&uwork->work, ucsi_poll_worker);
 	uwork->count = retries;
 	uwork->delay = delay;
-	uwork->con = con;
-	uwork->cb = cb;
 
 	queue_delayed_work(con->wq, &uwork->work, delay);
 
@@ -636,8 +617,8 @@ static void ucsi_pwr_opmode_change(struct ucsi_connector *con)
 	case UCSI_CONSTAT_PWR_OPMODE_PD:
 		con->rdo = con->status.request_data_obj;
 		typec_set_pwr_opmode(con->port, TYPEC_PWR_MODE_PD);
-		ucsi_partner_task(con, ucsi_get_src_pdos, 30, 0);
-		ucsi_partner_task(con, ucsi_check_altmodes, 30, 0);
+		ucsi_partner_task(&con->pdos_work, 30, 0);
+		ucsi_partner_task(&con->altmodes_work, 30, 0);
 		break;
 	case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5:
 		con->rdo = 0;
@@ -799,7 +780,7 @@ static void ucsi_handle_connector_change(struct work_struct *work)
 
 		if (con->status.flags & UCSI_CONSTAT_CONNECTED) {
 			ucsi_register_partner(con);
-			ucsi_partner_task(con, ucsi_check_connection, 1, HZ);
+			ucsi_partner_task(&con->connection_work, 1, HZ);
 		} else {
 			ucsi_unregister_partner(con);
 		}
@@ -818,7 +799,7 @@ static void ucsi_handle_connector_change(struct work_struct *work)
 	}
 
 	if (con->status.change & UCSI_CONSTAT_CAM_CHANGE)
-		ucsi_partner_task(con, ucsi_check_altmodes, 1, 0);
+		ucsi_partner_task(&con->altmodes_work, 1, 0);
 
 	clear_bit(EVENT_PENDING, &con->ucsi->flags);
 
@@ -1034,6 +1015,21 @@ static struct fwnode_handle *ucsi_find_fwnode(struct ucsi_connector *con)
 	return NULL;
 }
 
+static void ucsi_partner_task_init(struct ucsi_connector *con)
+{
+	INIT_DELAYED_WORK(&con->connection_work.work, ucsi_poll_worker);
+	con->connection_work.cb = ucsi_check_connection;
+	con->connection_work.con = con;
+
+	INIT_DELAYED_WORK(&con->pdos_work.work, ucsi_poll_worker);
+	con->pdos_work.cb = ucsi_get_src_pdos;
+	con->pdos_work.con = con;
+
+	INIT_DELAYED_WORK(&con->altmodes_work.work, ucsi_poll_worker);
+	con->altmodes_work.cb = ucsi_check_altmodes;
+	con->altmodes_work.con = con;
+}
+
 static int ucsi_register_port(struct ucsi *ucsi, int index)
 {
 	struct ucsi_connector *con = &ucsi->connector[index];
@@ -1053,6 +1049,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
 	if (!con->wq)
 		return -ENOMEM;
 
+	ucsi_partner_task_init(con);
 	INIT_WORK(&con->work, ucsi_handle_connector_change);
 	init_completion(&con->complete);
 	mutex_init(&con->lock);
@@ -1287,7 +1284,7 @@ static void ucsi_resume_work(struct work_struct *work)
 
 	for (con = ucsi->connector; con->port; con++) {
 		mutex_lock(&con->lock);
-		ucsi_partner_task(con, ucsi_check_connection, 1, 0);
+		ucsi_partner_task(&con->connection_work, 1, 0);
 		mutex_unlock(&con->lock);
 	}
 }
@@ -1396,6 +1393,17 @@ int ucsi_register(struct ucsi *ucsi)
 }
 EXPORT_SYMBOL_GPL(ucsi_register);
 
+static void ucsi_partner_task_destroy(struct ucsi_connector *con)
+{
+	mutex_lock(&con->lock);
+
+	cancel_delayed_work_sync(&con->connection_work.work);
+	cancel_delayed_work_sync(&con->pdos_work.work);
+	cancel_delayed_work_sync(&con->altmodes_work.work);
+
+	mutex_unlock(&con->lock);
+}
+
 /**
  * ucsi_unregister - Unregister UCSI interface
  * @ucsi: UCSI interface to be unregistered
@@ -1420,6 +1428,7 @@ void ucsi_unregister(struct ucsi *ucsi)
 		ucsi_unregister_altmodes(&ucsi->connector[i],
 					 UCSI_RECIPIENT_CON);
 		ucsi_unregister_port_psy(&ucsi->connector[i]);
+		ucsi_partner_task_destroy(&ucsi->connector[i]);
 		if (ucsi->connector[i].wq)
 			destroy_workqueue(ucsi->connector[i].wq);
 		typec_unregister_port(ucsi->connector[i].port);
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index c968474..40d39d2 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -314,6 +314,14 @@ struct ucsi {
 #define UCSI_TYPEC_1_5_CURRENT	1500
 #define UCSI_TYPEC_3_0_CURRENT	3000
 
+struct ucsi_work {
+	struct delayed_work work;
+	unsigned long delay;
+	unsigned int count;
+	struct ucsi_connector *con;
+	int (*cb)(struct ucsi_connector *con);
+};
+
 struct ucsi_connector {
 	int num;
 
@@ -322,6 +330,9 @@ struct ucsi_connector {
 	struct work_struct work;
 	struct completion complete;
 	struct workqueue_struct *wq;
+	struct ucsi_work connection_work;
+	struct ucsi_work pdos_work;
+	struct ucsi_work altmodes_work;
 
 	struct typec_port *port;
 	struct typec_partner *partner;
-- 
2.7.4

Re: [PATCH] usb: ucsi: fix connector partner ucsi work issue

[kernel test robot]

Hi,

Thanks for your patch.

FYI: kernel test robot notices the stable kernel rule is not satisfied.

Rule: 'Cc: stable@vger.kernel.org' or 'commit <sha1> upstream.'
Subject: [PATCH] usb: ucsi: fix connector partner ucsi work issue
Link: https://lore.kernel.org/stable/1672892324-12335-1-git-send-email-quic_linyyuan%40quicinc.com

The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests