[U-Boot] [PATCH v7] usb: gadget: avoid variable name clipping in cb_getvar

[U-Boot] [PATCH v7] usb: gadget: avoid variable name clipping in cb_getvar

[nicolas.le.bayon at st.com]

From: Nicolas Le Bayon <nicolas.le.bayon@st.com>

Instead of using a fixed-size array to store variable name, preferring a
dynamic allocation treats correctly all variable name lengths.
Variable names are growing through releases and features. By this way, name
clipping is prevented.

Signed-off-by: Nicolas Le Bayon <nicolas.le.bayon@st.com>
Reviewed-by: Marek Vasut <marex@denx.de>
Acked-by: Lukasz Majewski <lukma@denx.de>
---

Changes in v2:
 - instead of using a bigger fixed size, use malloc to fit with size needs
Changes in v3:
 - v2 was an error (intermediate version), so propose a complete one
Changes in v4:
 - be more explicit and detailed in label and description fields
 - remove intermediate variable only used one time
 - be more explicit in error message
 - fix indent issue
Changes in v5:
 - drop an unuseful error() call
Changes in v6:
 - add Marek review approval
Changes in v7:
 - add Lukasz ack approval

 drivers/usb/gadget/f_fastboot.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index 2160b1c..7cd6d24 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -432,9 +432,15 @@ 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(strlen("fastboot.") + strlen(cmd) + 1);
+		if (!envstr) {
+			fastboot_tx_write_str("FAILmalloc error");
+			return;
+		}
+
+		sprintf(envstr, "fastboot.%s", cmd);
 		s = getenv(envstr);
 		if (s) {
 			strncat(response, s, chars_left);
@@ -442,6 +448,8 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req)
 			printf("WARNING: unknown variable: %s\n", cmd);
 			strcpy(response, "FAILVariable not implemented");
 		}
+
+		free(envstr);
 	}
 	fastboot_tx_write_str(response);
 }
-- 
1.9.1

[U-Boot] [PATCH v7] usb: gadget: avoid variable name clipping in cb_getvar

[Nicolas LE BAYON]

Hi,

A kind reminder to look at this patch (already reviewed by Marek and acked by Lukasz), and if possible to put it in the next pull list, or the one after is timing is too short.

Thanks in advance for your time

Best Regards
Nicolas

-----Original Message-----
From: Nicolas LE BAYON 
Sent: mardi 25 avril 2017 10:18
To: Nicolas LE BAYON <nicolas.le.bayon@st.com>; u-boot at lists.denx.de; lukma at denx.de; marex at denx.de
Cc: nlebayon at gmail.com; Patrice CHOTARD <patrice.chotard@st.com>; Jean-philippe ROMAIN <jean-philippe.romain@st.com>
Subject: [U-Boot][PATCH v7] usb: gadget: avoid variable name clipping in cb_getvar

From: Nicolas Le Bayon <nicolas.le.bayon@st.com>

Instead of using a fixed-size array to store variable name, preferring a dynamic allocation treats correctly all variable name lengths.
Variable names are growing through releases and features. By this way, name clipping is prevented.

Signed-off-by: Nicolas Le Bayon <nicolas.le.bayon@st.com>
Reviewed-by: Marek Vasut <marex@denx.de>
Acked-by: Lukasz Majewski <lukma@denx.de>
---

Changes in v2:
 - instead of using a bigger fixed size, use malloc to fit with size needs Changes in v3:
 - v2 was an error (intermediate version), so propose a complete one Changes in v4:
 - be more explicit and detailed in label and description fields
 - remove intermediate variable only used one time
 - be more explicit in error message
 - fix indent issue
Changes in v5:
 - drop an unuseful error() call
Changes in v6:
 - add Marek review approval
Changes in v7:
 - add Lukasz ack approval

 drivers/usb/gadget/f_fastboot.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c index 2160b1c..7cd6d24 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -432,9 +432,15 @@ 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(strlen("fastboot.") + strlen(cmd) + 1);
+		if (!envstr) {
+			fastboot_tx_write_str("FAILmalloc error");
+			return;
+		}
+
+		sprintf(envstr, "fastboot.%s", cmd);
 		s = getenv(envstr);
 		if (s) {
 			strncat(response, s, chars_left);
@@ -442,6 +448,8 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req)
 			printf("WARNING: unknown variable: %s\n", cmd);
 			strcpy(response, "FAILVariable not implemented");
 		}
+
+		free(envstr);
 	}
 	fastboot_tx_write_str(response);
 }
--
1.9.1

[U-Boot] [PATCH v7] usb: gadget: avoid variable name clipping in cb_getvar

[Lukasz Majewski]

On Tue, 9 May 2017 15:58:36 +0000
Nicolas LE BAYON <nicolas.le.bayon@st.com> wrote:

> Hi,
> 
> A kind reminder to look at this patch (already reviewed by Marek and
> acked by Lukasz), and if possible to put it in the next pull list, or
> the one after is timing is too short.

I wanted to add this patch with other ones. Unfortunately those other
patches needed more review, so I postponed the PR.

I will ask Tom to take this one directly.



> 
> Thanks in advance for your time
> 
> Best Regards
> Nicolas
> 
> -----Original Message-----
> From: Nicolas LE BAYON 
> Sent: mardi 25 avril 2017 10:18
> To: Nicolas LE BAYON <nicolas.le.bayon@st.com>; u-boot at lists.denx.de;
> lukma at denx.de; marex at denx.de Cc: nlebayon at gmail.com; Patrice CHOTARD
> <patrice.chotard@st.com>; Jean-philippe ROMAIN
> <jean-philippe.romain@st.com> Subject: [U-Boot][PATCH v7] usb:
> gadget: avoid variable name clipping in cb_getvar
> 
> From: Nicolas Le Bayon <nicolas.le.bayon@st.com>
> 
> Instead of using a fixed-size array to store variable name,
> preferring a dynamic allocation treats correctly all variable name
> lengths. Variable names are growing through releases and features. By
> this way, name clipping is prevented.
> 
> Signed-off-by: Nicolas Le Bayon <nicolas.le.bayon@st.com>
> Reviewed-by: Marek Vasut <marex@denx.de>
> Acked-by: Lukasz Majewski <lukma@denx.de>
> ---
> 
> Changes in v2:
>  - instead of using a bigger fixed size, use malloc to fit with size
> needs Changes in v3:
>  - v2 was an error (intermediate version), so propose a complete one
> Changes in v4:
>  - be more explicit and detailed in label and description fields
>  - remove intermediate variable only used one time
>  - be more explicit in error message
>  - fix indent issue
> Changes in v5:
>  - drop an unuseful error() call
> Changes in v6:
>  - add Marek review approval
> Changes in v7:
>  - add Lukasz ack approval
> 
>  drivers/usb/gadget/f_fastboot.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/f_fastboot.c
> b/drivers/usb/gadget/f_fastboot.c index 2160b1c..7cd6d24 100644 ---
> a/drivers/usb/gadget/f_fastboot.c +++
> b/drivers/usb/gadget/f_fastboot.c @@ -432,9 +432,15 @@ 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(strlen("fastboot.") + strlen(cmd) +
> 1);
> +		if (!envstr) {
> +			fastboot_tx_write_str("FAILmalloc error");
> +			return;
> +		}
> +
> +		sprintf(envstr, "fastboot.%s", cmd);
>  		s = getenv(envstr);
>  		if (s) {
>  			strncat(response, s, chars_left);
> @@ -442,6 +448,8 @@ static void cb_getvar(struct usb_ep *ep, struct
> usb_request *req) printf("WARNING: unknown variable: %s\n", cmd);
>  			strcpy(response, "FAILVariable not
> implemented"); }
> +
> +		free(envstr);
>  	}
>  	fastboot_tx_write_str(response);
>  }
> --
> 1.9.1
> 




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: wd at denx.de

[U-Boot] [U-Boot, v7] usb: gadget: avoid variable name clipping in cb_getvar

[Tom Rini]

On Tue, May 09, 2017 at 03:58:36PM +0000, nicolas.le.bayon at st.com wrote:

> Hi,
> 
> A kind reminder to look at this patch (already reviewed by Marek and acked by Lukasz), and if possible to put it in the next pull list, or the one after is timing is too short.
> 
> Thanks in advance for your time
> 
> Best Regards
> Nicolas
> 

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170512/f79cf58c/attachment.sig>