[U-Boot] [PATCH v2] usb: gadget: dynamic envstr size in cb_getvar

[Marek Vasut]

On 03/10/2017 12:03 PM, Nicolas le bayon wrote:
> Hi,
> 
> here is a second patch proposal with a dynamic size allocation for evstr in
> cb_getvar.
> 
> Thanks in advance for your feedback/approval.

Please write a sensible commit message ...

Use git send-email to send a patch.

> Best Regards
> Nicolas
> 
> ---
> 
> ---
>  drivers/usb/gadget/f_fastboot.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/f_fastboot.c
> b/drivers/usb/gadget/f_fastboot.c
> index 2160b1c..8b73277 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -432,9 +432,11 @@ static void cb_getvar(struct usb_ep *ep, struct
> usb_request *req)
>   else
>   strcpy(response, "FAILValue not set");
>   } else {
> - char envstr[32];
> + char *envstr;
> 
> - snprintf(envstr, sizeof(envstr) - 1, "fastboot.%s", cmd);
> + envstr = malloc(sizeof("fastboot.%s", cmd) + 1);

You never even compile-tested this, did you ? There is so much wrong
with this patch I wanna cry ...

sizeof() takes one argument, if you bothered compiling the patch,
compiler would tell you.

If you fix the sizeof, it will return 12, always (because 11 chars + \0
at the end of string), so that cannot work. Use strlen to figure out the
size of the fastboot prefix and the size of cmd. Then malloc the correct
size.

malloc() can fail and return NULL, YES THIS REALLY HAPPENS (!!!). ALWAYS
check the return value of malloc, ALWAYS.

> + sprintf(envstr, "fastboot.%s", cmd);
>   s = getenv(envstr);
>   if (s) {
>   strncat(response, s, chars_left);
> 

You're leaking memory because you never free() the allocated mem.

-- 
Best regards,
Marek Vasut