Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] USB: chaoskey: fix error case of a timeout
@ 2019-11-07 14:28 Oliver Neukum
  2019-11-07 14:28 ` Oliver Neukum
  0 siblings, 1 reply; 4+ messages in thread
From: Oliver Neukum @ 2019-11-07 14:28 UTC (permalink / raw)
  To: keithp, gregkh, linux-usb; +Cc: Oliver Neukum, Oliver Neukum

In case of a timeout or if a signal aborts a read
communication with the device needs to be ended
lest we overwrite an active URB the next time we
do IO to the device, as the URB may still be active.

Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
 drivers/usb/misc/chaoskey.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c
index 34e6cd6f40d3..87067c3d6109 100644
--- a/drivers/usb/misc/chaoskey.c
+++ b/drivers/usb/misc/chaoskey.c
@@ -384,13 +384,17 @@ static int _chaoskey_fill(struct chaoskey *dev)
 		!dev->reading,
 		(started ? NAK_TIMEOUT : ALEA_FIRST_TIMEOUT) );
 
-	if (result < 0)
+	if (result < 0) {
+		usb_kill_urb(dev->urb);
 		goto out;
+	}
 
-	if (result == 0)
+	if (result == 0) {
 		result = -ETIMEDOUT;
-	else
+		usb_kill_urb(dev->urb);
+	} else {
 		result = dev->valid;
+	}
 out:
 	/* Let the device go back to sleep eventually */
 	usb_autopm_put_interface(dev->interface);
@@ -526,7 +530,21 @@ static int chaoskey_suspend(struct usb_interface *interface,
 
 static int chaoskey_resume(struct usb_interface *interface)
 {
+	struct chaoskey *dev;
+	struct usb_device *udev = interface_to_usbdev(interface);
+
 	usb_dbg(interface, "resume");
+	dev = usb_get_intfdata(interface);
+
+	/*
+	 * We may have lost power.
+	 * In that case the device that needs a long time
+	 * for the first requests needs an extended timeout
+	 * again
+	 */
+	if (le16_to_cpu(udev->descriptor.idVendor) == ALEA_VENDOR_ID)
+		dev->reads_started = false;
+
 	return 0;
 }
 #else
-- 
2.16.4


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

* [PATCH] USB: chaoskey: fix error case of a timeout
  2019-11-07 14:28 [PATCH] USB: chaoskey: fix error case of a timeout Oliver Neukum
@ 2019-11-07 14:28 ` Oliver Neukum
  2019-11-07 15:01   ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Oliver Neukum @ 2019-11-07 14:28 UTC (permalink / raw)
  To: keithp, gregkh, linux-usb; +Cc: Oliver Neukum, Oliver Neukum

In case of a timeout or if a signal aborts a read
communication with the device needs to be ended
lest we overwrite an active URB the next time we
do IO to the device, as the URB may still be active.

Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
 drivers/usb/misc/chaoskey.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c
index 34e6cd6f40d3..87067c3d6109 100644
--- a/drivers/usb/misc/chaoskey.c
+++ b/drivers/usb/misc/chaoskey.c
@@ -384,13 +384,17 @@ static int _chaoskey_fill(struct chaoskey *dev)
 		!dev->reading,
 		(started ? NAK_TIMEOUT : ALEA_FIRST_TIMEOUT) );
 
-	if (result < 0)
+	if (result < 0) {
+		usb_kill_urb(dev->urb);
 		goto out;
+	}
 
-	if (result == 0)
+	if (result == 0) {
 		result = -ETIMEDOUT;
-	else
+		usb_kill_urb(dev->urb);
+	} else {
 		result = dev->valid;
+	}
 out:
 	/* Let the device go back to sleep eventually */
 	usb_autopm_put_interface(dev->interface);
@@ -526,7 +530,21 @@ static int chaoskey_suspend(struct usb_interface *interface,
 
 static int chaoskey_resume(struct usb_interface *interface)
 {
+	struct chaoskey *dev;
+	struct usb_device *udev = interface_to_usbdev(interface);
+
 	usb_dbg(interface, "resume");
+	dev = usb_get_intfdata(interface);
+
+	/*
+	 * We may have lost power.
+	 * In that case the device that needs a long time
+	 * for the first requests needs an extended timeout
+	 * again
+	 */
+	if (le16_to_cpu(udev->descriptor.idVendor) == ALEA_VENDOR_ID)
+		dev->reads_started = false;
+
 	return 0;
 }
 #else
-- 
2.16.4


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

* Re: [PATCH] USB: chaoskey: fix error case of a timeout
  2019-11-07 14:28 ` Oliver Neukum
@ 2019-11-07 15:01   ` Greg KH
  2019-11-07 17:06     ` Oliver Neukum
  0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2019-11-07 15:01 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: keithp, linux-usb, Oliver Neukum

On Thu, Nov 07, 2019 at 03:28:56PM +0100, Oliver Neukum wrote:
> In case of a timeout or if a signal aborts a read
> communication with the device needs to be ended
> lest we overwrite an active URB the next time we
> do IO to the device, as the URB may still be active.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.de>
> ---
>  drivers/usb/misc/chaoskey.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c
> index 34e6cd6f40d3..87067c3d6109 100644
> --- a/drivers/usb/misc/chaoskey.c
> +++ b/drivers/usb/misc/chaoskey.c
> @@ -384,13 +384,17 @@ static int _chaoskey_fill(struct chaoskey *dev)
>  		!dev->reading,
>  		(started ? NAK_TIMEOUT : ALEA_FIRST_TIMEOUT) );
>  
> -	if (result < 0)
> +	if (result < 0) {
> +		usb_kill_urb(dev->urb);
>  		goto out;
> +	}
>  
> -	if (result == 0)
> +	if (result == 0) {
>  		result = -ETIMEDOUT;
> -	else
> +		usb_kill_urb(dev->urb);
> +	} else {
>  		result = dev->valid;
> +	}
>  out:
>  	/* Let the device go back to sleep eventually */
>  	usb_autopm_put_interface(dev->interface);
> @@ -526,7 +530,21 @@ static int chaoskey_suspend(struct usb_interface *interface,
>  
>  static int chaoskey_resume(struct usb_interface *interface)
>  {
> +	struct chaoskey *dev;
> +	struct usb_device *udev = interface_to_usbdev(interface);
> +
>  	usb_dbg(interface, "resume");
> +	dev = usb_get_intfdata(interface);
> +
> +	/*
> +	 * We may have lost power.
> +	 * In that case the device that needs a long time
> +	 * for the first requests needs an extended timeout
> +	 * again
> +	 */
> +	if (le16_to_cpu(udev->descriptor.idVendor) == ALEA_VENDOR_ID)

What is this helping with?

> +		dev->reads_started = false;
> +
>  	return 0;
>  }
>  #else
> -- 
> 2.16.4
> 

Why send this twice?

And did this pass the syzbot testing?

thanks,

greg k-h

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

* Re: [PATCH] USB: chaoskey: fix error case of a timeout
  2019-11-07 15:01   ` Greg KH
@ 2019-11-07 17:06     ` Oliver Neukum
  0 siblings, 0 replies; 4+ messages in thread
From: Oliver Neukum @ 2019-11-07 17:06 UTC (permalink / raw)
  To: Greg KH; +Cc: keithp, linux-usb

Am Donnerstag, den 07.11.2019, 16:01 +0100 schrieb Greg KH:
> On Thu, Nov 07, 2019 at 03:28:56PM +0100, Oliver Neukum wrote:
> > In case of a timeout or if a signal aborts a read
> > communication with the device needs to be ended
> > lest we overwrite an active URB the next time we
> > do IO to the device, as the URB may still be active.
> > 
> > Signed-off-by: Oliver Neukum <oneukum@suse.de>
> > ---
> >  drivers/usb/misc/chaoskey.c | 24 +++++++++++++++++++++---
> >  1 file changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c
> > index 34e6cd6f40d3..87067c3d6109 100644
> > --- a/drivers/usb/misc/chaoskey.c
> > +++ b/drivers/usb/misc/chaoskey.c
> > @@ -384,13 +384,17 @@ static int _chaoskey_fill(struct chaoskey *dev)
> >  		!dev->reading,
> >  		(started ? NAK_TIMEOUT : ALEA_FIRST_TIMEOUT) );
> >  
> > -	if (result < 0)
> > +	if (result < 0) {
> > +		usb_kill_urb(dev->urb);
> >  		goto out;
> > +	}
> >  
> > -	if (result == 0)
> > +	if (result == 0) {
> >  		result = -ETIMEDOUT;
> > -	else
> > +		usb_kill_urb(dev->urb);
> > +	} else {
> >  		result = dev->valid;
> > +	}
> >  out:
> >  	/* Let the device go back to sleep eventually */
> >  	usb_autopm_put_interface(dev->interface);
> > @@ -526,7 +530,21 @@ static int chaoskey_suspend(struct usb_interface *interface,
> >  
> >  static int chaoskey_resume(struct usb_interface *interface)
> >  {
> > +	struct chaoskey *dev;
> > +	struct usb_device *udev = interface_to_usbdev(interface);
> > +
> >  	usb_dbg(interface, "resume");
> > +	dev = usb_get_intfdata(interface);
> > +
> > +	/*
> > +	 * We may have lost power.
> > +	 * In that case the device that needs a long time
> > +	 * for the first requests needs an extended timeout
> > +	 * again
> > +	 */
> > +	if (le16_to_cpu(udev->descriptor.idVendor) == ALEA_VENDOR_ID)
> 
> What is this helping with?

THe quirk is specific for this model. The others do
not need it.

> > +		dev->reads_started = false;
> > +
> >  	return 0;
> >  }
> >  #else
> > -- 
> > 2.16.4
> > 
> 
> Why send this twice?

There must be an issue in my scripts. Investigating.

> And did this pass the syzbot testing?

Inconclusive. The test crashed but in another place.
I cannot claim that fix.

	Regards
		Oliver


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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-07 14:28 [PATCH] USB: chaoskey: fix error case of a timeout Oliver Neukum
2019-11-07 14:28 ` Oliver Neukum
2019-11-07 15:01   ` Greg KH
2019-11-07 17:06     ` Oliver Neukum

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git