diff mbox series

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

Message ID 20210608105656.10795-1-peter.chen@kernel.org
State New
Headers show
Series [1/1] usb: dwc3: core: fix kernel panic when do reboot | expand

Commit Message

Peter Chen June 8, 2021, 10:56 a.m. UTC
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 <jackp@codeaurora.org>
Fixes: 5ff90af9da8f ("usb: dwc3: debugfs: Add and remove endpoint dirs dynamically")
Signed-off-by: Peter Chen <peter.chen@kernel.org>
---
 drivers/usb/dwc3/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andy Shevchenko June 8, 2021, 1:37 p.m. UTC | #1
On Tue, Jun 8, 2021 at 2:01 PM Peter Chen <peter.chen@kernel.org> wrote:
>
> 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.

> [ 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.

> [ 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 ]---
Jack Pham June 8, 2021, 4:50 p.m. UTC | #2
Hi Peter,

On Tue, Jun 08, 2021 at 06:56:56PM +0800, Peter Chen wrote:
> 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/20210603151742.298243-1-alexandru.elisei@arm.com

> [ 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 <jackp@codeaurora.org>
> Fixes: 5ff90af9da8f ("usb: dwc3: debugfs: Add and remove endpoint dirs dynamically")
> Signed-off-by: Peter Chen <peter.chen@kernel.org>

You can also add

Tested-by: Jack Pham <jackp@codeaurora.org>

Thanks again and sorry for the regression!
Jack
Greg KH June 9, 2021, 9:01 a.m. UTC | #3
On Tue, Jun 08, 2021 at 09:50:01AM -0700, Jack Pham wrote:
> Hi Peter,

> 

> On Tue, Jun 08, 2021 at 06:56:56PM +0800, Peter Chen wrote:

> > 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.


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
Greg KH June 9, 2021, 9:42 a.m. UTC | #4
On Wed, Jun 09, 2021 at 11:01:07AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 08, 2021 at 09:50:01AM -0700, Jack Pham wrote:

> > Hi Peter,

> > 

> > On Tue, Jun 08, 2021 at 06:56:56PM +0800, Peter Chen wrote:

> > > 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.

> 

> 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/20210609093924.3293230-1-gregkh@linuxfoundation.org
Peter Chen June 10, 2021, 2 a.m. UTC | #5
On 21-06-09 11:42:53, Greg Kroah-Hartman wrote:
> On Wed, Jun 09, 2021 at 11:01:07AM +0200, Greg Kroah-Hartman wrote:

> > On Tue, Jun 08, 2021 at 09:50:01AM -0700, Jack Pham wrote:

> > > Hi Peter,

> > > 

> > > On Tue, Jun 08, 2021 at 06:56:56PM +0800, Peter Chen wrote:

> > > > 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.

> > 

> > 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/20210609093924.3293230-1-gregkh@linuxfoundation.org


Hi Felipe,

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

-- 

Thanks,
Peter Chen
Greg KH June 10, 2021, 6:36 a.m. UTC | #6
On Thu, Jun 10, 2021 at 10:00:47AM +0800, Peter Chen wrote:
> On 21-06-09 11:42:53, Greg Kroah-Hartman wrote:

> > On Wed, Jun 09, 2021 at 11:01:07AM +0200, Greg Kroah-Hartman wrote:

> > > On Tue, Jun 08, 2021 at 09:50:01AM -0700, Jack Pham wrote:

> > > > Hi Peter,

> > > > 

> > > > On Tue, Jun 08, 2021 at 06:56:56PM +0800, Peter Chen wrote:

> > > > > 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.

> > > 

> > > 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/20210609093924.3293230-1-gregkh@linuxfoundation.org

> 

> 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
Peter Chen June 10, 2021, 1:58 p.m. UTC | #7
On 21-06-10 08:36:57, Greg Kroah-Hartman wrote:
> On Thu, Jun 10, 2021 at 10:00:47AM +0800, Peter Chen wrote:

> > On 21-06-09 11:42:53, Greg Kroah-Hartman wrote:

> > > On Wed, Jun 09, 2021 at 11:01:07AM +0200, Greg Kroah-Hartman wrote:

> > > > On Tue, Jun 08, 2021 at 09:50:01AM -0700, Jack Pham wrote:

> > > > > Hi Peter,

> > > > > 

> > > > > On Tue, Jun 08, 2021 at 06:56:56PM +0800, Peter Chen wrote:

> > > > > > 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.

> > > > 

> > > > 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/20210609093924.3293230-1-gregkh@linuxfoundation.org

> > 

> > 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.

-- 

Thanks,
Peter Chen
Felipe Balbi June 11, 2021, 1:23 p.m. UTC | #8
Peter Chen <peter.chen@kernel.org> writes:

> On 21-06-10 08:36:57, Greg Kroah-Hartman wrote:

>> On Thu, Jun 10, 2021 at 10:00:47AM +0800, Peter Chen wrote:

>> > On 21-06-09 11:42:53, Greg Kroah-Hartman wrote:

>> > > On Wed, Jun 09, 2021 at 11:01:07AM +0200, Greg Kroah-Hartman wrote:

>> > > > On Tue, Jun 08, 2021 at 09:50:01AM -0700, Jack Pham wrote:

>> > > > > Hi Peter,

>> > > > > 

>> > > > > On Tue, Jun 08, 2021 at 06:56:56PM +0800, Peter Chen wrote:

>> > > > > > 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.

>> > > > 

>> > > > 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/20210609093924.3293230-1-gregkh@linuxfoundation.org

>> > 

>> > 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.


This one, Greg:
https://lore.kernel.org/r/20210609093924.3293230-1-gregkh@linuxfoundation.org

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

-- 
balbi
Greg KH June 11, 2021, 1:30 p.m. UTC | #9
On Fri, Jun 11, 2021 at 04:23:58PM +0300, Felipe Balbi wrote:
> Peter Chen <peter.chen@kernel.org> writes:

> 

> > On 21-06-10 08:36:57, Greg Kroah-Hartman wrote:

> >> On Thu, Jun 10, 2021 at 10:00:47AM +0800, Peter Chen wrote:

> >> > On 21-06-09 11:42:53, Greg Kroah-Hartman wrote:

> >> > > On Wed, Jun 09, 2021 at 11:01:07AM +0200, Greg Kroah-Hartman wrote:

> >> > > > On Tue, Jun 08, 2021 at 09:50:01AM -0700, Jack Pham wrote:

> >> > > > > Hi Peter,

> >> > > > > 

> >> > > > > On Tue, Jun 08, 2021 at 06:56:56PM +0800, Peter Chen wrote:

> >> > > > > > 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.

> >> > > > 

> >> > > > 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/20210609093924.3293230-1-gregkh@linuxfoundation.org

> >> > 

> >> > 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.

> 

> This one, Greg:

> https://lore.kernel.org/r/20210609093924.3293230-1-gregkh@linuxfoundation.org

> 

> 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
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index b6e53d8212cd..5174031503ca 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -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);