* [PATCH v2 0/1] Fix race condition causing NULL pointer dereference
@ 2025-03-05 11:17 Andrei Kuchynski
2025-03-05 11:17 ` [PATCH v2 1/1] usb: typec: ucsi: Fix NULL pointer access Andrei Kuchynski
0 siblings, 1 reply; 4+ messages in thread
From: Andrei Kuchynski @ 2025-03-05 11:17 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman, Dmitry Baryshkov,
Benson Leung, Christian A. Ehrhardt, Jameson Thies, linux-usb,
linux-kernel
Cc: Andrei Kuchynski
v2: tag update, no code changes.
The kernel crashes during UCSI initialization due to a race condition.
In ucsi_init():
1. ucsi_register_port() sets up a work queue and schedules
ucsi_check_connector_capability task.
2. "PPM policy conflict" causes ucsi_send_command to fail.
3. The error path (err_unregister) deallocates resources,
setting con->partner to NULL.
4. After that, ucsi_init() waits for the work queue to finish its task.
5. ucsi_check_connector_capability task, running in the work queue,
attempts to dereference the con->partner pointer, resulting in the crash.
The core issue is that con->partner is set to NULL before
the work queue task is guaranteed to have finished using it.
The crash log:
cros_ec_ucsi cros_ec_ucsi.3.auto: PPM Policy conflict
BUG: kernel NULL pointer dereference, address: 000000000000030c
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
CPU: 8 UID: 0 PID: 13 Comm: kworker/u64:1 Tainted: G U W
6.12.0-g15b373ee5573-dirty #1 b5276ebf6ba85f471d9524ce34509877165c9f58
Tainted: [U]=USER, [W]=WARN
Hardware name: Google Fatcat/Fatcat, BIOS Google_Fatcat.16163.0.0 01/15/2025
Workqueue: cros_ec_ucsi.3.auto-con1 ucsi_poll_worker [typec_ucsi]
RIP: 0010:typec_partner_set_pd_revision+0x5/0x80 [typec]
Code: cc cc cc b8 ea ff ff ff c3 cc cc cc cc cc 0f 1f 80 00 00 00 00 90 90 90
90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 <66> 39 b7 0c 03 00 00
75 06 c3 cc cc cc cc cc 55 48 89 e5 41 56 53
RSP: 0018:ffffb532400c7dd8 EFLAGS: 00010206
RAX: 0000000000000004 RBX: 0000000000000004 RCX: 0000000000000000
RDX: ffffb532400c7cc0 RSI: 0000000000000300 RDI: 0000000000000000
RBP: ffffb532400c7de8 R08: ffffa3ab042d28f0 R09: 0000000000000080
R10: 0000000000000080 R11: 00000000000000c0 R12: ffffa3ab01dc6480
R13: ffffa3ab120d12c0 R14: ffffa3ab120d12c0 R15: ffffa3ab12074000
FS: 0000000000000000(0000) GS:ffffa3ae6f800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000000030c CR3: 000000010700e004 CR4: 0000000000772ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff07f0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
<TASK>
? __die_body+0x6a/0xb0
? page_fault_oops+0x38e/0x400
? work_grab_pending+0x56/0x230
? exc_page_fault+0x5b/0xb0
? asm_exc_page_fault+0x22/0x30
? typec_partner_set_pd_revision+0x5/0x80
[typec bc1e7c7e089f4aaed440a0a5388387e3ef1ca2cb]
ucsi_check_connector_capability+0x71/0xa0 \
[typec_ucsi 843b0396f746abb17c01f8d4d12ead8b09b88609]
ucsi_poll_worker+0x3c/0x110
[typec_ucsi 843b0396f746abb17c01f8d4d12ead8b09b88609]
process_scheduled_works+0x20e/0x450
worker_thread+0x2e0/0x390
kthread+0xee/0x110
? __pfx_worker_thread+0x10/0x10
? __pfx_kthread+0x10/0x10
ret_from_fork+0x38/0x50
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
Andrei Kuchynski (1):
usb: typec: ucsi: Fix NULL pointer access
drivers/usb/typec/ucsi/ucsi.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
--
2.49.0.rc0.332.g42c0ae87b1-goog
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/1] usb: typec: ucsi: Fix NULL pointer access
2025-03-05 11:17 [PATCH v2 0/1] Fix race condition causing NULL pointer dereference Andrei Kuchynski
@ 2025-03-05 11:17 ` Andrei Kuchynski
2025-03-05 13:59 ` Heikki Krogerus
2025-03-06 20:28 ` Benson Leung
0 siblings, 2 replies; 4+ messages in thread
From: Andrei Kuchynski @ 2025-03-05 11:17 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman, Dmitry Baryshkov,
Benson Leung, Christian A. Ehrhardt, Jameson Thies, linux-usb,
linux-kernel
Cc: Andrei Kuchynski, stable
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(-)
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] usb: typec: ucsi: Fix NULL pointer access
2025-03-05 11:17 ` [PATCH v2 1/1] usb: typec: ucsi: Fix NULL pointer access Andrei Kuchynski
@ 2025-03-05 13:59 ` Heikki Krogerus
2025-03-06 20:28 ` Benson Leung
1 sibling, 0 replies; 4+ messages in thread
From: Heikki Krogerus @ 2025-03-05 13:59 UTC (permalink / raw)
To: Andrei Kuchynski
Cc: Greg Kroah-Hartman, Dmitry Baryshkov, Benson Leung,
Christian A. Ehrhardt, Jameson Thies, linux-usb, linux-kernel,
stable
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
--
heikki
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] usb: typec: ucsi: Fix NULL pointer access
2025-03-05 11:17 ` [PATCH v2 1/1] usb: typec: ucsi: Fix NULL pointer access Andrei Kuchynski
2025-03-05 13:59 ` Heikki Krogerus
@ 2025-03-06 20:28 ` Benson Leung
1 sibling, 0 replies; 4+ messages in thread
From: Benson Leung @ 2025-03-06 20:28 UTC (permalink / raw)
To: Andrei Kuchynski
Cc: Heikki Krogerus, Greg Kroah-Hartman, Dmitry Baryshkov,
Benson Leung, Christian A. Ehrhardt, Jameson Thies, linux-usb,
linux-kernel, stable
[-- Attachment #1: Type: text/plain, Size: 2408 bytes --]
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
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-03-06 20:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-05 11:17 [PATCH v2 0/1] Fix race condition causing NULL pointer dereference Andrei Kuchynski
2025-03-05 11:17 ` [PATCH v2 1/1] usb: typec: ucsi: Fix NULL pointer access Andrei Kuchynski
2025-03-05 13:59 ` Heikki Krogerus
2025-03-06 20:28 ` Benson Leung
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox