diff mbox

[U-Boot,V2,3/3] dfu: dfu_get_buf: check the value of env dfu_bufsiz before use

Message ID 1418636051-31901-3-git-send-email-p.marczak@samsung.com
State Accepted
Delegated to: Łukasz Majewski
Headers show

Commit Message

Przemyslaw Marczak Dec. 15, 2014, 9:34 a.m. UTC
In function dfu_get_buf(), the size of allocated buffer could
be defined by the env variable. The size from this variable
was passed for memalign() without checking its value.
And the the memalign will return non null pointer for size 0.

This could possibly cause data abort, so now the value of var
is checked before use. And if this variable is set to 0 then
the default size will be used.

This commit also changes the base passed to simple_strtoul()
to 0. Now decimal and hex values can be used for the variable
dfu_bufsiz.

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
---
Change v2:
- new patch
---
 drivers/dfu/dfu.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Łukasz Majewski Dec. 16, 2014, 1:48 p.m. UTC | #1
Hi Przemyslaw,

> In function dfu_get_buf(), the size of allocated buffer could
> be defined by the env variable. The size from this variable
> was passed for memalign() without checking its value.
> And the the memalign will return non null pointer for size 0.
> 
> This could possibly cause data abort, so now the value of var
> is checked before use. And if this variable is set to 0 then
> the default size will be used.
> 
> This commit also changes the base passed to simple_strtoul()
> to 0. Now decimal and hex values can be used for the variable
> dfu_bufsiz.
> 
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> ---
> Change v2:
> - new patch
> ---
>  drivers/dfu/dfu.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index c0aba6e..49abd85 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -111,8 +111,12 @@ unsigned char *dfu_get_buf(struct dfu_entity
> *dfu) return dfu_buf;
>  
>  	s = getenv("dfu_bufsiz");
> -	dfu_buf_size = s ? (unsigned long)simple_strtol(s, NULL,
> 16) :
> -			CONFIG_SYS_DFU_DATA_BUF_SIZE;
> +	if (s)
> +		dfu_buf_size = (unsigned long)simple_strtol(s, NULL,
> 0); +
> +	if (!s || !dfu_buf_size)
> +		dfu_buf_size = CONFIG_SYS_DFU_DATA_BUF_SIZE;
> +
>  	if (dfu->max_buf_size && dfu_buf_size > dfu->max_buf_size)
>  		dfu_buf_size = dfu->max_buf_size;
>  

Applied to u-boot-dfu, thanks!
Marek Vasut Dec. 16, 2014, 3:09 p.m. UTC | #2
On Tuesday, December 16, 2014 at 02:48:46 PM, Lukasz Majewski wrote:
[...]
> Applied to u-boot-dfu, thanks!

Hi,

Will you have any PR for me for this MW please ? If so, when ?

Best regards,
Marek Vasut
Łukasz Majewski Dec. 16, 2014, 4:07 p.m. UTC | #3
Hi Marek,

> On Tuesday, December 16, 2014 at 02:48:46 PM, Lukasz Majewski wrote:
> [...]
> > Applied to u-boot-dfu, thanks!
> 
> Hi,
> 
> Will you have any PR for me for this MW please ? If so, when ?

Some fixes and clean ups I hope. By the end of current week.

> 
> Best regards,
> Marek Vasut
Marek Vasut Dec. 16, 2014, 5:01 p.m. UTC | #4
On Tuesday, December 16, 2014 at 05:07:06 PM, Lukasz Majewski wrote:
> Hi Marek,
> 
> > On Tuesday, December 16, 2014 at 02:48:46 PM, Lukasz Majewski wrote:
> > [...]
> > 
> > > Applied to u-boot-dfu, thanks!
> > 
> > Hi,
> > 
> > Will you have any PR for me for this MW please ? If so, when ?
> 
> Some fixes and clean ups I hope. By the end of current week.

I pushed an up-to-date u-boot-usb/master tree, so you can use the
up-to-date code.

Hope that helps!

Thanks!

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index c0aba6e..49abd85 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -111,8 +111,12 @@  unsigned char *dfu_get_buf(struct dfu_entity *dfu)
 		return dfu_buf;
 
 	s = getenv("dfu_bufsiz");
-	dfu_buf_size = s ? (unsigned long)simple_strtol(s, NULL, 16) :
-			CONFIG_SYS_DFU_DATA_BUF_SIZE;
+	if (s)
+		dfu_buf_size = (unsigned long)simple_strtol(s, NULL, 0);
+
+	if (!s || !dfu_buf_size)
+		dfu_buf_size = CONFIG_SYS_DFU_DATA_BUF_SIZE;
+
 	if (dfu->max_buf_size && dfu_buf_size > dfu->max_buf_size)
 		dfu_buf_size = dfu->max_buf_size;