[PATCH 1/5] commands: menu: check return pointer properly

[PATCH 1/5] commands: menu: check return pointer properly

[Lucas Stach]

The called functions return error codes encoded in the pointer,
so the NULL check will never be true.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 commands/menu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commands/menu.c b/commands/menu.c
index 9ec2d57..e1079fd 100644
--- a/commands/menu.c
+++ b/commands/menu.c
@@ -83,7 +83,7 @@ static int do_menu_entry_add(struct cmd_menu *cm)
 	else
 		me = menu_add_command_entry(m, cm->description, cm->command,
 					    cm->type);
-	if (!me)
+	if (IS_ERR(me))
 		return PTR_ERR(me);
 
 	me->box_state = cm->box_state > 0 ? 1 : 0;
-- 
2.7.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/5] ARM: i.MX: iim: fix potential out of bounds array access

[Lucas Stach]

The check is off-by-one.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 arch/arm/mach-imx/iim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/iim.c b/arch/arm/mach-imx/iim.c
index c3ba67e..6addfed 100644
--- a/arch/arm/mach-imx/iim.c
+++ b/arch/arm/mach-imx/iim.c
@@ -196,7 +196,7 @@ int imx_iim_read(unsigned int banknum, int offset, void *buf, int count)
 	if (!imx_iim)
 		return -ENODEV;
 
-	if (banknum > IIM_NUM_BANKS)
+	if (banknum >= IIM_NUM_BANKS)
 		return -EINVAL;
 
 	bank = iim->bank[banknum];
-- 
2.7.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/5] ubifs: fix potential memory leak

[Lucas Stach]

Need to go through the regular error path in order to free
"buf" correctly.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 fs/ubifs/lprops.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/ubifs/lprops.c b/fs/ubifs/lprops.c
index 28a1d3d..f880a89 100644
--- a/fs/ubifs/lprops.c
+++ b/fs/ubifs/lprops.c
@@ -1095,14 +1095,16 @@ static int scan_check_cb(struct ubifs_info *c,
 		lst->empty_lebs += 1;
 		lst->total_free += c->leb_size;
 		lst->total_dark += ubifs_calc_dark(c, c->leb_size);
-		return LPT_SCAN_CONTINUE;
+		ret = LPT_SCAN_CONTINUE;
+		goto out;
 	}
 	if (lp->free + lp->dirty == c->leb_size &&
 	    !(lp->flags & LPROPS_INDEX)) {
 		lst->total_free  += lp->free;
 		lst->total_dirty += lp->dirty;
 		lst->total_dark  +=  ubifs_calc_dark(c, c->leb_size);
-		return LPT_SCAN_CONTINUE;
+		ret = LPT_SCAN_CONTINUE;
+		goto out;
 	}
 
 	sleb = ubifs_scan(c, lnum, 0, buf, 0);
-- 
2.7.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/5] ubifs: fix potential NULL ptr dereference

[Lucas Stach]

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 fs/ubifs/ubifs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
index 8062baa..bc1b521 100644
--- a/fs/ubifs/ubifs.c
+++ b/fs/ubifs/ubifs.c
@@ -479,7 +479,7 @@ out:
 		dbg_gen("cannot find next direntry, error %d", err);
 
 out_free:
-	if (file->private_data)
+	if (file && file->private_data)
 		kfree(file->private_data);
 	if (file)
 		free(file);
-- 
2.7.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/5] ubifs: check return pointer properly

[Lucas Stach]

ubifs_iget() returns error codes encoded in the pointer,
so the NULL check will never be true.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 fs/ubifs/ubifs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
index bc1b521..47eef7c 100644
--- a/fs/ubifs/ubifs.c
+++ b/fs/ubifs/ubifs.c
@@ -533,7 +533,7 @@ static unsigned long ubifs_findfile(struct super_block *sb, const char *filename
 			return 0;
 		inode = ubifs_iget(sb, inum);
 
-		if (!inode)
+		if (IS_ERR(inode))
 			return 0;
 		ui = ubifs_inode(inode);
 
@@ -1001,7 +1001,7 @@ static int ubifs_open(struct device_d *dev, FILE *file, const char *filename)
 		return -ENOENT;
 
 	inode = ubifs_iget(priv->sb, inum);
-	if (!inode)
+	if (IS_ERR(inode))
 		return -ENOENT;
 
 	uf = xzalloc(sizeof(*uf));
@@ -1126,7 +1126,7 @@ static DIR *ubifs_opendir(struct device_d *dev, const char *pathname)
 		return NULL;
 
 	inode = ubifs_iget(priv->sb, inum);
-	if (!inode)
+	if (IS_ERR(inode))
 		return NULL;
 
 	ubifs_iput(inode);
@@ -1206,7 +1206,7 @@ static int ubifs_stat(struct device_d *dev, const char *filename, struct stat *s
 		return -ENOENT;
 
 	inode = ubifs_iget(priv->sb, inum);
-	if (!inode)
+	if (IS_ERR(inode))
 		return -ENOENT;
 
 	s->st_size = inode->i_size;
-- 
2.7.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/5] ubifs: fix potential memory leak

[Sascha Hauer]

On Wed, Jul 06, 2016 at 09:32:50PM +0200, Lucas Stach wrote:
> Need to go through the regular error path in order to free
> "buf" correctly.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  fs/ubifs/lprops.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ubifs/lprops.c b/fs/ubifs/lprops.c
> index 28a1d3d..f880a89 100644
> --- a/fs/ubifs/lprops.c
> +++ b/fs/ubifs/lprops.c
> @@ -1095,14 +1095,16 @@ static int scan_check_cb(struct ubifs_info *c,
>  		lst->empty_lebs += 1;
>  		lst->total_free += c->leb_size;
>  		lst->total_dark += ubifs_calc_dark(c, c->leb_size);
> -		return LPT_SCAN_CONTINUE;
> +		ret = LPT_SCAN_CONTINUE;
> +		goto out;
>  	}
>  	if (lp->free + lp->dirty == c->leb_size &&
>  	    !(lp->flags & LPROPS_INDEX)) {
>  		lst->total_free  += lp->free;
>  		lst->total_dirty += lp->dirty;
>  		lst->total_dark  +=  ubifs_calc_dark(c, c->leb_size);
> -		return LPT_SCAN_CONTINUE;
> +		ret = LPT_SCAN_CONTINUE;
> +		goto out;
>  	}
>  
>  	sleb = ubifs_scan(c, lnum, 0, buf, 0);

"buf' is only used after these two if() blocks, so it should simply be
allocated afterwards.

The same bug is also present in the kernel, so you might want to send it
there aswell.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 4/5] ubifs: fix potential NULL ptr dereference

[Sascha Hauer]

On Wed, Jul 06, 2016 at 09:32:51PM +0200, Lucas Stach wrote:
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  fs/ubifs/ubifs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
> index 8062baa..bc1b521 100644
> --- a/fs/ubifs/ubifs.c
> +++ b/fs/ubifs/ubifs.c
> @@ -479,7 +479,7 @@ out:
>  		dbg_gen("cannot find next direntry, error %d", err);
>  
>  out_free:
> -	if (file->private_data)
> +	if (file && file->private_data)
>  		kfree(file->private_data);
>  	if (file)
>  		free(file);

We should rather use xzalloc, then we could drop all the if() in the
error path.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/5] commands: menu: check return pointer properly

[Sascha Hauer]

Hi Lucas,

I applied all patches from this and the other series I haven't commented
on.

Sascha

On Wed, Jul 06, 2016 at 09:32:48PM +0200, Lucas Stach wrote:
> The called functions return error codes encoded in the pointer,
> so the NULL check will never be true.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  commands/menu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/commands/menu.c b/commands/menu.c
> index 9ec2d57..e1079fd 100644
> --- a/commands/menu.c
> +++ b/commands/menu.c
> @@ -83,7 +83,7 @@ static int do_menu_entry_add(struct cmd_menu *cm)
>  	else
>  		me = menu_add_command_entry(m, cm->description, cm->command,
>  					    cm->type);
> -	if (!me)
> +	if (IS_ERR(me))
>  		return PTR_ERR(me);
>  
>  	me->box_state = cm->box_state > 0 ? 1 : 0;
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox