Skip to contentSkip to conversation

usb: dwc3: core: fix kernel panic when do reboot

PATCH In review Peter Chen 5 participants 10 messages Jun 8, 2021 – Jun 11, 2021

Subsystems (2)
USB DesignWare USB3 DRD IP driver
lore
Share & tools
lore

Peter Chen

author
#0 lore raw

PATCH 1/1 usb: dwc3: core: fix kernel panic when do reboot

When do system reboot, it calls dwc3_shutdown and the whole debugfs for dwc3 has removed first, when the gadget tries to do deinit, and remove debugfs for its endpoints, it meets NULL pointer dereference issue when call debugfs_lookup. Fix it by removing the whole dwc3 debugfs later than dwc3_drd_exit.

[ 2924.958838] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000002
....
[ 2925.030994] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
[ 2925.037005] pc : inode_permission+0x2c/0x198
[ 2925.041281] lr : lookup_one_len_common+0xb0/0xf8
[ 2925.045903] sp : ffff80001276ba70
[ 2925.049218] x29: ffff80001276ba70 x28: ffff0000c01f0000 x27: 0000000000000000
[ 2925.056364] x26: ffff800011791e70 x25: 0000000000000008 x24: dead000000000100
[ 2925.063510] x23: dead000000000122 x22: 0000000000000000 x21: 0000000000000001
[ 2925.070652] x20: ffff8000122c6188 x19: 0000000000000000 x18: 0000000000000000
[ 2925.077797] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000004
[ 2925.084943] x14: ffffffffffffffff x13: 0000000000000000 x12: 0000000000000030
[ 2925.092087] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f x9 : ffff8000102b2420
[ 2925.099232] x8 : 7f7f7f7f7f7f7f7f x7 : feff73746e2f6f64 x6 : 0000000000008080
[ 2925.106378] x5 : 61c8864680b583eb x4 : 209e6ec2d263dbb7 x3 : 000074756f307065
[ 2925.113523] x2 : 0000000000000001 x1 : 0000000000000000 x0 : ffff8000122c6188
[ 2925.120671] Call trace:
[ 2925.123119]  inode_permission+0x2c/0x198
[ 2925.127042]  lookup_one_len_common+0xb0/0xf8
[ 2925.131315]  lookup_one_len_unlocked+0x34/0xb0
[ 2925.135764]  lookup_positive_unlocked+0x14/0x50
[ 2925.140296]  debugfs_lookup+0x68/0xa0
[ 2925.143964]  dwc3_gadget_free_endpoints+0x84/0xb0
[ 2925.148675]  dwc3_gadget_exit+0x28/0x78
[ 2925.152518]  dwc3_drd_exit+0x100/0x1f8
[ 2925.156267]  dwc3_remove+0x11c/0x120
[ 2925.159851]  dwc3_shutdown+0x14/0x20
[ 2925.163432]  platform_shutdown+0x28/0x38
[ 2925.167360]  device_shutdown+0x15c/0x378
[ 2925.171291]  kernel_restart_prepare+0x3c/0x48
[ 2925.175650]  kernel_restart+0x1c/0x68
[ 2925.179316]  __do_sys_reboot+0x218/0x240
[ 2925.183247]  __arm64_sys_reboot+0x28/0x30
[ 2925.187262]  invoke_syscall+0x48/0x100
[ 2925.191017]  el0_svc_common.constprop.0+0x48/0xc8
[ 2925.195726]  do_el0_svc+0x28/0x88
[ 2925.199045]  el0_svc+0x20/0x30
[ 2925.202104]  el0_sync_handler+0xa8/0xb0
[ 2925.205942]  el0_sync+0x148/0x180
[ 2925.209270] Code: a9025bf5 2a0203f5 121f0056 370802b5 (79400660)
[ 2925.215372] ---[ end trace 124254d8e485a58b ]---
[ 2925.220012] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[ 2925.227676] Kernel Offset: disabled
[ 2925.231164] CPU features: 0x00001001,20000846
[ 2925.235521] Memory Limit: none
[ 2925.238580] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
Cc Jack Pham <[email protected]>
Fixes 5ff90af9da8f ("usb: dwc3: debugfs: Add and remove endpoint dirs dynamically")
Signed-off-by Peter Chen <[email protected]>
---
 drivers/usb/dwc3/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/dwc3/core.c +1 -1
index b6e53d8212cd..5174031503ca 100644
@@ -1671,8 +1671,8 @@ static int dwc3_remove(struct platform_device *pdev)
 
 	pm_runtime_get_sync(&pdev->dev);
 
-	dwc3_debugfs_exit(dwc);
 	dwc3_core_exit_mode(dwc);
+	dwc3_debugfs_exit(dwc);
 
 	dwc3_core_exit(dwc);
 	dwc3_ulpi_exit(dwc);
Signature · 1 line
2.17.1
#1 lore raw
Quoted 6 lines · Peter Chen

When do system reboot, it calls dwc3_shutdown and the whole debugfs for dwc3 has removed first, when the gadget tries to do deinit, and remove debugfs for its endpoints, it meets NULL pointer dereference issue when call debugfs_lookup. Fix it by removing the whole dwc3 debugfs later than dwc3_drd_exit.

Usually we refer to the function() like this.

Quoted 2 lines
[ 2924.958838] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000002

....

It's a bad habit to provide very noisy tracebacks, please consider to shrink this to ~3-4 most significant lines.

Quoted 44 lines
[ 2925.030994] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
[ 2925.037005] pc : inode_permission+0x2c/0x198
[ 2925.041281] lr : lookup_one_len_common+0xb0/0xf8
[ 2925.045903] sp : ffff80001276ba70
[ 2925.049218] x29: ffff80001276ba70 x28: ffff0000c01f0000 x27: 0000000000000000
[ 2925.056364] x26: ffff800011791e70 x25: 0000000000000008 x24: dead000000000100
[ 2925.063510] x23: dead000000000122 x22: 0000000000000000 x21: 0000000000000001
[ 2925.070652] x20: ffff8000122c6188 x19: 0000000000000000 x18: 0000000000000000
[ 2925.077797] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000004
[ 2925.084943] x14: ffffffffffffffff x13: 0000000000000000 x12: 0000000000000030
[ 2925.092087] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f x9 : ffff8000102b2420
[ 2925.099232] x8 : 7f7f7f7f7f7f7f7f x7 : feff73746e2f6f64 x6 : 0000000000008080
[ 2925.106378] x5 : 61c8864680b583eb x4 : 209e6ec2d263dbb7 x3 : 000074756f307065
[ 2925.113523] x2 : 0000000000000001 x1 : 0000000000000000 x0 : ffff8000122c6188
[ 2925.120671] Call trace:
[ 2925.123119]  inode_permission+0x2c/0x198
[ 2925.127042]  lookup_one_len_common+0xb0/0xf8
[ 2925.131315]  lookup_one_len_unlocked+0x34/0xb0
[ 2925.135764]  lookup_positive_unlocked+0x14/0x50
[ 2925.140296]  debugfs_lookup+0x68/0xa0
[ 2925.143964]  dwc3_gadget_free_endpoints+0x84/0xb0
[ 2925.148675]  dwc3_gadget_exit+0x28/0x78
[ 2925.152518]  dwc3_drd_exit+0x100/0x1f8
[ 2925.156267]  dwc3_remove+0x11c/0x120
[ 2925.159851]  dwc3_shutdown+0x14/0x20
[ 2925.163432]  platform_shutdown+0x28/0x38
[ 2925.167360]  device_shutdown+0x15c/0x378
[ 2925.171291]  kernel_restart_prepare+0x3c/0x48
[ 2925.175650]  kernel_restart+0x1c/0x68
[ 2925.179316]  __do_sys_reboot+0x218/0x240
[ 2925.183247]  __arm64_sys_reboot+0x28/0x30
[ 2925.187262]  invoke_syscall+0x48/0x100
[ 2925.191017]  el0_svc_common.constprop.0+0x48/0xc8
[ 2925.195726]  do_el0_svc+0x28/0x88
[ 2925.199045]  el0_svc+0x20/0x30
[ 2925.202104]  el0_sync_handler+0xa8/0xb0
[ 2925.205942]  el0_sync+0x148/0x180
[ 2925.209270] Code: a9025bf5 2a0203f5 121f0056 370802b5 (79400660)
[ 2925.215372] ---[ end trace 124254d8e485a58b ]---
[ 2925.220012] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[ 2925.227676] Kernel Offset: disabled
[ 2925.231164] CPU features: 0x00001001,20000846
[ 2925.235521] Memory Limit: none
[ 2925.238580] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
Signature · 2 lines
With Best Regards,
Andy Shevchenko
Tested-by on v1 1/1
#2 lore raw
In reply to Peter Chen

Hi Peter,

Quoted 5 lines · Peter Chen

When do system reboot, it calls dwc3_shutdown and the whole debugfs for dwc3 has removed first, when the gadget tries to do deinit, and remove debugfs for its endpoints, it meets NULL pointer dereference issue when call debugfs_lookup. Fix it by removing the whole dwc3 debugfs later than dwc3_drd_exit.

Ouch, thanks for catching this! I think in your previous reply[1] you did warn about the debugfs_remove_recursive() getting called twice, but it seems here the issue is due to the debugfs_lookup() getting called on a stale dwc->root pointer after it was already removed.

Fortunately this is slightly mitigated due to the recent revert commit 8f11fe7e4068[2]. So at least we might avoid the problem on the shutdown path since remove won't get called. However it can still trigger if dwc3_remove() is called in another way (driver unbind, module removal).

[1] https://lore.kernel.org/linux-usb/20210601070744.GA9087@nchen/
[2] https://lore.kernel.org/r/[email protected]

Quoted 47 lines
[ 2924.958838] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000002
....
[ 2925.030994] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
[ 2925.037005] pc : inode_permission+0x2c/0x198
[ 2925.041281] lr : lookup_one_len_common+0xb0/0xf8
[ 2925.045903] sp : ffff80001276ba70
[ 2925.049218] x29: ffff80001276ba70 x28: ffff0000c01f0000 x27: 0000000000000000
[ 2925.056364] x26: ffff800011791e70 x25: 0000000000000008 x24: dead000000000100
[ 2925.063510] x23: dead000000000122 x22: 0000000000000000 x21: 0000000000000001
[ 2925.070652] x20: ffff8000122c6188 x19: 0000000000000000 x18: 0000000000000000
[ 2925.077797] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000004
[ 2925.084943] x14: ffffffffffffffff x13: 0000000000000000 x12: 0000000000000030
[ 2925.092087] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f x9 : ffff8000102b2420
[ 2925.099232] x8 : 7f7f7f7f7f7f7f7f x7 : feff73746e2f6f64 x6 : 0000000000008080
[ 2925.106378] x5 : 61c8864680b583eb x4 : 209e6ec2d263dbb7 x3 : 000074756f307065
[ 2925.113523] x2 : 0000000000000001 x1 : 0000000000000000 x0 : ffff8000122c6188
[ 2925.120671] Call trace:
[ 2925.123119]  inode_permission+0x2c/0x198
[ 2925.127042]  lookup_one_len_common+0xb0/0xf8
[ 2925.131315]  lookup_one_len_unlocked+0x34/0xb0
[ 2925.135764]  lookup_positive_unlocked+0x14/0x50
[ 2925.140296]  debugfs_lookup+0x68/0xa0
[ 2925.143964]  dwc3_gadget_free_endpoints+0x84/0xb0
[ 2925.148675]  dwc3_gadget_exit+0x28/0x78
[ 2925.152518]  dwc3_drd_exit+0x100/0x1f8
[ 2925.156267]  dwc3_remove+0x11c/0x120
[ 2925.159851]  dwc3_shutdown+0x14/0x20
[ 2925.163432]  platform_shutdown+0x28/0x38
[ 2925.167360]  device_shutdown+0x15c/0x378
[ 2925.171291]  kernel_restart_prepare+0x3c/0x48
[ 2925.175650]  kernel_restart+0x1c/0x68
[ 2925.179316]  __do_sys_reboot+0x218/0x240
[ 2925.183247]  __arm64_sys_reboot+0x28/0x30
[ 2925.187262]  invoke_syscall+0x48/0x100
[ 2925.191017]  el0_svc_common.constprop.0+0x48/0xc8
[ 2925.195726]  do_el0_svc+0x28/0x88
[ 2925.199045]  el0_svc+0x20/0x30
[ 2925.202104]  el0_sync_handler+0xa8/0xb0
[ 2925.205942]  el0_sync+0x148/0x180
[ 2925.209270] Code: a9025bf5 2a0203f5 121f0056 370802b5 (79400660)
[ 2925.215372] ---[ end trace 124254d8e485a58b ]---
[ 2925.220012] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[ 2925.227676] Kernel Offset: disabled
[ 2925.231164] CPU features: 0x00001001,20000846
[ 2925.235521] Memory Limit: none
[ 2925.238580] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
Quoted 3 lines
Cc Jack Pham <[email protected]>
Fixes 5ff90af9da8f ("usb: dwc3: debugfs: Add and remove endpoint dirs dynamically")
Signed-off-by Peter Chen <[email protected]>

You can also add

Tested-by Jack Pham <[email protected]>

Thanks again and sorry for the regression!
Jack

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
maintainer
#3 lore raw
Quoted 9 lines · Jack Pham

Hi Peter,

Quoted 5 lines · Peter Chen

When do system reboot, it calls dwc3_shutdown and the whole debugfs for dwc3 has removed first, when the gadget tries to do deinit, and remove debugfs for its endpoints, it meets NULL pointer dereference issue when call debugfs_lookup. Fix it by removing the whole dwc3 debugfs later than dwc3_drd_exit.

Quoted 4 lines

Ouch, thanks for catching this! I think in your previous reply[1] you did warn about the debugfs_remove_recursive() getting called twice, but it seems here the issue is due to the debugfs_lookup() getting called on a stale dwc->root pointer after it was already removed.

We can also fix this by getting rid of that "root" pointer as it's useless (we can look it up if we need it.) I'll send a patch later to do that, as it's a good idea to do anyway, and is independant of this fix.

thanks,

greg k-h

maintainer
#4 lore raw
Quoted 15 lines · Greg Kroah-Hartman
Quoted 9 lines · Jack Pham

Hi Peter,

Quoted 5 lines · Peter Chen

When do system reboot, it calls dwc3_shutdown and the whole debugfs for dwc3 has removed first, when the gadget tries to do deinit, and remove debugfs for its endpoints, it meets NULL pointer dereference issue when call debugfs_lookup. Fix it by removing the whole dwc3 debugfs later than dwc3_drd_exit.

Quoted 4 lines

Ouch, thanks for catching this! I think in your previous reply[1] you did warn about the debugfs_remove_recursive() getting called twice, but it seems here the issue is due to the debugfs_lookup() getting called on a stale dwc->root pointer after it was already removed.

Quoted 4 lines

We can also fix this by getting rid of that "root" pointer as it's useless (we can look it up if we need it.) I'll send a patch later to do that, as it's a good idea to do anyway, and is independant of this fix.

Now sent:

	https://lore.kernel.org/r/[email protected]
author
#5 lore raw
Quoted 16 lines · Greg Kroah-Hartman
Quoted 15 lines · Greg Kroah-Hartman
Quoted 13 lines · Jack Pham

Hi Peter,

Quoted 5 lines · Peter Chen

When do system reboot, it calls dwc3_shutdown and the whole debugfs for dwc3 has removed first, when the gadget tries to do deinit, and remove debugfs for its endpoints, it meets NULL pointer dereference issue when call debugfs_lookup. Fix it by removing the whole dwc3 debugfs later than dwc3_drd_exit.

Ouch, thanks for catching this! I think in your previous reply[1] you did warn about the debugfs_remove_recursive() getting called twice, but it seems here the issue is due to the debugfs_lookup() getting called on a stale dwc->root pointer after it was already removed.

Quoted 7 lines
Quoted 4 lines

We can also fix this by getting rid of that "root" pointer as it's useless (we can look it up if we need it.) I'll send a patch later to do that, as it's a good idea to do anyway, and is independant of this fix.

Now sent:

	https://lore.kernel.org/r/[email protected]

Hi Felipe,

With Greg above patch, this issue is fixed indirectly. I don't think my patch is needed, do you think so?

Signature · 2 lines
Thanks,
Peter Chen
maintainer
#6 lore raw
Quoted 22 lines · Peter Chen
Quoted 21 lines · Greg Kroah-Hartman
Quoted 19 lines · Greg Kroah-Hartman
Quoted 9 lines · Jack Pham

Hi Peter,

Quoted 5 lines · Peter Chen

When do system reboot, it calls dwc3_shutdown and the whole debugfs for dwc3 has removed first, when the gadget tries to do deinit, and remove debugfs for its endpoints, it meets NULL pointer dereference issue when call debugfs_lookup. Fix it by removing the whole dwc3 debugfs later than dwc3_drd_exit.

Quoted 4 lines

Ouch, thanks for catching this! I think in your previous reply[1] you did warn about the debugfs_remove_recursive() getting called twice, but it seems here the issue is due to the debugfs_lookup() getting called on a stale dwc->root pointer after it was already removed.

We can also fix this by getting rid of that "root" pointer as it's useless (we can look it up if we need it.) I'll send a patch later to do that, as it's a good idea to do anyway, and is independant of this fix.

Quoted 7 lines
Quoted 2 lines

Now sent:

	https://lore.kernel.org/r/[email protected]

Hi Felipe,

With Greg above patch, this issue is fixed indirectly. I don't think my patch is needed, do you think so?

Which patch is "your patch" here?

thanks,

greg k-h

author
#7 lore raw
Quoted 26 lines · Greg Kroah-Hartman
Quoted 25 lines · Peter Chen
Quoted 23 lines · Greg Kroah-Hartman
Quoted 15 lines · Greg Kroah-Hartman
Quoted 9 lines · Jack Pham

Hi Peter,

Quoted 5 lines · Peter Chen

When do system reboot, it calls dwc3_shutdown and the whole debugfs for dwc3 has removed first, when the gadget tries to do deinit, and remove debugfs for its endpoints, it meets NULL pointer dereference issue when call debugfs_lookup. Fix it by removing the whole dwc3 debugfs later than dwc3_drd_exit.

Quoted 4 lines

Ouch, thanks for catching this! I think in your previous reply[1] you did warn about the debugfs_remove_recursive() getting called twice, but it seems here the issue is due to the debugfs_lookup() getting called on a stale dwc->root pointer after it was already removed.

Quoted 4 lines

We can also fix this by getting rid of that "root" pointer as it's useless (we can look it up if we need it.) I'll send a patch later to do that, as it's a good idea to do anyway, and is independant of this fix.

Now sent:

	https://lore.kernel.org/r/[email protected]
Quoted 7 lines
Quoted 4 lines

Hi Felipe,

With Greg above patch, this issue is fixed indirectly. I don't think my patch is needed, do you think so?

Which patch is "your patch" here?

The patch in this thread.

Signature · 2 lines
Thanks,
Peter Chen
#8 lore raw
Quoted 29 lines · Peter Chen
Quoted 28 lines · Greg Kroah-Hartman
Quoted 27 lines · Peter Chen
Quoted 16 lines · Greg Kroah-Hartman
Quoted 15 lines · Greg Kroah-Hartman
Quoted 13 lines · Jack Pham

Hi Peter,

Quoted 5 lines · Peter Chen

When do system reboot, it calls dwc3_shutdown and the whole debugfs for dwc3 has removed first, when the gadget tries to do deinit, and remove debugfs for its endpoints, it meets NULL pointer dereference issue when call debugfs_lookup. Fix it by removing the whole dwc3 debugfs later than dwc3_drd_exit.

Ouch, thanks for catching this! I think in your previous reply[1] you did warn about the debugfs_remove_recursive() getting called twice, but it seems here the issue is due to the debugfs_lookup() getting called on a stale dwc->root pointer after it was already removed.

Quoted 7 lines
Quoted 4 lines

We can also fix this by getting rid of that "root" pointer as it's useless (we can look it up if we need it.) I'll send a patch later to do that, as it's a good idea to do anyway, and is independant of this fix.

Now sent:

	https://lore.kernel.org/r/[email protected]

Hi Felipe,

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

With Greg above patch, this issue is fixed indirectly. I don't think my patch is needed, do you think so?

Which patch is "your patch" here?

The patch in this thread.

This one, Greg:
https://lore.kernel.org/r/[email protected]

Now that we have that, I don't think $subject is required.

Signature · 1 line
balbi
maintainer
#9 lore raw
Quoted 37 lines · Felipe Balbi
Quoted 35 lines · Peter Chen
Quoted 33 lines · Greg Kroah-Hartman
Quoted 22 lines · Peter Chen
Quoted 21 lines · Greg Kroah-Hartman
Quoted 19 lines · Greg Kroah-Hartman
Quoted 9 lines · Jack Pham

Hi Peter,

Quoted 5 lines · Peter Chen

When do system reboot, it calls dwc3_shutdown and the whole debugfs for dwc3 has removed first, when the gadget tries to do deinit, and remove debugfs for its endpoints, it meets NULL pointer dereference issue when call debugfs_lookup. Fix it by removing the whole dwc3 debugfs later than dwc3_drd_exit.

Quoted 4 lines

Ouch, thanks for catching this! I think in your previous reply[1] you did warn about the debugfs_remove_recursive() getting called twice, but it seems here the issue is due to the debugfs_lookup() getting called on a stale dwc->root pointer after it was already removed.

We can also fix this by getting rid of that "root" pointer as it's useless (we can look it up if we need it.) I'll send a patch later to do that, as it's a good idea to do anyway, and is independant of this fix.

Quoted 7 lines
Quoted 2 lines

Now sent:

	https://lore.kernel.org/r/[email protected]

Hi Felipe,

With Greg above patch, this issue is fixed indirectly. I don't think my patch is needed, do you think so?

Which patch is "your patch" here?

Quoted 6 lines
Quoted 1 line

The patch in this thread.

This one, Greg:
https://lore.kernel.org/r/[email protected]

Now that we have that, I don't think $subject is required.

I don't think it's needed to revert, it's all "safer" this way...

thanks,

greg k-h