* [PATCH 2/5] ARM: i.MX: iim: fix potential out of bounds array access
2016-07-06 19:32 [PATCH 1/5] commands: menu: check return pointer properly Lucas Stach
@ 2016-07-06 19:32 ` Lucas Stach
2016-07-06 19:32 ` [PATCH 3/5] ubifs: fix potential memory leak Lucas Stach
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Lucas Stach @ 2016-07-06 19:32 UTC (permalink / raw)
To: barebox
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/5] ubifs: fix potential memory leak
2016-07-06 19:32 [PATCH 1/5] commands: menu: check return pointer properly Lucas Stach
2016-07-06 19:32 ` [PATCH 2/5] ARM: i.MX: iim: fix potential out of bounds array access Lucas Stach
@ 2016-07-06 19:32 ` Lucas Stach
2016-07-07 7:06 ` Sascha Hauer
2016-07-06 19:32 ` [PATCH 4/5] ubifs: fix potential NULL ptr dereference Lucas Stach
` (2 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Lucas Stach @ 2016-07-06 19:32 UTC (permalink / raw)
To: barebox
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/5] ubifs: fix potential memory leak
2016-07-06 19:32 ` [PATCH 3/5] ubifs: fix potential memory leak Lucas Stach
@ 2016-07-07 7:06 ` Sascha Hauer
0 siblings, 0 replies; 8+ messages in thread
From: Sascha Hauer @ 2016-07-07 7:06 UTC (permalink / raw)
To: Lucas Stach; +Cc: barebox
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 4/5] ubifs: fix potential NULL ptr dereference
2016-07-06 19:32 [PATCH 1/5] commands: menu: check return pointer properly Lucas Stach
2016-07-06 19:32 ` [PATCH 2/5] ARM: i.MX: iim: fix potential out of bounds array access Lucas Stach
2016-07-06 19:32 ` [PATCH 3/5] ubifs: fix potential memory leak Lucas Stach
@ 2016-07-06 19:32 ` Lucas Stach
2016-07-07 7:15 ` Sascha Hauer
2016-07-06 19:32 ` [PATCH 5/5] ubifs: check return pointer properly Lucas Stach
2016-07-07 7:46 ` [PATCH 1/5] commands: menu: " Sascha Hauer
4 siblings, 1 reply; 8+ messages in thread
From: Lucas Stach @ 2016-07-06 19:32 UTC (permalink / raw)
To: barebox
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/5] ubifs: fix potential NULL ptr dereference
2016-07-06 19:32 ` [PATCH 4/5] ubifs: fix potential NULL ptr dereference Lucas Stach
@ 2016-07-07 7:15 ` Sascha Hauer
0 siblings, 0 replies; 8+ messages in thread
From: Sascha Hauer @ 2016-07-07 7:15 UTC (permalink / raw)
To: Lucas Stach; +Cc: barebox
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 5/5] ubifs: check return pointer properly
2016-07-06 19:32 [PATCH 1/5] commands: menu: check return pointer properly Lucas Stach
` (2 preceding siblings ...)
2016-07-06 19:32 ` [PATCH 4/5] ubifs: fix potential NULL ptr dereference Lucas Stach
@ 2016-07-06 19:32 ` Lucas Stach
2016-07-07 7:46 ` [PATCH 1/5] commands: menu: " Sascha Hauer
4 siblings, 0 replies; 8+ messages in thread
From: Lucas Stach @ 2016-07-06 19:32 UTC (permalink / raw)
To: barebox
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/5] commands: menu: check return pointer properly
2016-07-06 19:32 [PATCH 1/5] commands: menu: check return pointer properly Lucas Stach
` (3 preceding siblings ...)
2016-07-06 19:32 ` [PATCH 5/5] ubifs: check return pointer properly Lucas Stach
@ 2016-07-07 7:46 ` Sascha Hauer
4 siblings, 0 replies; 8+ messages in thread
From: Sascha Hauer @ 2016-07-07 7:46 UTC (permalink / raw)
To: Lucas Stach; +Cc: barebox
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
^ permalink raw reply [flat|nested] 8+ messages in thread