Re: [U-Boot] [PATCH v2] cmd: fastboot: Validate user input
Hi Lukasz, On Wed, Jul 25, 2018 at 12:22 AM, Lukasz Majewski wrote: > > Hi Sam, > > > On Sat, Jun 30, 2018 at 7:20 AM, Simon Glass wrote: > > > On 29 June 2018 at 11:59, Sam Protsenko > > > wrote: > > >> In case when user provides '-' as USB controller index, like this: > > >> > > >> => fastboot - > > >> > > >> data abort occurs in strcmp() function in do_fastboot(), here: > > >> > > >> if (!strcmp(argv[1], "udp")) > > >> > > >> (tested on BeagleBone Black). > > >> > > >> That's because argv[1] is NULL when user types in the '-', and null > > >> pointer dereference occurs in strcmp() (which is ok according to C > > >> standard specification). So we must validate user input to prevent > > >> such behavior. > > >> > > >> While at it, check also the result of strtoul() function and handle > > >> error cases properly. > > >> > > >> Signed-off-by: Sam Protsenko > > >> --- > > >> Changes for v2: > > >> - replace argv check with argc check > > >> - add mentioning of testing platform in commit message > > >> > > >> cmd/fastboot.c | 13 - > > >> 1 file changed, 12 insertions(+), 1 deletion(-) > > > > > > Reviewed-by: Simon Glass > > > > Hi Lukasz, > > > > Can you please review and merge? > > I've noticed that I was not CC'ed, so I've missed the patch from the > mailing list. > You may consider using patman for sending patches (which adds > recipients automatically). > > > The patch itself seems OK - thanks. > > Reviewed-by: Lukasz Majewski > > I've added it to u-boot-dfu tree. Lets wait for Travis-CI output. > u-boot-usb was merged into master recently, but this patch is still missing in master. Can you please check that for me, I'm afraid it could have been lost... Thanks! > > > > Thanks! > > > > > Best regards, > > Lukasz Majewski > > -- > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] cmd: fastboot: Validate user input
Hi Sam, > On Sat, Jun 30, 2018 at 7:20 AM, Simon Glass wrote: > > On 29 June 2018 at 11:59, Sam Protsenko > > wrote: > >> In case when user provides '-' as USB controller index, like this: > >> > >> => fastboot - > >> > >> data abort occurs in strcmp() function in do_fastboot(), here: > >> > >> if (!strcmp(argv[1], "udp")) > >> > >> (tested on BeagleBone Black). > >> > >> That's because argv[1] is NULL when user types in the '-', and null > >> pointer dereference occurs in strcmp() (which is ok according to C > >> standard specification). So we must validate user input to prevent > >> such behavior. > >> > >> While at it, check also the result of strtoul() function and handle > >> error cases properly. > >> > >> Signed-off-by: Sam Protsenko > >> --- > >> Changes for v2: > >> - replace argv check with argc check > >> - add mentioning of testing platform in commit message > >> > >> cmd/fastboot.c | 13 - > >> 1 file changed, 12 insertions(+), 1 deletion(-) > > > > Reviewed-by: Simon Glass > > Hi Lukasz, > > Can you please review and merge? I've noticed that I was not CC'ed, so I've missed the patch from the mailing list. You may consider using patman for sending patches (which adds recipients automatically). The patch itself seems OK - thanks. Reviewed-by: Lukasz Majewski I've added it to u-boot-dfu tree. Lets wait for Travis-CI output. > > Thanks! Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de pgpU5LrCqo47J.pgp Description: OpenPGP digital signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] cmd: fastboot: Validate user input
On Sat, Jun 30, 2018 at 7:20 AM, Simon Glass wrote: > On 29 June 2018 at 11:59, Sam Protsenko wrote: >> In case when user provides '-' as USB controller index, like this: >> >> => fastboot - >> >> data abort occurs in strcmp() function in do_fastboot(), here: >> >> if (!strcmp(argv[1], "udp")) >> >> (tested on BeagleBone Black). >> >> That's because argv[1] is NULL when user types in the '-', and null >> pointer dereference occurs in strcmp() (which is ok according to C >> standard specification). So we must validate user input to prevent such >> behavior. >> >> While at it, check also the result of strtoul() function and handle >> error cases properly. >> >> Signed-off-by: Sam Protsenko >> --- >> Changes for v2: >> - replace argv check with argc check >> - add mentioning of testing platform in commit message >> >> cmd/fastboot.c | 13 - >> 1 file changed, 12 insertions(+), 1 deletion(-) > > Reviewed-by: Simon Glass Hi Lukasz, Can you please review and merge? Thanks! ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] cmd: fastboot: Validate user input
Hi Tom, If there is no objections here, can we apply this patch? Thanks. On Sat, Jun 30, 2018 at 7:20 AM, Simon Glass wrote: > On 29 June 2018 at 11:59, Sam Protsenko wrote: >> In case when user provides '-' as USB controller index, like this: >> >> => fastboot - >> >> data abort occurs in strcmp() function in do_fastboot(), here: >> >> if (!strcmp(argv[1], "udp")) >> >> (tested on BeagleBone Black). >> >> That's because argv[1] is NULL when user types in the '-', and null >> pointer dereference occurs in strcmp() (which is ok according to C >> standard specification). So we must validate user input to prevent such >> behavior. >> >> While at it, check also the result of strtoul() function and handle >> error cases properly. >> >> Signed-off-by: Sam Protsenko >> --- >> Changes for v2: >> - replace argv check with argc check >> - add mentioning of testing platform in commit message >> >> cmd/fastboot.c | 13 - >> 1 file changed, 12 insertions(+), 1 deletion(-) > > Reviewed-by: Simon Glass ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] cmd: fastboot: Validate user input
On 29 June 2018 at 11:59, Sam Protsenko wrote: > In case when user provides '-' as USB controller index, like this: > > => fastboot - > > data abort occurs in strcmp() function in do_fastboot(), here: > > if (!strcmp(argv[1], "udp")) > > (tested on BeagleBone Black). > > That's because argv[1] is NULL when user types in the '-', and null > pointer dereference occurs in strcmp() (which is ok according to C > standard specification). So we must validate user input to prevent such > behavior. > > While at it, check also the result of strtoul() function and handle > error cases properly. > > Signed-off-by: Sam Protsenko > --- > Changes for v2: > - replace argv check with argc check > - add mentioning of testing platform in commit message > > cmd/fastboot.c | 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2] cmd: fastboot: Validate user input
In case when user provides '-' as USB controller index, like this:
=> fastboot -
data abort occurs in strcmp() function in do_fastboot(), here:
if (!strcmp(argv[1], "udp"))
(tested on BeagleBone Black).
That's because argv[1] is NULL when user types in the '-', and null
pointer dereference occurs in strcmp() (which is ok according to C
standard specification). So we must validate user input to prevent such
behavior.
While at it, check also the result of strtoul() function and handle
error cases properly.
Signed-off-by: Sam Protsenko
---
Changes for v2:
- replace argv check with argc check
- add mentioning of testing platform in commit message
cmd/fastboot.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/cmd/fastboot.c b/cmd/fastboot.c
index e6ae0570d5..ae3a5f627f 100644
--- a/cmd/fastboot.c
+++ b/cmd/fastboot.c
@@ -38,13 +38,18 @@ static int do_fastboot_usb(int argc, char *const argv[],
#if CONFIG_IS_ENABLED(USB_FUNCTION_FASTBOOT)
int controller_index;
char *usb_controller;
+ char *endp;
int ret;
if (argc < 2)
return CMD_RET_USAGE;
usb_controller = argv[1];
- controller_index = simple_strtoul(usb_controller, NULL, 0);
+ controller_index = simple_strtoul(usb_controller, &endp, 0);
+ if (*endp != '\0') {
+ pr_err("Error: Wrong USB controller index format\n");
+ return CMD_RET_FAILURE;
+ }
ret = board_usb_init(controller_index, USB_INIT_DEVICE);
if (ret) {
@@ -120,6 +125,12 @@ NXTARG:
;
}
+ /* Handle case when USB controller param is just '-' */
+ if (argc == 1) {
+ pr_err("Error: Incorrect USB controller index\n");
+ return CMD_RET_USAGE;
+ }
+
fastboot_init((void *)buf_addr, buf_size);
if (!strcmp(argv[1], "udp"))
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
