All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Input: iforce - wake up after clearing IFORCE_XMIT_RUNNING flag
@ 2022-07-26  5:35 Tetsuo Handa
  2022-07-26 22:00 ` [PATCH v2] " Tetsuo Handa
  0 siblings, 1 reply; 5+ messages in thread
From: Tetsuo Handa @ 2022-07-26  5:35 UTC (permalink / raw)
  To: Fabio M. De Francesco, Dmitry Torokhov, Johan Hovold,
	Henrik Rydberg, ira.weiny, Hillf Danton
  Cc: linux-input

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")
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>
---
 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(-)

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,
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2] Input: iforce - wake up after clearing IFORCE_XMIT_RUNNING flag
  2022-07-26  5:35 [PATCH] Input: iforce - wake up after clearing IFORCE_XMIT_RUNNING flag Tetsuo Handa
@ 2022-07-26 22:00 ` Tetsuo Handa
  2022-08-16  9:50   ` Tetsuo Handa
  2022-08-28  3:54   ` Dmitry Torokhov
  0 siblings, 2 replies; 5+ messages in thread
From: Tetsuo Handa @ 2022-07-26 22:00 UTC (permalink / raw)
  To: Fabio M. De Francesco, Dmitry Torokhov, Johan Hovold,
	Henrik Rydberg, ira.weiny, Hillf Danton
  Cc: linux-input

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(-)

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,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Input: iforce - wake up after clearing IFORCE_XMIT_RUNNING flag
  2022-07-26 22:00 ` [PATCH v2] " Tetsuo Handa
@ 2022-08-16  9:50   ` Tetsuo Handa
  2022-08-27  6:17     ` Tetsuo Handa
  2022-08-28  3:54   ` Dmitry Torokhov
  1 sibling, 1 reply; 5+ messages in thread
From: Tetsuo Handa @ 2022-08-16  9:50 UTC (permalink / raw)
  To: Fabio M. De Francesco, Dmitry Torokhov, Johan Hovold,
	Henrik Rydberg, ira.weiny, Hillf Danton
  Cc: linux-input

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>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Input: iforce - wake up after clearing IFORCE_XMIT_RUNNING flag
  2022-08-16  9:50   ` Tetsuo Handa
@ 2022-08-27  6:17     ` Tetsuo Handa
  0 siblings, 0 replies; 5+ messages in thread
From: Tetsuo Handa @ 2022-08-27  6:17 UTC (permalink / raw)
  To: Fabio M. De Francesco, Dmitry Torokhov, Johan Hovold,
	Henrik Rydberg, ira.weiny, Hillf Danton
  Cc: linux-input, Greg Kroah-Hartman, Andrew Morton

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Input: iforce - wake up after clearing IFORCE_XMIT_RUNNING flag
  2022-07-26 22:00 ` [PATCH v2] " Tetsuo Handa
  2022-08-16  9:50   ` Tetsuo Handa
@ 2022-08-28  3:54   ` Dmitry Torokhov
  1 sibling, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2022-08-28  3:54 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: Fabio M. De Francesco, Johan Hovold, Henrik Rydberg, ira.weiny,
	Hillf Danton, linux-input

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.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-08-28  3:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-26  5:35 [PATCH] Input: iforce - wake up after clearing IFORCE_XMIT_RUNNING flag Tetsuo Handa
2022-07-26 22:00 ` [PATCH v2] " Tetsuo Handa
2022-08-16  9:50   ` Tetsuo Handa
2022-08-27  6:17     ` Tetsuo Handa
2022-08-28  3:54   ` Dmitry Torokhov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.