| Message ID | 20220624192729.1893873-1-miquel.raynal@bootlin.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | fs/sqashfs: Use kcalloc when relevant | expand |
On Fri, Jun 24, 2022 at 09:27:29PM +0200, Miquel Raynal wrote: > A crafted squashfs image could embed a huge number of empty metadata > blocks in order to make the amount of malloc()'d memory overflow and be > much smaller than expected. Because of this flaw, any random code > positioned at the right location in the squashfs image could be memcpy'd > from the squashfs structures into U-Boot code location while trying to > access the rearmost blocks, before being executed. > > In order to prevent this vulnerability from being exploited in eg. a > secure boot environment, let's add a check over the amount of data > that is going to be allocated. Such a check could look like: > > if (!elem_size || n > SIZE_MAX / elem_size) > return NULL; > > The right way to do it would be to enhance the calloc() implementation > but this is quite an impacting change for such a small fix. Another > solution would be to add the check before the malloc call in the > squashfs implementation, but this does not look right. So for now, let's > use the kcalloc() compatibility function from Linux, which has this > check. > > Reported-by: Tatsuhiko Yasumatsu <Tatsuhiko.Yasumatsu@sony.com> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> Reviewed-by: Tom Rini <trini@konsulko.com>
diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c index c1489966112..ee648ad425e 100644 --- a/fs/squashfs/sqfs.c +++ b/fs/squashfs/sqfs.c @@ -14,6 +14,7 @@ #include <linux/types.h> #include <linux/byteorder/little_endian.h> #include <linux/byteorder/generic.h> +#include <linux/compat.h> #include <memalign.h> #include <stdlib.h> #include <string.h> @@ -727,7 +728,8 @@ static int sqfs_read_inode_table(unsigned char **inode_table) goto free_itb; } - *inode_table = malloc(metablks_count * SQFS_METADATA_BLOCK_SIZE); + *inode_table = kcalloc(metablks_count, SQFS_METADATA_BLOCK_SIZE, + GFP_KERNEL); if (!*inode_table) { ret = -ENOMEM; printf("Error: failed to allocate squashfs inode_table of size %i, increasing CONFIG_SYS_MALLOC_LEN could help\n",
A crafted squashfs image could embed a huge number of empty metadata blocks in order to make the amount of malloc()'d memory overflow and be much smaller than expected. Because of this flaw, any random code positioned at the right location in the squashfs image could be memcpy'd from the squashfs structures into U-Boot code location while trying to access the rearmost blocks, before being executed. In order to prevent this vulnerability from being exploited in eg. a secure boot environment, let's add a check over the amount of data that is going to be allocated. Such a check could look like: if (!elem_size || n > SIZE_MAX / elem_size) return NULL; The right way to do it would be to enhance the calloc() implementation but this is quite an impacting change for such a small fix. Another solution would be to add the check before the malloc call in the squashfs implementation, but this does not look right. So for now, let's use the kcalloc() compatibility function from Linux, which has this check. Reported-by: Tatsuhiko Yasumatsu <Tatsuhiko.Yasumatsu@sony.com> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- fs/squashfs/sqfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)