diff mbox series

[v2] Fix USB Type C hub crash in typec_altmode_update_active

Message ID alpine.DEB.2.21.2008070831040.7310@montezuma.home (mailing list archive)
State New, archived
Headers show
Series [v2] Fix USB Type C hub crash in typec_altmode_update_active | expand

Commit Message

Zwane Mwaikambo Aug. 7, 2020, 3:43 p.m. UTC 
This patch was posted previously on lkml. I've updated the patch to fix 
another memory overrun in ucsi_unregister_altmodes

The following crash occurs whilst plugging in a Thinkpad USB type C dock 
and appears to be affecting other users as seen below;

https://bbs.archlinux.org/viewtopic.php?id=256404
https://forums.linuxmint.com/viewtopic.php?p=1826251

My specific crash is;

[  565.427757] input: USBPS2 as 
/devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2.1/1-2.1.4/1-2.1.4:1.0/0003:0D3D:0001.0006/input/input31
[  565.452023] BUG: kernel NULL pointer dereference, address: 00000000000002fe
[  565.452025] #PF: supervisor read access in kernel mode
[  565.452026] #PF: error_code(0x0000) - not-present page
[  565.452026] PGD 0 P4D 0 
[  565.452028] Oops: 0000 [#1] PREEMPT SMP NOPTI
[  565.452030] CPU: 0 PID: 502 Comm: kworker/0:3 Not tainted 5.8.0-rc3+ #1
[  565.452031] Hardware name: LENOVO 20RD002VUS/20RD002VUS, BIOS R16ET25W (1.11 ) 04/21/2020
[  565.452034] Workqueue: events ucsi_handle_connector_change [typec_ucsi]
[  565.452039] RIP: 0010:typec_altmode_update_active+0x1f/0x100 [typec]
[  565.452040] Code: 0f 1f 84 00 00 00 00 00 0f 1f 00 0f 1f 44 00 00 55 48 
89 e5 41 54 53 48 83 ec 10 65 48 8b 04 25 28 00 00 00 48 89 45 e8 31 c0 
<0f> b6 87 fc 02 00 00 83 e0 01 40 38 f0 0f 84 95 00 00 00 48 8b 47
[  565.452041] RSP: 0018:ffffb729c066bdb0 EFLAGS: 00010246
[  565.452042] RAX: 0000000000000000 RBX: ffffa067c3e64a70 RCX: 0000000000000000
[  565.452043] RDX: ffffb729c066bd20 RSI: 0000000000000000 RDI: 0000000000000002
[  565.452044] RBP: ffffb729c066bdd0 R08: 00000083a7910a4f R09: 0000000000000000
[  565.452044] R10: ffffffffa106a220 R11: 0000000000000000 R12: 0000000000000000
[  565.452045] R13: ffffa067c3e64a98 R14: ffffa067c3e64810 R15: ffffa067c3e64800
[  565.452046] FS:  0000000000000000(0000) GS:ffffa067d1400000(0000) knlGS:0000000000000000
[  565.452047] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  565.452048] CR2: 00000000000002fe CR3: 00000001b060a003 CR4: 00000000003606f0
[  565.452048] Call Trace:
[  565.452052]  ucsi_altmode_update_active+0x83/0xd0 [typec_ucsi]
[  565.452054]  ucsi_handle_connector_change+0x1dc/0x320 [typec_ucsi]
[  565.452057]  process_one_work+0x1df/0x3d0
[  565.452059]  worker_thread+0x4d/0x3d0
[  565.452060]  ? process_one_work+0x3d0/0x3d0
[  565.452062]  kthread+0x127/0x170
[  565.452063]  ? kthread_park+0x90/0x90
[  565.452065]  ret_from_fork+0x1f/0x30

The failing instruction is;

0x0000000000000380 <+0>:     callq  0x385 <typec_altmode_update_active+5>
0x0000000000000385 <+5>:     push   %rbp
0x0000000000000386 <+6>:     mov    %rsp,%rbp
0x0000000000000389 <+9>:     push   %r12
0x000000000000038b <+11>:    push   %rbx
0x000000000000038c <+12>:    sub    $0x10,%rsp
0x0000000000000390 <+16>:    mov    %gs:0x28,%rax
0x0000000000000399 <+25>:    mov    %rax,-0x18(%rbp)
0x000000000000039d <+29>:    xor    %eax,%eax
0x000000000000039f <+31>:    movzbl 0x2fc(%rdi),%eax <======
0x00000000000003a6 <+38>:    and    $0x1,%eax

(gdb) list  *typec_altmode_update_active+0x1f
0x39f is in typec_altmode_update_active (drivers/usb/typec/class.c:221).
216      */
217     void typec_altmode_update_active(struct typec_altmode *adev, bool active)
218     {
219             char dir[6];
220
221             if (adev->active == active)
222                     return;
223
224             if (!is_typec_port(adev->dev.parent) && adev->dev.driver) {
225                     if (!active)

(gdb) list *ucsi_altmode_update_active+0x83
0x12a3 is in ucsi_altmode_update_active (drivers/usb/typec/ucsi/ucsi.c:221).
216             }
217
218             if (cur < UCSI_MAX_ALTMODES)
219                     altmode = typec_altmode_get_partner(con->port_altmode[cur]);
220
221             for (i = 0; con->partner_altmode[i]; i++)
222                     typec_altmode_update_active(con->partner_altmode[i],
223                                                con->partner_altmode[i] == altmode);
224     }

con->partner_altmode[i] ends up with the value 0x2 in the call and it's 
because the array has been accessed out of bounds. There is also another 
occurence of this which shows up as the following crash;

[ 4372.429633] usb 1-2.1.4: USB disconnect, device number 16
[ 4372.523235] usb 1-2.2: USB disconnect, device number 7
[ 4372.570537] usb 1-2.5: USB disconnect, device number 9
[ 4373.153246] BUG: kernel NULL pointer dereference, address: 00000000000002f2
[ 4373.153267] #PF: supervisor read access in kernel mode
[ 4373.153271] #PF: error_code(0x0000) - not-present page
[ 4373.153275] PGD 0 P4D 0 
[ 4373.153284] Oops: 0000 [#2] PREEMPT SMP NOPTI
[ 4373.153292] CPU: 0 PID: 13242 Comm: kworker/0:0 Tainted: G      D           5.8.0-rc6+ #1
[ 4373.153296] Hardware name: LENOVO 20RD002VUS/20RD002VUS, BIOS R16ET25W (1.11 ) 04/21/2020
[ 4373.153308] Workqueue: events ucsi_handle_connector_change [typec_ucsi]
[ 4373.153320] RIP: 0010:ucsi_unregister_altmodes+0x5f/0xa0 [typec_ucsi]
[ 4373.153326] Code: 54 48 8b 3b 41 83 c4 01 e8 9e f9 0c 00 49 63 c4 48 c7 
03 00 00 00 00 49 8d 5c c5 00 48 8b 3b 48 85 ff 74 31 41 80 fe 01 75 d7 
<0f> b7 87 f0 02 00 00 66 3d 01 ff 74 0f 66 3d 55 09 75 c4 83 bf f8
[ 4373.153332] RSP: 0018:ffffb2ef036b3dc8 EFLAGS: 00010246
[ 4373.153338] RAX: 000000000000001e RBX: ffff94268b006a60 RCX: 0000000080800067
[ 4373.153342] RDX: 0000000080800068 RSI: 0000000000000001 RDI: 0000000000000002
[ 4373.153347] RBP: ffffb2ef036b3de8 R08: 0000000000000000 R09: ffffffff8dc65400
[ 4373.153351] R10: ffff9426678d7200 R11: 0000000000000001 R12: 000000000000001e
[ 4373.153355] R13: ffff94268b006970 R14: 0000000000000001 R15: ffff94268b006800
[ 4373.153361] FS:  0000000000000000(0000) GS:ffff942691400000(0000) knlGS:0000000000000000
[ 4373.153366] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 4373.153371] CR2: 00000000000002f2 CR3: 00000004445a6005 CR4: 00000000003606f0
[ 4373.153375] Call Trace:
[ 4373.153389]  ucsi_unregister_partner.part.0+0x17/0x30 [typec_ucsi]
[ 4373.153400]  ucsi_handle_connector_change+0x25c/0x320 [typec_ucsi]
[ 4373.153418]  process_one_work+0x1df/0x3d0
[ 4373.153428]  worker_thread+0x4a/0x3d0
[ 4373.153436]  ? process_one_work+0x3d0/0x3d0
[ 4373.153444]  kthread+0x127/0x170
[ 4373.153451]  ? kthread_park+0x90/0x90
[ 4373.153461]  ret_from_fork+0x1f/0x30
[ 4373.153661] CR2: 00000000000002f2

Comments

Heikki Krogerus Aug. 13, 2020, 11:20 a.m. UTC | #1 
Hi,

On Fri, Aug 07, 2020 at 08:43:15AM -0700, Zwane Mwaikambo wrote:
> This patch was posted previously on lkml. I've updated the patch to fix 
> another memory overrun in ucsi_unregister_altmodes

Could you please give a link to the thread in lkml that you are
referring to (from here: https://lore.kernel.org/lkml/)?

This is not formated as a patch. It's not signed and the subject is
not appropriate. Is this meant to be a bug report?

If this is meant to be a patch, please prepare it according to the
rules that are explained for example here:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html

thanks,

> The following crash occurs whilst plugging in a Thinkpad USB type C dock 
> and appears to be affecting other users as seen below;
> 
> https://bbs.archlinux.org/viewtopic.php?id=256404
> https://forums.linuxmint.com/viewtopic.php?p=1826251
> 
> My specific crash is;
> 
> [  565.427757] input: USBPS2 as 
> /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2.1/1-2.1.4/1-2.1.4:1.0/0003:0D3D:0001.0006/input/input31
> [  565.452023] BUG: kernel NULL pointer dereference, address: 00000000000002fe
> [  565.452025] #PF: supervisor read access in kernel mode
> [  565.452026] #PF: error_code(0x0000) - not-present page
> [  565.452026] PGD 0 P4D 0 
> [  565.452028] Oops: 0000 [#1] PREEMPT SMP NOPTI
> [  565.452030] CPU: 0 PID: 502 Comm: kworker/0:3 Not tainted 5.8.0-rc3+ #1
> [  565.452031] Hardware name: LENOVO 20RD002VUS/20RD002VUS, BIOS R16ET25W (1.11 ) 04/21/2020
> [  565.452034] Workqueue: events ucsi_handle_connector_change [typec_ucsi]
> [  565.452039] RIP: 0010:typec_altmode_update_active+0x1f/0x100 [typec]
> [  565.452040] Code: 0f 1f 84 00 00 00 00 00 0f 1f 00 0f 1f 44 00 00 55 48 
> 89 e5 41 54 53 48 83 ec 10 65 48 8b 04 25 28 00 00 00 48 89 45 e8 31 c0 
> <0f> b6 87 fc 02 00 00 83 e0 01 40 38 f0 0f 84 95 00 00 00 48 8b 47
> [  565.452041] RSP: 0018:ffffb729c066bdb0 EFLAGS: 00010246
> [  565.452042] RAX: 0000000000000000 RBX: ffffa067c3e64a70 RCX: 0000000000000000
> [  565.452043] RDX: ffffb729c066bd20 RSI: 0000000000000000 RDI: 0000000000000002
> [  565.452044] RBP: ffffb729c066bdd0 R08: 00000083a7910a4f R09: 0000000000000000
> [  565.452044] R10: ffffffffa106a220 R11: 0000000000000000 R12: 0000000000000000
> [  565.452045] R13: ffffa067c3e64a98 R14: ffffa067c3e64810 R15: ffffa067c3e64800
> [  565.452046] FS:  0000000000000000(0000) GS:ffffa067d1400000(0000) knlGS:0000000000000000
> [  565.452047] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  565.452048] CR2: 00000000000002fe CR3: 00000001b060a003 CR4: 00000000003606f0
> [  565.452048] Call Trace:
> [  565.452052]  ucsi_altmode_update_active+0x83/0xd0 [typec_ucsi]
> [  565.452054]  ucsi_handle_connector_change+0x1dc/0x320 [typec_ucsi]
> [  565.452057]  process_one_work+0x1df/0x3d0
> [  565.452059]  worker_thread+0x4d/0x3d0
> [  565.452060]  ? process_one_work+0x3d0/0x3d0
> [  565.452062]  kthread+0x127/0x170
> [  565.452063]  ? kthread_park+0x90/0x90
> [  565.452065]  ret_from_fork+0x1f/0x30
> 
> The failing instruction is;
> 
> 0x0000000000000380 <+0>:     callq  0x385 <typec_altmode_update_active+5>
> 0x0000000000000385 <+5>:     push   %rbp
> 0x0000000000000386 <+6>:     mov    %rsp,%rbp
> 0x0000000000000389 <+9>:     push   %r12
> 0x000000000000038b <+11>:    push   %rbx
> 0x000000000000038c <+12>:    sub    $0x10,%rsp
> 0x0000000000000390 <+16>:    mov    %gs:0x28,%rax
> 0x0000000000000399 <+25>:    mov    %rax,-0x18(%rbp)
> 0x000000000000039d <+29>:    xor    %eax,%eax
> 0x000000000000039f <+31>:    movzbl 0x2fc(%rdi),%eax <======
> 0x00000000000003a6 <+38>:    and    $0x1,%eax
> 
> (gdb) list  *typec_altmode_update_active+0x1f
> 0x39f is in typec_altmode_update_active (drivers/usb/typec/class.c:221).
> 216      */
> 217     void typec_altmode_update_active(struct typec_altmode *adev, bool active)
> 218     {
> 219             char dir[6];
> 220
> 221             if (adev->active == active)
> 222                     return;
> 223
> 224             if (!is_typec_port(adev->dev.parent) && adev->dev.driver) {
> 225                     if (!active)
> 
> (gdb) list *ucsi_altmode_update_active+0x83
> 0x12a3 is in ucsi_altmode_update_active (drivers/usb/typec/ucsi/ucsi.c:221).
> 216             }
> 217
> 218             if (cur < UCSI_MAX_ALTMODES)
> 219                     altmode = typec_altmode_get_partner(con->port_altmode[cur]);
> 220
> 221             for (i = 0; con->partner_altmode[i]; i++)
> 222                     typec_altmode_update_active(con->partner_altmode[i],
> 223                                                con->partner_altmode[i] == altmode);
> 224     }
> 
> con->partner_altmode[i] ends up with the value 0x2 in the call and it's 
> because the array has been accessed out of bounds. There is also another 
> occurence of this which shows up as the following crash;
> 
> [ 4372.429633] usb 1-2.1.4: USB disconnect, device number 16
> [ 4372.523235] usb 1-2.2: USB disconnect, device number 7
> [ 4372.570537] usb 1-2.5: USB disconnect, device number 9
> [ 4373.153246] BUG: kernel NULL pointer dereference, address: 00000000000002f2
> [ 4373.153267] #PF: supervisor read access in kernel mode
> [ 4373.153271] #PF: error_code(0x0000) - not-present page
> [ 4373.153275] PGD 0 P4D 0 
> [ 4373.153284] Oops: 0000 [#2] PREEMPT SMP NOPTI
> [ 4373.153292] CPU: 0 PID: 13242 Comm: kworker/0:0 Tainted: G      D           5.8.0-rc6+ #1
> [ 4373.153296] Hardware name: LENOVO 20RD002VUS/20RD002VUS, BIOS R16ET25W (1.11 ) 04/21/2020
> [ 4373.153308] Workqueue: events ucsi_handle_connector_change [typec_ucsi]
> [ 4373.153320] RIP: 0010:ucsi_unregister_altmodes+0x5f/0xa0 [typec_ucsi]
> [ 4373.153326] Code: 54 48 8b 3b 41 83 c4 01 e8 9e f9 0c 00 49 63 c4 48 c7 
> 03 00 00 00 00 49 8d 5c c5 00 48 8b 3b 48 85 ff 74 31 41 80 fe 01 75 d7 
> <0f> b7 87 f0 02 00 00 66 3d 01 ff 74 0f 66 3d 55 09 75 c4 83 bf f8
> [ 4373.153332] RSP: 0018:ffffb2ef036b3dc8 EFLAGS: 00010246
> [ 4373.153338] RAX: 000000000000001e RBX: ffff94268b006a60 RCX: 0000000080800067
> [ 4373.153342] RDX: 0000000080800068 RSI: 0000000000000001 RDI: 0000000000000002
> [ 4373.153347] RBP: ffffb2ef036b3de8 R08: 0000000000000000 R09: ffffffff8dc65400
> [ 4373.153351] R10: ffff9426678d7200 R11: 0000000000000001 R12: 000000000000001e
> [ 4373.153355] R13: ffff94268b006970 R14: 0000000000000001 R15: ffff94268b006800
> [ 4373.153361] FS:  0000000000000000(0000) GS:ffff942691400000(0000) knlGS:0000000000000000
> [ 4373.153366] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 4373.153371] CR2: 00000000000002f2 CR3: 00000004445a6005 CR4: 00000000003606f0
> [ 4373.153375] Call Trace:
> [ 4373.153389]  ucsi_unregister_partner.part.0+0x17/0x30 [typec_ucsi]
> [ 4373.153400]  ucsi_handle_connector_change+0x25c/0x320 [typec_ucsi]
> [ 4373.153418]  process_one_work+0x1df/0x3d0
> [ 4373.153428]  worker_thread+0x4a/0x3d0
> [ 4373.153436]  ? process_one_work+0x3d0/0x3d0
> [ 4373.153444]  kthread+0x127/0x170
> [ 4373.153451]  ? kthread_park+0x90/0x90
> [ 4373.153461]  ret_from_fork+0x1f/0x30
> [ 4373.153661] CR2: 00000000000002f2
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index d0c63afaf..79061705e 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -218,9 +218,10 @@ void ucsi_altmode_update_active(struct ucsi_connector *con)
>  	if (cur < UCSI_MAX_ALTMODES)
>  		altmode = typec_altmode_get_partner(con->port_altmode[cur]);
>  
> -	for (i = 0; con->partner_altmode[i]; i++)
> -		typec_altmode_update_active(con->partner_altmode[i],
> -					    con->partner_altmode[i] == altmode);
> +	for (i = 0; i < UCSI_MAX_ALTMODES; i++)
> +		if (con->partner_altmode[i])
> +			typec_altmode_update_active(con->partner_altmode[i],
> +										con->partner_altmode[i] == altmode);
>  }
>  
>  static u8 ucsi_altmode_next_mode(struct typec_altmode **alt, u16 svid)
> @@ -479,7 +480,10 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient)
>  		return;
>  	}
>  
> -	while (adev[i]) {
> +	for (i = 0; i < UCSI_MAX_ALTMODES; i++) {
> +		if (!adev[i])
> +			break;
> +
>  		if (recipient == UCSI_RECIPIENT_SOP &&
>  		    (adev[i]->svid == USB_TYPEC_DP_SID ||
>  			(adev[i]->svid == USB_TYPEC_NVIDIA_VLINK_SID &&
> @@ -488,7 +492,7 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient)
>  			ucsi_displayport_remove_partner((void *)pdev);
>  		}
>  		typec_unregister_altmode(adev[i]);
> -		adev[i++] = NULL;
> +		adev[i] = NULL;
>  	}
>  }
>
Zwane Mwaikambo Aug. 14, 2020, 1:39 a.m. UTC | #2 
On Thu, Aug 13, 2020 at 4:20 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
> On Fri, Aug 07, 2020 at 08:43:15AM -0700, Zwane Mwaikambo wrote:
> > This patch was posted previously on lkml. I've updated the patch to fix
> > another memory overrun in ucsi_unregister_altmodes
>
> Could you please give a link to the thread in lkml that you are
> referring to (from here: https://lore.kernel.org/lkml/)?
>
> This is not formated as a patch. It's not signed and the subject is
> not appropriate. Is this meant to be a bug report?
>
> If this is meant to be a patch, please prepare it according to the
> rules that are explained for example here:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html

My bad, the original was on lkml only and can be seen here;
https://lkml.org/lkml/2020/7/26/24

I'll resubmit it with the proper formatting etc.

Thanks,
      Zwane

> > The following crash occurs whilst plugging in a Thinkpad USB type C dock
> > and appears to be affecting other users as seen below;
> >
> > https://bbs.archlinux.org/viewtopic.php?id=256404
> > https://forums.linuxmint.com/viewtopic.php?p=1826251
> >
> > My specific crash is;
> >
> > [  565.427757] input: USBPS2 as
> > /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2.1/1-2.1.4/1-2.1.4:1.0/0003:0D3D:0001.0006/input/input31
> > [  565.452023] BUG: kernel NULL pointer dereference, address: 00000000000002fe
> > [  565.452025] #PF: supervisor read access in kernel mode
> > [  565.452026] #PF: error_code(0x0000) - not-present page
> > [  565.452026] PGD 0 P4D 0
> > [  565.452028] Oops: 0000 [#1] PREEMPT SMP NOPTI
> > [  565.452030] CPU: 0 PID: 502 Comm: kworker/0:3 Not tainted 5.8.0-rc3+ #1
> > [  565.452031] Hardware name: LENOVO 20RD002VUS/20RD002VUS, BIOS R16ET25W (1.11 ) 04/21/2020
> > [  565.452034] Workqueue: events ucsi_handle_connector_change [typec_ucsi]
> > [  565.452039] RIP: 0010:typec_altmode_update_active+0x1f/0x100 [typec]
> > [  565.452040] Code: 0f 1f 84 00 00 00 00 00 0f 1f 00 0f 1f 44 00 00 55 48
> > 89 e5 41 54 53 48 83 ec 10 65 48 8b 04 25 28 00 00 00 48 89 45 e8 31 c0
> > <0f> b6 87 fc 02 00 00 83 e0 01 40 38 f0 0f 84 95 00 00 00 48 8b 47
> > [  565.452041] RSP: 0018:ffffb729c066bdb0 EFLAGS: 00010246
> > [  565.452042] RAX: 0000000000000000 RBX: ffffa067c3e64a70 RCX: 0000000000000000
> > [  565.452043] RDX: ffffb729c066bd20 RSI: 0000000000000000 RDI: 0000000000000002
> > [  565.452044] RBP: ffffb729c066bdd0 R08: 00000083a7910a4f R09: 0000000000000000
> > [  565.452044] R10: ffffffffa106a220 R11: 0000000000000000 R12: 0000000000000000
> > [  565.452045] R13: ffffa067c3e64a98 R14: ffffa067c3e64810 R15: ffffa067c3e64800
> > [  565.452046] FS:  0000000000000000(0000) GS:ffffa067d1400000(0000) knlGS:0000000000000000
> > [  565.452047] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [  565.452048] CR2: 00000000000002fe CR3: 00000001b060a003 CR4: 00000000003606f0
> > [  565.452048] Call Trace:
> > [  565.452052]  ucsi_altmode_update_active+0x83/0xd0 [typec_ucsi]
> > [  565.452054]  ucsi_handle_connector_change+0x1dc/0x320 [typec_ucsi]
> > [  565.452057]  process_one_work+0x1df/0x3d0
> > [  565.452059]  worker_thread+0x4d/0x3d0
> > [  565.452060]  ? process_one_work+0x3d0/0x3d0
> > [  565.452062]  kthread+0x127/0x170
> > [  565.452063]  ? kthread_park+0x90/0x90
> > [  565.452065]  ret_from_fork+0x1f/0x30
> >
> > The failing instruction is;
> >
> > 0x0000000000000380 <+0>:     callq  0x385 <typec_altmode_update_active+5>
> > 0x0000000000000385 <+5>:     push   %rbp
> > 0x0000000000000386 <+6>:     mov    %rsp,%rbp
> > 0x0000000000000389 <+9>:     push   %r12
> > 0x000000000000038b <+11>:    push   %rbx
> > 0x000000000000038c <+12>:    sub    $0x10,%rsp
> > 0x0000000000000390 <+16>:    mov    %gs:0x28,%rax
> > 0x0000000000000399 <+25>:    mov    %rax,-0x18(%rbp)
> > 0x000000000000039d <+29>:    xor    %eax,%eax
> > 0x000000000000039f <+31>:    movzbl 0x2fc(%rdi),%eax <======
> > 0x00000000000003a6 <+38>:    and    $0x1,%eax
> >
> > (gdb) list  *typec_altmode_update_active+0x1f
> > 0x39f is in typec_altmode_update_active (drivers/usb/typec/class.c:221).
> > 216      */
> > 217     void typec_altmode_update_active(struct typec_altmode *adev, bool active)
> > 218     {
> > 219             char dir[6];
> > 220
> > 221             if (adev->active == active)
> > 222                     return;
> > 223
> > 224             if (!is_typec_port(adev->dev.parent) && adev->dev.driver) {
> > 225                     if (!active)
> >
> > (gdb) list *ucsi_altmode_update_active+0x83
> > 0x12a3 is in ucsi_altmode_update_active (drivers/usb/typec/ucsi/ucsi.c:221).
> > 216             }
> > 217
> > 218             if (cur < UCSI_MAX_ALTMODES)
> > 219                     altmode = typec_altmode_get_partner(con->port_altmode[cur]);
> > 220
> > 221             for (i = 0; con->partner_altmode[i]; i++)
> > 222                     typec_altmode_update_active(con->partner_altmode[i],
> > 223                                                con->partner_altmode[i] == altmode);
> > 224     }
> >
> > con->partner_altmode[i] ends up with the value 0x2 in the call and it's
> > because the array has been accessed out of bounds. There is also another
> > occurence of this which shows up as the following crash;
> >
> > [ 4372.429633] usb 1-2.1.4: USB disconnect, device number 16
> > [ 4372.523235] usb 1-2.2: USB disconnect, device number 7
> > [ 4372.570537] usb 1-2.5: USB disconnect, device number 9
> > [ 4373.153246] BUG: kernel NULL pointer dereference, address: 00000000000002f2
> > [ 4373.153267] #PF: supervisor read access in kernel mode
> > [ 4373.153271] #PF: error_code(0x0000) - not-present page
> > [ 4373.153275] PGD 0 P4D 0
> > [ 4373.153284] Oops: 0000 [#2] PREEMPT SMP NOPTI
> > [ 4373.153292] CPU: 0 PID: 13242 Comm: kworker/0:0 Tainted: G      D           5.8.0-rc6+ #1
> > [ 4373.153296] Hardware name: LENOVO 20RD002VUS/20RD002VUS, BIOS R16ET25W (1.11 ) 04/21/2020
> > [ 4373.153308] Workqueue: events ucsi_handle_connector_change [typec_ucsi]
> > [ 4373.153320] RIP: 0010:ucsi_unregister_altmodes+0x5f/0xa0 [typec_ucsi]
> > [ 4373.153326] Code: 54 48 8b 3b 41 83 c4 01 e8 9e f9 0c 00 49 63 c4 48 c7
> > 03 00 00 00 00 49 8d 5c c5 00 48 8b 3b 48 85 ff 74 31 41 80 fe 01 75 d7
> > <0f> b7 87 f0 02 00 00 66 3d 01 ff 74 0f 66 3d 55 09 75 c4 83 bf f8
> > [ 4373.153332] RSP: 0018:ffffb2ef036b3dc8 EFLAGS: 00010246
> > [ 4373.153338] RAX: 000000000000001e RBX: ffff94268b006a60 RCX: 0000000080800067
> > [ 4373.153342] RDX: 0000000080800068 RSI: 0000000000000001 RDI: 0000000000000002
> > [ 4373.153347] RBP: ffffb2ef036b3de8 R08: 0000000000000000 R09: ffffffff8dc65400
> > [ 4373.153351] R10: ffff9426678d7200 R11: 0000000000000001 R12: 000000000000001e
> > [ 4373.153355] R13: ffff94268b006970 R14: 0000000000000001 R15: ffff94268b006800
> > [ 4373.153361] FS:  0000000000000000(0000) GS:ffff942691400000(0000) knlGS:0000000000000000
> > [ 4373.153366] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 4373.153371] CR2: 00000000000002f2 CR3: 00000004445a6005 CR4: 00000000003606f0
> > [ 4373.153375] Call Trace:
> > [ 4373.153389]  ucsi_unregister_partner.part.0+0x17/0x30 [typec_ucsi]
> > [ 4373.153400]  ucsi_handle_connector_change+0x25c/0x320 [typec_ucsi]
> > [ 4373.153418]  process_one_work+0x1df/0x3d0
> > [ 4373.153428]  worker_thread+0x4a/0x3d0
> > [ 4373.153436]  ? process_one_work+0x3d0/0x3d0
> > [ 4373.153444]  kthread+0x127/0x170
> > [ 4373.153451]  ? kthread_park+0x90/0x90
> > [ 4373.153461]  ret_from_fork+0x1f/0x30
> > [ 4373.153661] CR2: 00000000000002f2
> >
> > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> > index d0c63afaf..79061705e 100644
> > --- a/drivers/usb/typec/ucsi/ucsi.c
> > +++ b/drivers/usb/typec/ucsi/ucsi.c
> > @@ -218,9 +218,10 @@ void ucsi_altmode_update_active(struct ucsi_connector *con)
> >       if (cur < UCSI_MAX_ALTMODES)
> >               altmode = typec_altmode_get_partner(con->port_altmode[cur]);
> >
> > -     for (i = 0; con->partner_altmode[i]; i++)
> > -             typec_altmode_update_active(con->partner_altmode[i],
> > -                                         con->partner_altmode[i] == altmode);
> > +     for (i = 0; i < UCSI_MAX_ALTMODES; i++)
> > +             if (con->partner_altmode[i])
> > +                     typec_altmode_update_active(con->partner_altmode[i],
> > +                                                                             con->partner_altmode[i] == altmode);
> >  }
> >
> >  static u8 ucsi_altmode_next_mode(struct typec_altmode **alt, u16 svid)
> > @@ -479,7 +480,10 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient)
> >               return;
> >       }
> >
> > -     while (adev[i]) {
> > +     for (i = 0; i < UCSI_MAX_ALTMODES; i++) {
> > +             if (!adev[i])
> > +                     break;
> > +
> >               if (recipient == UCSI_RECIPIENT_SOP &&
> >                   (adev[i]->svid == USB_TYPEC_DP_SID ||
> >                       (adev[i]->svid == USB_TYPEC_NVIDIA_VLINK_SID &&
> > @@ -488,7 +492,7 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient)
> >                       ucsi_displayport_remove_partner((void *)pdev);
> >               }
> >               typec_unregister_altmode(adev[i]);
> > -             adev[i++] = NULL;
> > +             adev[i] = NULL;
> >       }
> >  }
> >
>
> --
> heikki
diff mbox series

Patch

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index d0c63afaf..79061705e 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -218,9 +218,10 @@  void ucsi_altmode_update_active(struct ucsi_connector *con)
 	if (cur < UCSI_MAX_ALTMODES)
 		altmode = typec_altmode_get_partner(con->port_altmode[cur]);
 
-	for (i = 0; con->partner_altmode[i]; i++)
-		typec_altmode_update_active(con->partner_altmode[i],
-					    con->partner_altmode[i] == altmode);
+	for (i = 0; i < UCSI_MAX_ALTMODES; i++)
+		if (con->partner_altmode[i])
+			typec_altmode_update_active(con->partner_altmode[i],
+										con->partner_altmode[i] == altmode);
 }
 
 static u8 ucsi_altmode_next_mode(struct typec_altmode **alt, u16 svid)
@@ -479,7 +480,10 @@  static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient)
 		return;
 	}
 
-	while (adev[i]) {
+	for (i = 0; i < UCSI_MAX_ALTMODES; i++) {
+		if (!adev[i])
+			break;
+
 		if (recipient == UCSI_RECIPIENT_SOP &&
 		    (adev[i]->svid == USB_TYPEC_DP_SID ||
 			(adev[i]->svid == USB_TYPEC_NVIDIA_VLINK_SID &&
@@ -488,7 +492,7 @@  static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient)
 			ucsi_displayport_remove_partner((void *)pdev);
 		}
 		typec_unregister_altmode(adev[i]);
-		adev[i++] = NULL;
+		adev[i] = NULL;
 	}
 }