linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).