Hi Sascha,
On 19.02.25 15:18, Sascha Hauer wrote:
> The pattern malloc(size + constant) is dangerous when size can be
> manipulated by an attacker. In that case 'size' can be manipulated
> in a way that 'size + constant' is 0 due to integer overflow. The
> result is a zero sized buffer to which is then data written to.
>
> Avoid this by using size_add() and struct_size() instead.
>
> Reported-by: Jonathan Bar Or <jonathanbaror@xxxxxxxxx>
> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
With below comment addressed:
Reviewed-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> ---
> fs/jffs2/malloc.c | 4 ++--
> fs/jffs2/nodelist.h | 2 +-
> fs/jffs2/readinode.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/jffs2/malloc.c b/fs/jffs2/malloc.c
> index e0e29fa648..61c2430c18 100644
> --- a/fs/jffs2/malloc.c
> +++ b/fs/jffs2/malloc.c
> @@ -15,10 +15,10 @@
> #include <linux/jffs2.h>
> #include "nodelist.h"
>
> -struct jffs2_full_dirent *jffs2_alloc_full_dirent(int namesize)
> +struct jffs2_full_dirent *jffs2_alloc_full_dirent(size_t namesize)
> {
> struct jffs2_full_dirent *ret;
> - ret = kmalloc(sizeof(*ret) + namesize, GFP_KERNEL);
> + ret = kmalloc(struct_size(ret, name, namesize), GFP_KERNEL);
I was under the impression that struct_size requires the last argument to be
the name of a C99 flexible array member[] and not that of a zero-sized array.
Could you squash a change from struct jffs2_full_dirent::name[0] to ::name[] in
fs/jffs2/nodelist.h into this commit?
Thanks,
Ahmad
> dbg_memalloc("%p\n", ret);
> return ret;
> }
> diff --git a/fs/jffs2/nodelist.h b/fs/jffs2/nodelist.h
> index d8687319c7..28b35d6d58 100644
> --- a/fs/jffs2/nodelist.h
> +++ b/fs/jffs2/nodelist.h
> @@ -440,7 +440,7 @@ void jffs2_do_clear_inode(struct jffs2_sb_info *c, struct jffs2_inode_info *f);
>
> /* malloc.c */
>
> -struct jffs2_full_dirent *jffs2_alloc_full_dirent(int namesize);
> +struct jffs2_full_dirent *jffs2_alloc_full_dirent(size_t namesize);
> void jffs2_free_full_dirent(struct jffs2_full_dirent *);
> struct jffs2_full_dnode *jffs2_alloc_full_dnode(void);
> void jffs2_free_full_dnode(struct jffs2_full_dnode *dnode);
> diff --git a/fs/jffs2/readinode.c b/fs/jffs2/readinode.c
> index 605130d60c..4634ee5818 100644
> --- a/fs/jffs2/readinode.c
> +++ b/fs/jffs2/readinode.c
> @@ -601,7 +601,7 @@ static inline int read_direntry(struct jffs2_sb_info *c, struct jffs2_raw_node_r
> spin_unlock(&c->erase_completion_lock);
> }
>
> - fd = jffs2_alloc_full_dirent(rd->nsize + 1);
> + fd = jffs2_alloc_full_dirent(size_add(rd->nsize, 1));
> if (unlikely(!fd))
> return -ENOMEM;
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
