Re: [PATCH 4/5] CVE-2025-26725: fs: jffs2: fix malloc(size + constant) buffer overflow issues

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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@gmail.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

With below comment addressed:
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  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 |