| Message ID | 887021c3-4f13-40ce-c8b9-aa6e09faa3a7@I-love.SAKURA.ne.jp |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] Input: iforce - wake up after clearing IFORCE_XMIT_RUNNING flag | expand |
Any questions? On 2022/07/27 7:00, Tetsuo Handa wrote: > syzbot is reporting hung task at __input_unregister_device() [1], for > iforce_close() waiting at wait_event_interruptible() with dev->mutex held > is blocking input_disconnect_device() from __input_unregister_device(). > > It seems that the cause is simply that commit c2b27ef672992a20 ("Input: > iforce - wait for command completion when closing the device") forgot to > call wake_up() after clear_bit(). > > Fix this problem by introducing a helper that calls clear_bit() followed > by wake_up_all(). > > Link: https://syzkaller.appspot.com/bug?extid=deb6abc36aad4008f407 [1] > Reported-by: syzbot <syzbot+deb6abc36aad4008f407@syzkaller.appspotmail.com> > Fixes: c2b27ef672992a20 ("Input: iforce - wait for command completion when closing the device") > Tested-by: syzbot <syzbot+deb6abc36aad4008f407@syzkaller.appspotmail.com> > Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > Co-developed-by: Hillf Danton <hdanton@sina.com> > Signed-off-by: Hillf Danton <hdanton@sina.com> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
No response from maintainers for one month. Do I need to directly ask Greg or Andrew to take this patch? On 2022/08/16 18:50, Tetsuo Handa wrote: > Any questions? > > On 2022/07/27 7:00, Tetsuo Handa wrote: >> syzbot is reporting hung task at __input_unregister_device() [1], for >> iforce_close() waiting at wait_event_interruptible() with dev->mutex held >> is blocking input_disconnect_device() from __input_unregister_device(). >> >> It seems that the cause is simply that commit c2b27ef672992a20 ("Input: >> iforce - wait for command completion when closing the device") forgot to >> call wake_up() after clear_bit(). >> >> Fix this problem by introducing a helper that calls clear_bit() followed >> by wake_up_all(). >> >> Link: https://syzkaller.appspot.com/bug?extid=deb6abc36aad4008f407 [1] >> Reported-by: syzbot <syzbot+deb6abc36aad4008f407@syzkaller.appspotmail.com> >> Fixes: c2b27ef672992a20 ("Input: iforce - wait for command completion when closing the device") >> Tested-by: syzbot <syzbot+deb6abc36aad4008f407@syzkaller.appspotmail.com> >> Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> >> Co-developed-by: Hillf Danton <hdanton@sina.com> >> Signed-off-by: Hillf Danton <hdanton@sina.com> >> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >
On Wed, Jul 27, 2022 at 07:00:00AM +0900, Tetsuo Handa wrote: > syzbot is reporting hung task at __input_unregister_device() [1], for > iforce_close() waiting at wait_event_interruptible() with dev->mutex held > is blocking input_disconnect_device() from __input_unregister_device(). > > It seems that the cause is simply that commit c2b27ef672992a20 ("Input: > iforce - wait for command completion when closing the device") forgot to > call wake_up() after clear_bit(). > > Fix this problem by introducing a helper that calls clear_bit() followed > by wake_up_all(). > > Link: https://syzkaller.appspot.com/bug?extid=deb6abc36aad4008f407 [1] > Reported-by: syzbot <syzbot+deb6abc36aad4008f407@syzkaller.appspotmail.com> > Fixes: c2b27ef672992a20 ("Input: iforce - wait for command completion when closing the device") > Tested-by: syzbot <syzbot+deb6abc36aad4008f407@syzkaller.appspotmail.com> > Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > Co-developed-by: Hillf Danton <hdanton@sina.com> > Signed-off-by: Hillf Danton <hdanton@sina.com> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Applied, thank you.
diff --git a/drivers/input/joystick/iforce/iforce-serio.c b/drivers/input/joystick/iforce/iforce-serio.c index f95a81b9fac7..2380546d7978 100644 --- a/drivers/input/joystick/iforce/iforce-serio.c +++ b/drivers/input/joystick/iforce/iforce-serio.c @@ -39,7 +39,7 @@ static void iforce_serio_xmit(struct iforce *iforce) again: if (iforce->xmit.head == iforce->xmit.tail) { - clear_bit(IFORCE_XMIT_RUNNING, iforce->xmit_flags); + iforce_clear_xmit_and_wake(iforce); spin_unlock_irqrestore(&iforce->xmit_lock, flags); return; } @@ -64,7 +64,7 @@ static void iforce_serio_xmit(struct iforce *iforce) if (test_and_clear_bit(IFORCE_XMIT_AGAIN, iforce->xmit_flags)) goto again; - clear_bit(IFORCE_XMIT_RUNNING, iforce->xmit_flags); + iforce_clear_xmit_and_wake(iforce); spin_unlock_irqrestore(&iforce->xmit_lock, flags); } @@ -169,7 +169,7 @@ static irqreturn_t iforce_serio_irq(struct serio *serio, iforce_serio->cmd_response_len = iforce_serio->len; /* Signal that command is done */ - wake_up(&iforce->wait); + wake_up_all(&iforce->wait); } else if (likely(iforce->type)) { iforce_process_packet(iforce, iforce_serio->id, iforce_serio->data_in, diff --git a/drivers/input/joystick/iforce/iforce-usb.c b/drivers/input/joystick/iforce/iforce-usb.c index ea58805c480f..cba92bd590a8 100644 --- a/drivers/input/joystick/iforce/iforce-usb.c +++ b/drivers/input/joystick/iforce/iforce-usb.c @@ -30,7 +30,7 @@ static void __iforce_usb_xmit(struct iforce *iforce) spin_lock_irqsave(&iforce->xmit_lock, flags); if (iforce->xmit.head == iforce->xmit.tail) { - clear_bit(IFORCE_XMIT_RUNNING, iforce->xmit_flags); + iforce_clear_xmit_and_wake(iforce); spin_unlock_irqrestore(&iforce->xmit_lock, flags); return; } @@ -58,9 +58,9 @@ static void __iforce_usb_xmit(struct iforce *iforce) XMIT_INC(iforce->xmit.tail, n); if ( (n=usb_submit_urb(iforce_usb->out, GFP_ATOMIC)) ) { - clear_bit(IFORCE_XMIT_RUNNING, iforce->xmit_flags); dev_warn(&iforce_usb->intf->dev, "usb_submit_urb failed %d\n", n); + iforce_clear_xmit_and_wake(iforce); } /* The IFORCE_XMIT_RUNNING bit is not cleared here. That's intended. @@ -175,15 +175,15 @@ static void iforce_usb_out(struct urb *urb) struct iforce *iforce = &iforce_usb->iforce; if (urb->status) { - clear_bit(IFORCE_XMIT_RUNNING, iforce->xmit_flags); dev_dbg(&iforce_usb->intf->dev, "urb->status %d, exiting\n", urb->status); + iforce_clear_xmit_and_wake(iforce); return; } __iforce_usb_xmit(iforce); - wake_up(&iforce->wait); + wake_up_all(&iforce->wait); } static int iforce_usb_probe(struct usb_interface *intf, diff --git a/drivers/input/joystick/iforce/iforce.h b/drivers/input/joystick/iforce/iforce.h index 6aa761ebbdf7..9ccb9107ccbe 100644 --- a/drivers/input/joystick/iforce/iforce.h +++ b/drivers/input/joystick/iforce/iforce.h @@ -119,6 +119,12 @@ static inline int iforce_get_id_packet(struct iforce *iforce, u8 id, response_data, response_len); } +static inline void iforce_clear_xmit_and_wake(struct iforce *iforce) +{ + clear_bit(IFORCE_XMIT_RUNNING, iforce->xmit_flags); + wake_up_all(&iforce->wait); +} + /* Public functions */ /* iforce-main.c */ int iforce_init_device(struct device *parent, u16 bustype,
syzbot is reporting hung task at __input_unregister_device() [1], for iforce_close() waiting at wait_event_interruptible() with dev->mutex held is blocking input_disconnect_device() from __input_unregister_device(). It seems that the cause is simply that commit c2b27ef672992a20 ("Input: iforce - wait for command completion when closing the device") forgot to call wake_up() after clear_bit(). Fix this problem by introducing a helper that calls clear_bit() followed by wake_up_all(). Link: https://syzkaller.appspot.com/bug?extid=deb6abc36aad4008f407 [1] Reported-by: syzbot <syzbot+deb6abc36aad4008f407@syzkaller.appspotmail.com> Fixes: c2b27ef672992a20 ("Input: iforce - wait for command completion when closing the device") Tested-by: syzbot <syzbot+deb6abc36aad4008f407@syzkaller.appspotmail.com> Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> Co-developed-by: Hillf Danton <hdanton@sina.com> Signed-off-by: Hillf Danton <hdanton@sina.com> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> --- Changes in v2: Add Suggested-by: and Tested-by: tags. drivers/input/joystick/iforce/iforce-serio.c | 6 +++--- drivers/input/joystick/iforce/iforce-usb.c | 8 ++++---- drivers/input/joystick/iforce/iforce.h | 6 ++++++ 3 files changed, 13 insertions(+), 7 deletions(-)