mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* usbgadget fix some crashes
@ 2017-09-27  9:12 Sascha Hauer
  2017-09-27  9:12 ` [PATCH 1/4] usbgadget: autostart: Handle errors in file list gracefully Sascha Hauer
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Sascha Hauer @ 2017-09-27  9:12 UTC (permalink / raw)
  To: Barebox List; +Cc: Oleksij Rempel

The following fixes some crashes I stumbled upon when trying to use
the usbgadget autostart feature.

Sascha

----------------------------------------------------------------
Sascha Hauer (4):
      usbgadget: autostart: Handle errors in file list gracefully
      usbgadget: do not register when no functions present
      usbgadget: only set to peripheral mode when error checking is done
      usbgadget: unregister when usb_composite_probe() fails

 drivers/usb/gadget/autostart.c | 18 +++++++++++++++---
 drivers/usb/gadget/multi.c     | 10 +++++++++-
 2 files changed, 24 insertions(+), 4 deletions(-)

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH 1/4] usbgadget: autostart: Handle errors in file list gracefully
  2017-09-27  9:12 usbgadget fix some crashes Sascha Hauer
@ 2017-09-27  9:12 ` Sascha Hauer
  2017-09-29 13:02   ` Oleksij Rempel
  2017-09-27  9:12 ` [PATCH 2/4] usbgadget: do not register when no functions present Sascha Hauer
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Sascha Hauer @ 2017-09-27  9:12 UTC (permalink / raw)
  To: Barebox List; +Cc: Oleksij Rempel

file_list_parse() can fail and returns an error pointer. Print
an error message when it fails and also set to NULL again so
that usb_multi_register() does not stumble upon the error pointer.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/gadget/autostart.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/autostart.c b/drivers/usb/gadget/autostart.c
index 465d8fd380..3fa43137fa 100644
--- a/drivers/usb/gadget/autostart.c
+++ b/drivers/usb/gadget/autostart.c
@@ -11,6 +11,8 @@
  * GNU General Public License for more details.
  *
  */
+#define pr_fmt(fmt) "usbgadget autostart: " fmt
+
 #include <common.h>
 #include <command.h>
 #include <errno.h>
@@ -42,8 +44,14 @@ static int usbgadget_autostart(void)
 	opts = xzalloc(sizeof(*opts));
 	opts->release = usb_multi_opts_release;
 
-	if (fastboot_function)
+	if (fastboot_function) {
 		opts->fastboot_opts.files = file_list_parse(fastboot_function);
+		if (IS_ERR(opts->fastboot_opts.files)) {
+			pr_err("Parsing file list \"%s\" failed: %s\n", fastboot_function,
+			       strerrorp(opts->fastboot_opts.files));
+			opts->fastboot_opts.files = NULL;
+		}
+	}
 
 	opts->create_acm = acm;
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH 2/4] usbgadget: do not register when no functions present
  2017-09-27  9:12 usbgadget fix some crashes Sascha Hauer
  2017-09-27  9:12 ` [PATCH 1/4] usbgadget: autostart: Handle errors in file list gracefully Sascha Hauer
@ 2017-09-27  9:12 ` Sascha Hauer
  2017-09-29 13:05   ` Oleksij Rempel
  2017-09-27  9:12 ` [PATCH 3/4] usbgadget: only set to peripheral mode when error checking is done Sascha Hauer
  2017-09-27  9:12 ` [PATCH 4/4] usbgadget: unregister when usb_composite_probe() fails Sascha Hauer
  3 siblings, 1 reply; 9+ messages in thread
From: Sascha Hauer @ 2017-09-27  9:12 UTC (permalink / raw)
  To: Barebox List; +Cc: Oleksij Rempel

registering a multifunction device makes only sense when there's
at least one function configured. Just return otherwise.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/gadget/autostart.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/gadget/autostart.c b/drivers/usb/gadget/autostart.c
index 3fa43137fa..a27be899c3 100644
--- a/drivers/usb/gadget/autostart.c
+++ b/drivers/usb/gadget/autostart.c
@@ -55,6 +55,10 @@ static int usbgadget_autostart(void)
 
 	opts->create_acm = acm;
 
+	if (!opts->fastboot_opts.files && !opts->create_acm) {
+		pr_warn("No functions to register\n");
+		return 0;
+	}
 
 	ret = usb_multi_register(opts);
 	if (ret)
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH 3/4] usbgadget: only set to peripheral mode when error checking is done
  2017-09-27  9:12 usbgadget fix some crashes Sascha Hauer
  2017-09-27  9:12 ` [PATCH 1/4] usbgadget: autostart: Handle errors in file list gracefully Sascha Hauer
  2017-09-27  9:12 ` [PATCH 2/4] usbgadget: do not register when no functions present Sascha Hauer
@ 2017-09-27  9:12 ` Sascha Hauer
  2017-09-29 13:06   ` Oleksij Rempel
  2017-09-27  9:12 ` [PATCH 4/4] usbgadget: unregister when usb_composite_probe() fails Sascha Hauer
  3 siblings, 1 reply; 9+ messages in thread
From: Sascha Hauer @ 2017-09-27  9:12 UTC (permalink / raw)
  To: Barebox List; +Cc: Oleksij Rempel

Only begin to modify the hardware when the obious errors have been
checked.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/gadget/autostart.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/autostart.c b/drivers/usb/gadget/autostart.c
index a27be899c3..2ca979057e 100644
--- a/drivers/usb/gadget/autostart.c
+++ b/drivers/usb/gadget/autostart.c
@@ -39,8 +39,6 @@ static int usbgadget_autostart(void)
 	if (!autostart)
 		return 0;
 
-	setenv("otg.mode", "peripheral");
-
 	opts = xzalloc(sizeof(*opts));
 	opts->release = usb_multi_opts_release;
 
@@ -60,6 +58,8 @@ static int usbgadget_autostart(void)
 		return 0;
 	}
 
+	setenv("otg.mode", "peripheral");
+
 	ret = usb_multi_register(opts);
 	if (ret)
 		usb_multi_opts_release(opts);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH 4/4] usbgadget: unregister when usb_composite_probe() fails
  2017-09-27  9:12 usbgadget fix some crashes Sascha Hauer
                   ` (2 preceding siblings ...)
  2017-09-27  9:12 ` [PATCH 3/4] usbgadget: only set to peripheral mode when error checking is done Sascha Hauer
@ 2017-09-27  9:12 ` Sascha Hauer
  2017-09-29 13:08   ` Oleksij Rempel
  3 siblings, 1 reply; 9+ messages in thread
From: Sascha Hauer @ 2017-09-27  9:12 UTC (permalink / raw)
  To: Barebox List; +Cc: Oleksij Rempel

When usb_multi_register() returns an error it can't be called again
as we do not unregister the driver properly. Fix this.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/gadget/multi.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 6385c16186..6eeeb4e982 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -234,6 +234,8 @@ static struct usb_composite_driver multi_driver = {
 
 int usb_multi_register(struct f_multi_opts *opts)
 {
+	int ret;
+
 	if (gadget_multi_opts) {
 		pr_err("USB multi gadget already registered\n");
 		return -EBUSY;
@@ -241,7 +243,13 @@ int usb_multi_register(struct f_multi_opts *opts)
 
 	gadget_multi_opts = opts;
 
-	return usb_composite_probe(&multi_driver);
+	ret = usb_composite_probe(&multi_driver);
+	if (ret) {
+		usb_composite_unregister(&multi_driver);
+		gadget_multi_opts = NULL;
+	}
+
+	return ret;
 }
 
 void usb_multi_unregister(void)
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] usbgadget: autostart: Handle errors in file list gracefully
  2017-09-27  9:12 ` [PATCH 1/4] usbgadget: autostart: Handle errors in file list gracefully Sascha Hauer
@ 2017-09-29 13:02   ` Oleksij Rempel
  0 siblings, 0 replies; 9+ messages in thread
From: Oleksij Rempel @ 2017-09-29 13:02 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On Wed, Sep 27, 2017 at 11:12:42AM +0200, Sascha Hauer wrote:
> file_list_parse() can fail and returns an error pointer. Print
> an error message when it fails and also set to NULL again so
> that usb_multi_register() does not stumble upon the error pointer.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/usb/gadget/autostart.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/autostart.c b/drivers/usb/gadget/autostart.c
> index 465d8fd380..3fa43137fa 100644
> --- a/drivers/usb/gadget/autostart.c
> +++ b/drivers/usb/gadget/autostart.c
> @@ -11,6 +11,8 @@
>   * GNU General Public License for more details.
>   *
>   */
> +#define pr_fmt(fmt) "usbgadget autostart: " fmt
> +
>  #include <common.h>
>  #include <command.h>
>  #include <errno.h>
> @@ -42,8 +44,14 @@ static int usbgadget_autostart(void)
>  	opts = xzalloc(sizeof(*opts));
>  	opts->release = usb_multi_opts_release;
>  
> -	if (fastboot_function)
> +	if (fastboot_function) {
>  		opts->fastboot_opts.files = file_list_parse(fastboot_function);
> +		if (IS_ERR(opts->fastboot_opts.files)) {
> +			pr_err("Parsing file list \"%s\" failed: %s\n", fastboot_function,
> +			       strerrorp(opts->fastboot_opts.files));
> +			opts->fastboot_opts.files = NULL;
> +		}
> +	}
>  
>  	opts->create_acm = acm;

Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH 2/4] usbgadget: do not register when no functions present
  2017-09-27  9:12 ` [PATCH 2/4] usbgadget: do not register when no functions present Sascha Hauer
@ 2017-09-29 13:05   ` Oleksij Rempel
  0 siblings, 0 replies; 9+ messages in thread
From: Oleksij Rempel @ 2017-09-29 13:05 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On Wed, Sep 27, 2017 at 11:12:43AM +0200, Sascha Hauer wrote:
> registering a multifunction device makes only sense when there's
> at least one function configured. Just return otherwise.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/usb/gadget/autostart.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/gadget/autostart.c b/drivers/usb/gadget/autostart.c
> index 3fa43137fa..a27be899c3 100644
> --- a/drivers/usb/gadget/autostart.c
> +++ b/drivers/usb/gadget/autostart.c
> @@ -55,6 +55,10 @@ static int usbgadget_autostart(void)
>  
>  	opts->create_acm = acm;
>  
> +	if (!opts->fastboot_opts.files && !opts->create_acm) {
> +		pr_warn("No functions to register\n");
> +		return 0;
> +	}
>  
>  	ret = usb_multi_register(opts);
>  	if (ret)
> -- 
> 2.11.0
> 
> 

Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH 3/4] usbgadget: only set to peripheral mode when error checking is done
  2017-09-27  9:12 ` [PATCH 3/4] usbgadget: only set to peripheral mode when error checking is done Sascha Hauer
@ 2017-09-29 13:06   ` Oleksij Rempel
  0 siblings, 0 replies; 9+ messages in thread
From: Oleksij Rempel @ 2017-09-29 13:06 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On Wed, Sep 27, 2017 at 11:12:44AM +0200, Sascha Hauer wrote:
> Only begin to modify the hardware when the obious errors have been
> checked.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/usb/gadget/autostart.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/autostart.c b/drivers/usb/gadget/autostart.c
> index a27be899c3..2ca979057e 100644
> --- a/drivers/usb/gadget/autostart.c
> +++ b/drivers/usb/gadget/autostart.c
> @@ -39,8 +39,6 @@ static int usbgadget_autostart(void)
>  	if (!autostart)
>  		return 0;
>  
> -	setenv("otg.mode", "peripheral");
> -
>  	opts = xzalloc(sizeof(*opts));
>  	opts->release = usb_multi_opts_release;
>  
> @@ -60,6 +58,8 @@ static int usbgadget_autostart(void)
>  		return 0;
>  	}
>  
> +	setenv("otg.mode", "peripheral");
> +
>  	ret = usb_multi_register(opts);
>  	if (ret)
>  		usb_multi_opts_release(opts);
> -- 
> 2.11.0
> 
> 

Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] usbgadget: unregister when usb_composite_probe() fails
  2017-09-27  9:12 ` [PATCH 4/4] usbgadget: unregister when usb_composite_probe() fails Sascha Hauer
@ 2017-09-29 13:08   ` Oleksij Rempel
  0 siblings, 0 replies; 9+ messages in thread
From: Oleksij Rempel @ 2017-09-29 13:08 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On Wed, Sep 27, 2017 at 11:12:45AM +0200, Sascha Hauer wrote:
> When usb_multi_register() returns an error it can't be called again
> as we do not unregister the driver properly. Fix this.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/usb/gadget/multi.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
> index 6385c16186..6eeeb4e982 100644
> --- a/drivers/usb/gadget/multi.c
> +++ b/drivers/usb/gadget/multi.c
> @@ -234,6 +234,8 @@ static struct usb_composite_driver multi_driver = {
>  
>  int usb_multi_register(struct f_multi_opts *opts)
>  {
> +	int ret;
> +
>  	if (gadget_multi_opts) {
>  		pr_err("USB multi gadget already registered\n");
>  		return -EBUSY;
> @@ -241,7 +243,13 @@ int usb_multi_register(struct f_multi_opts *opts)
>  
>  	gadget_multi_opts = opts;
>  
> -	return usb_composite_probe(&multi_driver);
> +	ret = usb_composite_probe(&multi_driver);
> +	if (ret) {
> +		usb_composite_unregister(&multi_driver);
> +		gadget_multi_opts = NULL;
> +	}
> +
> +	return ret;
>  }
>  
>  void usb_multi_unregister(void)
> -- 
> 2.11.0

Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-09-29 13:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-27  9:12 usbgadget fix some crashes Sascha Hauer
2017-09-27  9:12 ` [PATCH 1/4] usbgadget: autostart: Handle errors in file list gracefully Sascha Hauer
2017-09-29 13:02   ` Oleksij Rempel
2017-09-27  9:12 ` [PATCH 2/4] usbgadget: do not register when no functions present Sascha Hauer
2017-09-29 13:05   ` Oleksij Rempel
2017-09-27  9:12 ` [PATCH 3/4] usbgadget: only set to peripheral mode when error checking is done Sascha Hauer
2017-09-29 13:06   ` Oleksij Rempel
2017-09-27  9:12 ` [PATCH 4/4] usbgadget: unregister when usb_composite_probe() fails Sascha Hauer
2017-09-29 13:08   ` Oleksij Rempel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox