[PATCH] USB: cdc-wdm: Fix ODEBUG bug in wdm_disconnect

[PATCH] USB: cdc-wdm: Fix ODEBUG bug in wdm_disconnect

[qiang.zhang]

From: Zqiang <qiang.zhang@windriver.com>

ODEBUG: free active (active state 0) object type:
work_struct hint: service_interrupt_work+0x0/0x110
arch/x86/include/asm/bitops.h:207
WARNING: CPU: 0 PID: 9431 at lib/debugobjects.c:505
debug_print_object+0x16e/0x250 lib/debugobjects.c:505
Call Trace:
 __debug_check_no_obj_freed
 debug_check_no_obj_freed
 slab_free_hook
 slab_free_freelist_hook
 slab_free
 kfree+0xdb/0x3b0
 wdm_disconnect+0x3bd/0x450
 usb_unbind_interface+0x1d8/0x8d0
 __device_release_driver+0x3bd/0x6f0
 device_release_driver_internal
 device_release_driver+0x26/0x40
 bus_remove_device+0x2eb/0x5a0
 device_del+0x502/0xd40
 usb_disable_device+0x35b/0x7b0
 usb_disconnect.cold+0x27d/0x791
 hub_port_connect [inline]
 hub_port_connect_change [inline]
 port_event [inline]
 hub_event+0x1c9c/0x4320
 process_one_work+0x98d/0x1580
 worker_thread+0x64c/0x1120
 kthread+0x38c/0x460
 ret_from_fork+0x1f/0x30

This warning is generated because when kfree wdm_device,
it is found that there is still an active work in workqueue,
This phenomenon may be due to the following reasons.
when the devices disconnect, although the work was cancelled,
but the schedule_work still may be called, therefore, before
scheduling work, we need to detect the status of the device.

Reported-by: syzbot <syzbot+7da71853830ac3289474@syzkaller.appspotmail.com>
Signed-off-by: Zqiang <qiang.zhang@windriver.com>
---
 drivers/usb/class/cdc-wdm.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 508b1c3f8b73..50fa951d84a1 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -221,7 +221,8 @@ static void wdm_in_callback(struct urb *urb)
 		 * We should respond to further attempts from the device to send
 		 * data, so that we can get unstuck.
 		 */
-		schedule_work(&desc->service_outs_intr);
+		if (!test_bit(WDM_DISCONNECTING, &desc->flags))
+			schedule_work(&desc->service_outs_intr);
 	} else {
 		set_bit(WDM_READ, &desc->flags);
 		wake_up(&desc->wait);
@@ -306,10 +307,12 @@ static void wdm_int_callback(struct urb *urb)
 			return;
 		if (rv == -ENOMEM) {
 sw:
-			rv = schedule_work(&desc->rxwork);
-			if (rv)
-				dev_err(&desc->intf->dev,
-					"Cannot schedule work\n");
+			if (!test_bit(WDM_DISCONNECTING, &desc->flags)) {
+				rv = schedule_work(&desc->rxwork);
+				if (rv)
+					dev_err(&desc->intf->dev,
+						"Cannot schedule work\n");
+			}
 		}
 	}
 exit:
-- 
2.17.1

Re: [PATCH] USB: cdc-wdm: Fix ODEBUG bug in wdm_disconnect

[Tetsuo Handa]

On 2021/05/11 12:31, qiang.zhang@windriver.com wrote:
> This warning is generated because when kfree wdm_device,
> it is found that there is still an active work in workqueue,
> This phenomenon may be due to the following reasons.
> when the devices disconnect, although the work was cancelled,
> but the schedule_work still may be called, therefore, before
> scheduling work, we need to detect the status of the device.
> 
> Reported-by: syzbot <syzbot+7da71853830ac3289474@syzkaller.appspotmail.com>
> Signed-off-by: Zqiang <qiang.zhang@windriver.com>
> ---
>  drivers/usb/class/cdc-wdm.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 

Oliver Neukum is aware of this problem, and is considering poisoning approach than
checking WDM_SUSPENDING/WDM_RESETTING/WDM_DISCONNECTING approach. I guess we could
replace three test_bit() tests into OR'ed-bits test (something like test_bits()) ?

  https://lkml.kernel.org/r/2db36d52015b644cc1891fcffc87ef09c2b728b7.camel@suse.com

Oliver, how do we want to fix this problem?

Re: [PATCH] USB: cdc-wdm: Fix ODEBUG bug in wdm_disconnect

[Oliver Neukum]

Am Dienstag, den 11.05.2021, 14:48 +0900 schrieb Tetsuo Handa:
> On 2021/05/11 12:31, qiang.zhang@windriver.com wrote:
> > This warning is generated because when kfree wdm_device,
> > it is found that there is still an active work in workqueue,
> > This phenomenon may be due to the following reasons.
> > when the devices disconnect, although the work was cancelled,
> > but the schedule_work still may be called, therefore, before
> > scheduling work, we need to detect the status of the device.
> > 
> > Reported-by: syzbot <
> > syzbot+7da71853830ac3289474@syzkaller.appspotmail.com>
> > Signed-off-by: Zqiang <qiang.zhang@windriver.com>
> > ---
> >  drivers/usb/class/cdc-wdm.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> 
> Oliver Neukum is aware of this problem, and is considering poisoning
> approach than
> checking WDM_SUSPENDING/WDM_RESETTING/WDM_DISCONNECTING approach. I
> guess we could
> replace three test_bit() tests into OR'ed-bits test (something like
> test_bits()) ?
> 
>   
> https://lkml.kernel.org/r/2db36d52015b644cc1891fcffc87ef09c2b728b7.camel@suse.com
> 
> Oliver, how do we want to fix this problem?

Hi,

I was under the assumption that Greg had already merged the fix
18abf874367456540846319574864e6ff32752e2
Am I wrong?

	Regards
		Oliver

Re: [PATCH] USB: cdc-wdm: Fix ODEBUG bug in wdm_disconnect

[Greg KH]

On Tue, May 11, 2021 at 10:11:41AM +0200, Oliver Neukum wrote:
> Am Dienstag, den 11.05.2021, 14:48 +0900 schrieb Tetsuo Handa:
> > On 2021/05/11 12:31, qiang.zhang@windriver.com wrote:
> > > This warning is generated because when kfree wdm_device,
> > > it is found that there is still an active work in workqueue,
> > > This phenomenon may be due to the following reasons.
> > > when the devices disconnect, although the work was cancelled,
> > > but the schedule_work still may be called, therefore, before
> > > scheduling work, we need to detect the status of the device.
> > > 
> > > Reported-by: syzbot <
> > > syzbot+7da71853830ac3289474@syzkaller.appspotmail.com>
> > > Signed-off-by: Zqiang <qiang.zhang@windriver.com>
> > > ---
> > >  drivers/usb/class/cdc-wdm.c | 13 ++++++++-----
> > >  1 file changed, 8 insertions(+), 5 deletions(-)
> > > 
> > 
> > Oliver Neukum is aware of this problem, and is considering poisoning
> > approach than
> > checking WDM_SUSPENDING/WDM_RESETTING/WDM_DISCONNECTING approach. I
> > guess we could
> > replace three test_bit() tests into OR'ed-bits test (something like
> > test_bits()) ?
> > 
> >   
> > https://lkml.kernel.org/r/2db36d52015b644cc1891fcffc87ef09c2b728b7.camel@suse.com
> > 
> > Oliver, how do we want to fix this problem?
> 
> Hi,
> 
> I was under the assumption that Greg had already merged the fix
> 18abf874367456540846319574864e6ff32752e2
> Am I wrong?

That is in my usb-linus branch and is not in Linus's tree yet.  Give it
a week or so to get there...

thanks,

greg k-h