Hi Simon,
On 27 June 2018 at 02:18, Simon Glass <s...@chromium.org> wrote:
> Hi Sam,
>
> On 26 June 2018 at 10:20, Sam Protsenko <semen.protse...@linaro.org> 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"))
>>
>> 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 <semen.protse...@linaro.org>
>> ---
>> cmd/fastboot.c | 13 ++++++++++++-
>> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> Reviewed-by: Simon Glass <s...@chromium.org>
>
> Question below.
>
>>
>> diff --git a/cmd/fastboot.c b/cmd/fastboot.c
>> index e6ae0570d5..dab686eef4 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 (!argv[1]) {
>
> Can you check argc instead? I think that is nicer.
>
You are right. Code above my changes modifies the argc w.r.t. argv, so
we can check if argc == 1, and this would be the error case. I just
sent the v2. Thanks for reviewing.
>> + 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
