| Message ID | 20210511033140.29658-1-qiang.zhang@windriver.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | USB: cdc-wdm: Fix ODEBUG bug in wdm_disconnect | expand |
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?
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
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
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: