| Message ID | 20250305111739.1489003-2-akuchynski@chromium.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b13abcb7ddd8d38de769486db5bd917537b32ab1 |
| Headers | show |
| Series | Fix race condition causing NULL pointer dereference | expand |
On Wed, Mar 05, 2025 at 11:17:39AM +0000, Andrei Kuchynski wrote: > Resources should be released only after all threads that utilize them > have been destroyed. > This commit ensures that resources are not released prematurely by waiting > for the associated workqueue to complete before deallocating them. > > Cc: stable@vger.kernel.org > Fixes: b9aa02ca39a4 ("usb: typec: ucsi: Add polling mechanism for partner tasks like alt mode checking") > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/usb/typec/ucsi/ucsi.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index fcf499cc9458..43b4f8207bb3 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -1825,11 +1825,11 @@ static int ucsi_init(struct ucsi *ucsi) > > err_unregister: > for (con = connector; con->port; con++) { > + if (con->wq) > + destroy_workqueue(con->wq); > ucsi_unregister_partner(con); > ucsi_unregister_altmodes(con, UCSI_RECIPIENT_CON); > ucsi_unregister_port_psy(con); > - if (con->wq) > - destroy_workqueue(con->wq); > > usb_power_delivery_unregister_capabilities(con->port_sink_caps); > con->port_sink_caps = NULL; > @@ -2013,10 +2013,6 @@ void ucsi_unregister(struct ucsi *ucsi) > > for (i = 0; i < ucsi->cap.num_connectors; i++) { > cancel_work_sync(&ucsi->connector[i].work); > - ucsi_unregister_partner(&ucsi->connector[i]); > - ucsi_unregister_altmodes(&ucsi->connector[i], > - UCSI_RECIPIENT_CON); > - ucsi_unregister_port_psy(&ucsi->connector[i]); > > if (ucsi->connector[i].wq) { > struct ucsi_work *uwork; > @@ -2032,6 +2028,11 @@ void ucsi_unregister(struct ucsi *ucsi) > destroy_workqueue(ucsi->connector[i].wq); > } > > + ucsi_unregister_partner(&ucsi->connector[i]); > + ucsi_unregister_altmodes(&ucsi->connector[i], > + UCSI_RECIPIENT_CON); > + ucsi_unregister_port_psy(&ucsi->connector[i]); > + > usb_power_delivery_unregister_capabilities(ucsi->connector[i].port_sink_caps); > ucsi->connector[i].port_sink_caps = NULL; > usb_power_delivery_unregister_capabilities(ucsi->connector[i].port_source_caps); > -- > 2.49.0.rc0.332.g42c0ae87b1-goog
Hi Andrei, On Wed, Mar 05, 2025 at 11:17:39AM +0000, Andrei Kuchynski wrote: > Resources should be released only after all threads that utilize them > have been destroyed. > This commit ensures that resources are not released prematurely by waiting > for the associated workqueue to complete before deallocating them. > > Cc: stable@vger.kernel.org > Fixes: b9aa02ca39a4 ("usb: typec: ucsi: Add polling mechanism for partner tasks like alt mode checking") > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org> Reviewed-by: Benson Leung <bleung@chromium.org> > --- > drivers/usb/typec/ucsi/ucsi.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index fcf499cc9458..43b4f8207bb3 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -1825,11 +1825,11 @@ static int ucsi_init(struct ucsi *ucsi) > > err_unregister: > for (con = connector; con->port; con++) { > + if (con->wq) > + destroy_workqueue(con->wq); > ucsi_unregister_partner(con); > ucsi_unregister_altmodes(con, UCSI_RECIPIENT_CON); > ucsi_unregister_port_psy(con); > - if (con->wq) > - destroy_workqueue(con->wq); > > usb_power_delivery_unregister_capabilities(con->port_sink_caps); > con->port_sink_caps = NULL; > @@ -2013,10 +2013,6 @@ void ucsi_unregister(struct ucsi *ucsi) > > for (i = 0; i < ucsi->cap.num_connectors; i++) { > cancel_work_sync(&ucsi->connector[i].work); > - ucsi_unregister_partner(&ucsi->connector[i]); > - ucsi_unregister_altmodes(&ucsi->connector[i], > - UCSI_RECIPIENT_CON); > - ucsi_unregister_port_psy(&ucsi->connector[i]); > > if (ucsi->connector[i].wq) { > struct ucsi_work *uwork; > @@ -2032,6 +2028,11 @@ void ucsi_unregister(struct ucsi *ucsi) > destroy_workqueue(ucsi->connector[i].wq); > } > > + ucsi_unregister_partner(&ucsi->connector[i]); > + ucsi_unregister_altmodes(&ucsi->connector[i], > + UCSI_RECIPIENT_CON); > + ucsi_unregister_port_psy(&ucsi->connector[i]); > + > usb_power_delivery_unregister_capabilities(ucsi->connector[i].port_sink_caps); > ucsi->connector[i].port_sink_caps = NULL; > usb_power_delivery_unregister_capabilities(ucsi->connector[i].port_source_caps); > -- > 2.49.0.rc0.332.g42c0ae87b1-goog >
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index fcf499cc9458..43b4f8207bb3 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -1825,11 +1825,11 @@ static int ucsi_init(struct ucsi *ucsi) err_unregister: for (con = connector; con->port; con++) { + if (con->wq) + destroy_workqueue(con->wq); ucsi_unregister_partner(con); ucsi_unregister_altmodes(con, UCSI_RECIPIENT_CON); ucsi_unregister_port_psy(con); - if (con->wq) - destroy_workqueue(con->wq); usb_power_delivery_unregister_capabilities(con->port_sink_caps); con->port_sink_caps = NULL; @@ -2013,10 +2013,6 @@ void ucsi_unregister(struct ucsi *ucsi) for (i = 0; i < ucsi->cap.num_connectors; i++) { cancel_work_sync(&ucsi->connector[i].work); - ucsi_unregister_partner(&ucsi->connector[i]); - ucsi_unregister_altmodes(&ucsi->connector[i], - UCSI_RECIPIENT_CON); - ucsi_unregister_port_psy(&ucsi->connector[i]); if (ucsi->connector[i].wq) { struct ucsi_work *uwork; @@ -2032,6 +2028,11 @@ void ucsi_unregister(struct ucsi *ucsi) destroy_workqueue(ucsi->connector[i].wq); } + ucsi_unregister_partner(&ucsi->connector[i]); + ucsi_unregister_altmodes(&ucsi->connector[i], + UCSI_RECIPIENT_CON); + ucsi_unregister_port_psy(&ucsi->connector[i]); + usb_power_delivery_unregister_capabilities(ucsi->connector[i].port_sink_caps); ucsi->connector[i].port_sink_caps = NULL; usb_power_delivery_unregister_capabilities(ucsi->connector[i].port_source_caps);
Resources should be released only after all threads that utilize them have been destroyed. This commit ensures that resources are not released prematurely by waiting for the associated workqueue to complete before deallocating them. Cc: stable@vger.kernel.org Fixes: b9aa02ca39a4 ("usb: typec: ucsi: Add polling mechanism for partner tasks like alt mode checking") Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org> --- drivers/usb/typec/ucsi/ucsi.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)