* [PATCH 0/2] Fix invalid pointer access
@ 2025-03-21 14:37 Andrei Kuchynski
2025-03-21 14:37 ` [PATCH 1/2] usb: typec: class: Fix NULL " Andrei Kuchynski
2025-03-21 14:37 ` [PATCH 2/2] usb: typec: class: Invalidate USB device pointers on partner unregistration Andrei Kuchynski
0 siblings, 2 replies; 7+ messages in thread
From: Andrei Kuchynski @ 2025-03-21 14:37 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman, linux-usb, linux-kernel
Cc: Benson Leung, Jameson Thies, Andrei Kuchynski
These patches fix bugs in the USB Type-C class driver, addressing a NULL
pointer dereference and invalid pointer usage during partner
registration / unregistration.
1. The first patch resolves NULL pointer dereference resulting from a race
condition where the port driver and USB hub driver simultaneously invoke
typec_partner_unlink_device. The crash originated from dev->kdef.name being
NULL during the second unlinking process.
The crash log:
BUG: kernel NULL pointer dereference, address: 0000000000000000
CPU: 0 PID: 55688 Comm: kworker/0:2 Tainted: G U 6.6.56-05934
Hardware name: Google Redrix/Redrix, BIOS Google_Redrix.14505.778.0
Workqueue: events cros_typec_port_work [cros_ec_typec]
RIP: 0010:strlen+0xb/0x20
RSP: 0000:ffffbe400c5b3b90 EFLAGS: 00010202
RAX: ffffffffffffffff RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000001 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffffbe400c5b3b90 R08: ffffa2a011187000 R09: ffffffff8d633a4d
R10: 0000000000000000 R11: ffffffffc092a6b0 R12: ffffa29ff4d02990
R13: ffffa29ff4d02d98 R14: 0000000000000000 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffffa2a6df800000(0000) knlGS:00000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 0000000191cb0000 CR4: 0000000000750ef0
Call Trace:
<TASK>
? __die_body+0x68/0xb0
? page_fault_oops+0x379/0x3e0
? ep_poll_callback+0x132/0x260
? exc_page_fault+0x4f/0xa0
? asm_exc_page_fault+0x22/0x30
? __pfx_pmc_usb_mux_set+0x10/0x10 [intel_pmc_mux (HASH:1400 30)]
? kobject_uevent_env+0x13d/0x3c0
? strlen+0xb/0x20
kernfs_name_hash+0x17/0x90
kernfs_find_ns+0x3e/0xe0
kernfs_remove_by_name_ns+0x47/0xb0
typec_unregister_partner+0x4f/0xd0 [typec (HASH:1400 31)]
cros_typec_remove_partner+0x19f/0x200 [cros_ec_typec (HASH:1400 32)]
cros_typec_port_update+0x555/0x1690 [cros_ec_typec (HASH:1400 32)]
? snprintf+0xd0/0x640
? cros_ec_typec_event+0x2a/0x40 [cros_ec_typec (HASH:1400 32)]
? blocking_notifier_call_chain+0x73/0xf0
cros_typec_port_work+0x34/0x60 [cros_ec_typec (HASH:1400 32)]
worker_thread+0x395/0x940
kthread+0xed/0x110
? __pfx_worker_thread+0x10/0x10
? __pfx_kthread+0x10/0x10
ret_from_fork+0x38/0x50
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1b/0x30
</TASK>
2. The USB HUB driver's failure to notify the Type-C class driver about
device detachments resulted in typec_partner_detach() not being invoked.
This omission caused the port driver to retain a reference to a released
device, triggering a kernel WARNING upon unplug. Furthermore, when a new
device was connected, typec_register_partner() attempted to use an obsolete
port->usb3_dev pointer. The crash, occurring in typec_register_partner(),
is resolved in a second patch.
The WARNING log occurred during device unplug:
[534629.006366] kernfs: can not remove 'typec', no directory
WARNING: CPU: 0 PID: 864020 at fs/kernfs/dir.c:1684
kernfs_remove_by_name_ns+0x79/0xb0
CPU: 0 PID: 864020 Comm: kworker/0:2 Tainted: G U 6.6.56-05934
Hardware name: Google Screebo/Screebo, BIOS Google_Screebo.15709.185.0
Workqueue: events cros_typec_port_work [cros_ec_typec]
RIP: 0010:kernfs_remove_by_name_ns+0x79/0xb0
Call Trace:
<TASK>
? __warn+0xca/0x1c0
? kernfs_remove_by_name_ns+0x79/0xb0
? report_bug+0x14e/0x1f0
? handle_bug+0x41/0x70
? exc_invalid_op+0x1b/0x50
? asm_exc_invalid_op+0x16/0x20
? kernfs_remove_by_name_ns+0x79/0xb0
typec_unregister_partner+0x5e/0xd0 [typec (HASH:1400 25)]
cros_typec_remove_partner+0x19f/0x200 [cros_ec_typec (HASH:1400 26)]
cros_typec_port_update+0x4a0/0x12a0 [cros_ec_typec (HASH:1400 26)]
? cros_usbpd_charger_ec_command+0x20/0xd0
? __queue_work+0x3f4/0x520
? queue_work_on+0x6a/0x80
? cros_ec_typec_event+0x2a/0x40 [cros_ec_typec (HASH:1400 26)]
? blocking_notifier_call_chain+0xce/0xf0
cros_typec_port_work+0x34/0x60 [cros_ec_typec (HASH:1400 26)]
process_scheduled_works+0x299/0x490
worker_thread+0x2d7/0x3b0
The crash log occurred during the subsequent plug:
[536103.301256] BUG: kernel NULL pointer dereference,
address: 000000000000050b
Oops: 0000 [#1] PREEMPT SMP NOPTI
CPU: 0 PID: 864020 Comm: kworker/0:2 Tainted: G U 6.6.56-05934
Hardware name: Google Screebo/Screebo, BIOS Google_Screebo.15709.185.0
Workqueue: events cros_typec_port_work [cros_ec_typec]
RIP: 0010:kernfs_new_node+0x1b/0x80
RSP: 0018:ffffbd318c433bd8 EFLAGS: 00010246
RAX: 0000000000000004 RBX: 0000000000000498 RCX: 000000000000a1ff
RDX: ffffffffc0a3b435 RSI: ffffffffc0a3b435 RDI: 0000000000000498
RBP: ffffbd318c433be0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: ffffffffa61156d0 R12: ffff9e573ad2d400
R13: ffff9e5653c47e10 R14: 0000000000000498 R15: 0000000000000498
FS: 0000000000000000(0000) GS:ffff9e5d5fe00000(0000) knlGS:00000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000000050b CR3: 00000002ea838003 CR4: 0000000000770ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff07f0 DR7: 0000000000000400
Call Trace:
<TASK>
? __die_body+0x68/0xb0
? page_fault_oops+0x379/0x3e0
? exc_page_fault+0x4f/0xa0
? asm_exc_page_fault+0x22/0x30
? __pfx_klist_class_dev_get+0x10/0x10
? kernfs_new_node+0x1b/0x80
kernfs_create_link+0x3d/0xa0
sysfs_do_create_link_sd+0x68/0xd0
typec_register_partner+0x12f/0x220 [typec (HASH:1400 25)]
cros_typec_port_update+0x5dc/0x12a0 [cros_ec_typec (HASH:1400 26)]
? cros_usbpd_charger_ec_command+0x20/0xd0
? __queue_work+0x3f4/0x520
? cros_ec_typec_event+0x2a/0x40 [cros_ec_typec (HASH:1400 26)]
? blocking_notifier_call_chain+0xce/0xf0
cros_typec_port_work+0x34/0x60 [cros_ec_typec (HASH:1400 26)]
process_scheduled_works+0x299/0x490
worker_thread+0x2d7/0x3b0
kthread+0xed/0x110
? __pfx_worker_thread+0x10/0x10
? __pfx_kthread+0x10/0x10
ret_from_fork+0x38/0x50
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1b/0x30
</TASK>
Andrei Kuchynski (2):
usb: typec: class: Fix NULL pointer access
usb: typec: class: Invalidate USB device pointers on partner
unregistration
drivers/usb/typec/class.c | 23 +++++++++++++++++++----
drivers/usb/typec/class.h | 1 +
2 files changed, 20 insertions(+), 4 deletions(-)
--
2.49.0.395.g12beb8f557-goog
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] usb: typec: class: Fix NULL pointer access
2025-03-21 14:37 [PATCH 0/2] Fix invalid pointer access Andrei Kuchynski
@ 2025-03-21 14:37 ` Andrei Kuchynski
2025-03-21 23:16 ` Benson Leung
2025-03-24 9:42 ` Heikki Krogerus
2025-03-21 14:37 ` [PATCH 2/2] usb: typec: class: Invalidate USB device pointers on partner unregistration Andrei Kuchynski
1 sibling, 2 replies; 7+ messages in thread
From: Andrei Kuchynski @ 2025-03-21 14:37 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman, linux-usb, linux-kernel
Cc: Benson Leung, Jameson Thies, Andrei Kuchynski, stable
Concurrent calls to typec_partner_unlink_device can lead to a NULL pointer
dereference. This patch adds a mutex to protect USB device pointers and
prevent this issue. The same mutex protects both the device pointers and
the partner device registration.
Cc: stable@vger.kernel.org
Fixes: 59de2a56d127 ("usb: typec: Link enumerated USB devices with Type-C partner")
Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
drivers/usb/typec/class.c | 15 +++++++++++++--
drivers/usb/typec/class.h | 1 +
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 9c76c3d0c6cf..eadb150223f8 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1052,6 +1052,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
partner->usb_mode = USB_MODE_USB3;
}
+ mutex_lock(&port->partner_link_lock);
ret = device_register(&partner->dev);
if (ret) {
dev_err(&port->dev, "failed to register partner (%d)\n", ret);
@@ -1063,6 +1064,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
typec_partner_link_device(partner, port->usb2_dev);
if (port->usb3_dev)
typec_partner_link_device(partner, port->usb3_dev);
+ mutex_unlock(&port->partner_link_lock);
return partner;
}
@@ -1083,12 +1085,14 @@ void typec_unregister_partner(struct typec_partner *partner)
port = to_typec_port(partner->dev.parent);
+ mutex_lock(&port->partner_link_lock);
if (port->usb2_dev)
typec_partner_unlink_device(partner, port->usb2_dev);
if (port->usb3_dev)
typec_partner_unlink_device(partner, port->usb3_dev);
device_unregister(&partner->dev);
+ mutex_unlock(&port->partner_link_lock);
}
EXPORT_SYMBOL_GPL(typec_unregister_partner);
@@ -2041,10 +2045,11 @@ static struct typec_partner *typec_get_partner(struct typec_port *port)
static void typec_partner_attach(struct typec_connector *con, struct device *dev)
{
struct typec_port *port = container_of(con, struct typec_port, con);
- struct typec_partner *partner = typec_get_partner(port);
+ struct typec_partner *partner;
struct usb_device *udev = to_usb_device(dev);
enum usb_mode usb_mode;
+ mutex_lock(&port->partner_link_lock);
if (udev->speed < USB_SPEED_SUPER) {
usb_mode = USB_MODE_USB2;
port->usb2_dev = dev;
@@ -2053,18 +2058,22 @@ static void typec_partner_attach(struct typec_connector *con, struct device *dev
port->usb3_dev = dev;
}
+ partner = typec_get_partner(port);
if (partner) {
typec_partner_set_usb_mode(partner, usb_mode);
typec_partner_link_device(partner, dev);
put_device(&partner->dev);
}
+ mutex_unlock(&port->partner_link_lock);
}
static void typec_partner_deattach(struct typec_connector *con, struct device *dev)
{
struct typec_port *port = container_of(con, struct typec_port, con);
- struct typec_partner *partner = typec_get_partner(port);
+ struct typec_partner *partner;
+ mutex_lock(&port->partner_link_lock);
+ partner = typec_get_partner(port);
if (partner) {
typec_partner_unlink_device(partner, dev);
put_device(&partner->dev);
@@ -2074,6 +2083,7 @@ static void typec_partner_deattach(struct typec_connector *con, struct device *d
port->usb2_dev = NULL;
else if (port->usb3_dev == dev)
port->usb3_dev = NULL;
+ mutex_unlock(&port->partner_link_lock);
}
/**
@@ -2614,6 +2624,7 @@ struct typec_port *typec_register_port(struct device *parent,
ida_init(&port->mode_ids);
mutex_init(&port->port_type_lock);
+ mutex_init(&port->partner_link_lock);
port->id = id;
port->ops = cap->ops;
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index b3076a24ad2e..db2fe96c48ff 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -59,6 +59,7 @@ struct typec_port {
enum typec_port_type port_type;
enum usb_mode usb_mode;
struct mutex port_type_lock;
+ struct mutex partner_link_lock;
enum typec_orientation orientation;
struct typec_switch *sw;
--
2.49.0.395.g12beb8f557-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] usb: typec: class: Invalidate USB device pointers on partner unregistration
2025-03-21 14:37 [PATCH 0/2] Fix invalid pointer access Andrei Kuchynski
2025-03-21 14:37 ` [PATCH 1/2] usb: typec: class: Fix NULL " Andrei Kuchynski
@ 2025-03-21 14:37 ` Andrei Kuchynski
2025-03-21 23:19 ` Benson Leung
2025-03-24 9:43 ` Heikki Krogerus
1 sibling, 2 replies; 7+ messages in thread
From: Andrei Kuchynski @ 2025-03-21 14:37 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman, linux-usb, linux-kernel
Cc: Benson Leung, Jameson Thies, Andrei Kuchynski, stable
To avoid using invalid USB device pointers after a Type-C partner
disconnects, this patch clears the pointers upon partner unregistration.
This ensures a clean state for future connections.
Cc: stable@vger.kernel.org
Fixes: 59de2a56d127 ("usb: typec: Link enumerated USB devices with Type-C partner")
Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
drivers/usb/typec/class.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index eadb150223f8..3df3e3736916 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1086,10 +1086,14 @@ void typec_unregister_partner(struct typec_partner *partner)
port = to_typec_port(partner->dev.parent);
mutex_lock(&port->partner_link_lock);
- if (port->usb2_dev)
+ if (port->usb2_dev) {
typec_partner_unlink_device(partner, port->usb2_dev);
- if (port->usb3_dev)
+ port->usb2_dev = NULL;
+ }
+ if (port->usb3_dev) {
typec_partner_unlink_device(partner, port->usb3_dev);
+ port->usb3_dev = NULL;
+ }
device_unregister(&partner->dev);
mutex_unlock(&port->partner_link_lock);
--
2.49.0.395.g12beb8f557-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] usb: typec: class: Fix NULL pointer access
2025-03-21 14:37 ` [PATCH 1/2] usb: typec: class: Fix NULL " Andrei Kuchynski
@ 2025-03-21 23:16 ` Benson Leung
2025-03-24 9:42 ` Heikki Krogerus
1 sibling, 0 replies; 7+ messages in thread
From: Benson Leung @ 2025-03-21 23:16 UTC (permalink / raw)
To: Andrei Kuchynski
Cc: Heikki Krogerus, Greg Kroah-Hartman, linux-usb, linux-kernel,
Benson Leung, Jameson Thies, stable
[-- Attachment #1: Type: text/plain, Size: 4531 bytes --]
Hi Andrei,
On Fri, Mar 21, 2025 at 02:37:26PM +0000, Andrei Kuchynski wrote:
> Concurrent calls to typec_partner_unlink_device can lead to a NULL pointer
> dereference. This patch adds a mutex to protect USB device pointers and
> prevent this issue. The same mutex protects both the device pointers and
> the partner device registration.
>
> Cc: stable@vger.kernel.org
> Fixes: 59de2a56d127 ("usb: typec: Link enumerated USB devices with Type-C partner")
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
Reviewed-by: Benson Leung <bleung@chromium.org>
> ---
> drivers/usb/typec/class.c | 15 +++++++++++++--
> drivers/usb/typec/class.h | 1 +
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 9c76c3d0c6cf..eadb150223f8 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -1052,6 +1052,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
> partner->usb_mode = USB_MODE_USB3;
> }
>
> + mutex_lock(&port->partner_link_lock);
> ret = device_register(&partner->dev);
> if (ret) {
> dev_err(&port->dev, "failed to register partner (%d)\n", ret);
> @@ -1063,6 +1064,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
> typec_partner_link_device(partner, port->usb2_dev);
> if (port->usb3_dev)
> typec_partner_link_device(partner, port->usb3_dev);
> + mutex_unlock(&port->partner_link_lock);
>
> return partner;
> }
> @@ -1083,12 +1085,14 @@ void typec_unregister_partner(struct typec_partner *partner)
>
> port = to_typec_port(partner->dev.parent);
>
> + mutex_lock(&port->partner_link_lock);
> if (port->usb2_dev)
> typec_partner_unlink_device(partner, port->usb2_dev);
> if (port->usb3_dev)
> typec_partner_unlink_device(partner, port->usb3_dev);
>
> device_unregister(&partner->dev);
> + mutex_unlock(&port->partner_link_lock);
> }
> EXPORT_SYMBOL_GPL(typec_unregister_partner);
>
> @@ -2041,10 +2045,11 @@ static struct typec_partner *typec_get_partner(struct typec_port *port)
> static void typec_partner_attach(struct typec_connector *con, struct device *dev)
> {
> struct typec_port *port = container_of(con, struct typec_port, con);
> - struct typec_partner *partner = typec_get_partner(port);
> + struct typec_partner *partner;
> struct usb_device *udev = to_usb_device(dev);
> enum usb_mode usb_mode;
>
> + mutex_lock(&port->partner_link_lock);
> if (udev->speed < USB_SPEED_SUPER) {
> usb_mode = USB_MODE_USB2;
> port->usb2_dev = dev;
> @@ -2053,18 +2058,22 @@ static void typec_partner_attach(struct typec_connector *con, struct device *dev
> port->usb3_dev = dev;
> }
>
> + partner = typec_get_partner(port);
> if (partner) {
> typec_partner_set_usb_mode(partner, usb_mode);
> typec_partner_link_device(partner, dev);
> put_device(&partner->dev);
> }
> + mutex_unlock(&port->partner_link_lock);
> }
>
> static void typec_partner_deattach(struct typec_connector *con, struct device *dev)
> {
> struct typec_port *port = container_of(con, struct typec_port, con);
> - struct typec_partner *partner = typec_get_partner(port);
> + struct typec_partner *partner;
>
> + mutex_lock(&port->partner_link_lock);
> + partner = typec_get_partner(port);
> if (partner) {
> typec_partner_unlink_device(partner, dev);
> put_device(&partner->dev);
> @@ -2074,6 +2083,7 @@ static void typec_partner_deattach(struct typec_connector *con, struct device *d
> port->usb2_dev = NULL;
> else if (port->usb3_dev == dev)
> port->usb3_dev = NULL;
> + mutex_unlock(&port->partner_link_lock);
> }
>
> /**
> @@ -2614,6 +2624,7 @@ struct typec_port *typec_register_port(struct device *parent,
>
> ida_init(&port->mode_ids);
> mutex_init(&port->port_type_lock);
> + mutex_init(&port->partner_link_lock);
>
> port->id = id;
> port->ops = cap->ops;
> diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> index b3076a24ad2e..db2fe96c48ff 100644
> --- a/drivers/usb/typec/class.h
> +++ b/drivers/usb/typec/class.h
> @@ -59,6 +59,7 @@ struct typec_port {
> enum typec_port_type port_type;
> enum usb_mode usb_mode;
> struct mutex port_type_lock;
> + struct mutex partner_link_lock;
>
> enum typec_orientation orientation;
> struct typec_switch *sw;
> --
> 2.49.0.395.g12beb8f557-goog
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] usb: typec: class: Invalidate USB device pointers on partner unregistration
2025-03-21 14:37 ` [PATCH 2/2] usb: typec: class: Invalidate USB device pointers on partner unregistration Andrei Kuchynski
@ 2025-03-21 23:19 ` Benson Leung
2025-03-24 9:43 ` Heikki Krogerus
1 sibling, 0 replies; 7+ messages in thread
From: Benson Leung @ 2025-03-21 23:19 UTC (permalink / raw)
To: Andrei Kuchynski
Cc: Heikki Krogerus, Greg Kroah-Hartman, linux-usb, linux-kernel,
Benson Leung, Jameson Thies, stable
[-- Attachment #1: Type: text/plain, Size: 1415 bytes --]
Hi Andrei,
On Fri, Mar 21, 2025 at 02:37:27PM +0000, Andrei Kuchynski wrote:
> To avoid using invalid USB device pointers after a Type-C partner
> disconnects, this patch clears the pointers upon partner unregistration.
> This ensures a clean state for future connections.
>
> Cc: stable@vger.kernel.org
> Fixes: 59de2a56d127 ("usb: typec: Link enumerated USB devices with Type-C partner")
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
Reviewed-by: Benson Leung <bleung@chromium.org>
> ---
> drivers/usb/typec/class.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index eadb150223f8..3df3e3736916 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -1086,10 +1086,14 @@ void typec_unregister_partner(struct typec_partner *partner)
> port = to_typec_port(partner->dev.parent);
>
> mutex_lock(&port->partner_link_lock);
> - if (port->usb2_dev)
> + if (port->usb2_dev) {
> typec_partner_unlink_device(partner, port->usb2_dev);
> - if (port->usb3_dev)
> + port->usb2_dev = NULL;
> + }
> + if (port->usb3_dev) {
> typec_partner_unlink_device(partner, port->usb3_dev);
> + port->usb3_dev = NULL;
> + }
>
> device_unregister(&partner->dev);
> mutex_unlock(&port->partner_link_lock);
> --
> 2.49.0.395.g12beb8f557-goog
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] usb: typec: class: Fix NULL pointer access
2025-03-21 14:37 ` [PATCH 1/2] usb: typec: class: Fix NULL " Andrei Kuchynski
2025-03-21 23:16 ` Benson Leung
@ 2025-03-24 9:42 ` Heikki Krogerus
1 sibling, 0 replies; 7+ messages in thread
From: Heikki Krogerus @ 2025-03-24 9:42 UTC (permalink / raw)
To: Andrei Kuchynski
Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Benson Leung,
Jameson Thies, stable
On Fri, Mar 21, 2025 at 02:37:26PM +0000, Andrei Kuchynski wrote:
> Concurrent calls to typec_partner_unlink_device can lead to a NULL pointer
> dereference. This patch adds a mutex to protect USB device pointers and
> prevent this issue. The same mutex protects both the device pointers and
> the partner device registration.
>
> Cc: stable@vger.kernel.org
> Fixes: 59de2a56d127 ("usb: typec: Link enumerated USB devices with Type-C partner")
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/class.c | 15 +++++++++++++--
> drivers/usb/typec/class.h | 1 +
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 9c76c3d0c6cf..eadb150223f8 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -1052,6 +1052,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
> partner->usb_mode = USB_MODE_USB3;
> }
>
> + mutex_lock(&port->partner_link_lock);
> ret = device_register(&partner->dev);
> if (ret) {
> dev_err(&port->dev, "failed to register partner (%d)\n", ret);
> @@ -1063,6 +1064,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
> typec_partner_link_device(partner, port->usb2_dev);
> if (port->usb3_dev)
> typec_partner_link_device(partner, port->usb3_dev);
> + mutex_unlock(&port->partner_link_lock);
>
> return partner;
> }
> @@ -1083,12 +1085,14 @@ void typec_unregister_partner(struct typec_partner *partner)
>
> port = to_typec_port(partner->dev.parent);
>
> + mutex_lock(&port->partner_link_lock);
> if (port->usb2_dev)
> typec_partner_unlink_device(partner, port->usb2_dev);
> if (port->usb3_dev)
> typec_partner_unlink_device(partner, port->usb3_dev);
>
> device_unregister(&partner->dev);
> + mutex_unlock(&port->partner_link_lock);
> }
> EXPORT_SYMBOL_GPL(typec_unregister_partner);
>
> @@ -2041,10 +2045,11 @@ static struct typec_partner *typec_get_partner(struct typec_port *port)
> static void typec_partner_attach(struct typec_connector *con, struct device *dev)
> {
> struct typec_port *port = container_of(con, struct typec_port, con);
> - struct typec_partner *partner = typec_get_partner(port);
> + struct typec_partner *partner;
> struct usb_device *udev = to_usb_device(dev);
> enum usb_mode usb_mode;
>
> + mutex_lock(&port->partner_link_lock);
> if (udev->speed < USB_SPEED_SUPER) {
> usb_mode = USB_MODE_USB2;
> port->usb2_dev = dev;
> @@ -2053,18 +2058,22 @@ static void typec_partner_attach(struct typec_connector *con, struct device *dev
> port->usb3_dev = dev;
> }
>
> + partner = typec_get_partner(port);
> if (partner) {
> typec_partner_set_usb_mode(partner, usb_mode);
> typec_partner_link_device(partner, dev);
> put_device(&partner->dev);
> }
> + mutex_unlock(&port->partner_link_lock);
> }
>
> static void typec_partner_deattach(struct typec_connector *con, struct device *dev)
> {
> struct typec_port *port = container_of(con, struct typec_port, con);
> - struct typec_partner *partner = typec_get_partner(port);
> + struct typec_partner *partner;
>
> + mutex_lock(&port->partner_link_lock);
> + partner = typec_get_partner(port);
> if (partner) {
> typec_partner_unlink_device(partner, dev);
> put_device(&partner->dev);
> @@ -2074,6 +2083,7 @@ static void typec_partner_deattach(struct typec_connector *con, struct device *d
> port->usb2_dev = NULL;
> else if (port->usb3_dev == dev)
> port->usb3_dev = NULL;
> + mutex_unlock(&port->partner_link_lock);
> }
>
> /**
> @@ -2614,6 +2624,7 @@ struct typec_port *typec_register_port(struct device *parent,
>
> ida_init(&port->mode_ids);
> mutex_init(&port->port_type_lock);
> + mutex_init(&port->partner_link_lock);
>
> port->id = id;
> port->ops = cap->ops;
> diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> index b3076a24ad2e..db2fe96c48ff 100644
> --- a/drivers/usb/typec/class.h
> +++ b/drivers/usb/typec/class.h
> @@ -59,6 +59,7 @@ struct typec_port {
> enum typec_port_type port_type;
> enum usb_mode usb_mode;
> struct mutex port_type_lock;
> + struct mutex partner_link_lock;
>
> enum typec_orientation orientation;
> struct typec_switch *sw;
> --
> 2.49.0.395.g12beb8f557-goog
--
heikki
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] usb: typec: class: Invalidate USB device pointers on partner unregistration
2025-03-21 14:37 ` [PATCH 2/2] usb: typec: class: Invalidate USB device pointers on partner unregistration Andrei Kuchynski
2025-03-21 23:19 ` Benson Leung
@ 2025-03-24 9:43 ` Heikki Krogerus
1 sibling, 0 replies; 7+ messages in thread
From: Heikki Krogerus @ 2025-03-24 9:43 UTC (permalink / raw)
To: Andrei Kuchynski
Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Benson Leung,
Jameson Thies, stable
On Fri, Mar 21, 2025 at 02:37:27PM +0000, Andrei Kuchynski wrote:
> To avoid using invalid USB device pointers after a Type-C partner
> disconnects, this patch clears the pointers upon partner unregistration.
> This ensures a clean state for future connections.
>
> Cc: stable@vger.kernel.org
> Fixes: 59de2a56d127 ("usb: typec: Link enumerated USB devices with Type-C partner")
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/class.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index eadb150223f8..3df3e3736916 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -1086,10 +1086,14 @@ void typec_unregister_partner(struct typec_partner *partner)
> port = to_typec_port(partner->dev.parent);
>
> mutex_lock(&port->partner_link_lock);
> - if (port->usb2_dev)
> + if (port->usb2_dev) {
> typec_partner_unlink_device(partner, port->usb2_dev);
> - if (port->usb3_dev)
> + port->usb2_dev = NULL;
> + }
> + if (port->usb3_dev) {
> typec_partner_unlink_device(partner, port->usb3_dev);
> + port->usb3_dev = NULL;
> + }
>
> device_unregister(&partner->dev);
> mutex_unlock(&port->partner_link_lock);
> --
> 2.49.0.395.g12beb8f557-goog
--
heikki
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-03-24 9:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-21 14:37 [PATCH 0/2] Fix invalid pointer access Andrei Kuchynski
2025-03-21 14:37 ` [PATCH 1/2] usb: typec: class: Fix NULL " Andrei Kuchynski
2025-03-21 23:16 ` Benson Leung
2025-03-24 9:42 ` Heikki Krogerus
2025-03-21 14:37 ` [PATCH 2/2] usb: typec: class: Invalidate USB device pointers on partner unregistration Andrei Kuchynski
2025-03-21 23:19 ` Benson Leung
2025-03-24 9:43 ` Heikki Krogerus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).