linux-kernel - Re: [PATCH] fs/jfs: prevent potential integer overflow in dbMapFileSizeToMapSize

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <yogb3soaqqg5vnon6ogryhdmmatrvrc2xwlh2exwlhyf6gnfw7@7tybsy2s52qb>
Date: Tue, 25 Feb 2025 22:52:04 +0300
From: Fedor Pchelkin <pchelkin@...ras.ru>
To: Rand Deeb <rand.sec96@...il.com>
Cc: Dave Kleikamp <shaggy@...nel.org>, 
	jfs-discussion@...ts.sourceforge.net, linux-kernel@...r.kernel.org, 
	voskresenski.stanislav@...fident.ru, deeb.rand@...fident.ru, lvc-project@...uxtesting.org
Subject: Re: [PATCH] fs/jfs: prevent potential integer overflow in
 dbMapFileSizeToMapSize

On Tue, 25. Feb 14:41, Rand Deeb wrote:
> In dbMapFileSizeToMapSize(), the calculation involving 'complete' and
> 'LPERCTL' can result in a 32-bit integer overflow when handling large
> filesystems. Specifically, multiplying 'complete' by LPERCTL * LPERCTL
> (1,048,576) can exceed the 32-bit integer limit if 'complete' is greater
> than approximately 2,047.
> 
> While there is no evidence that 'complete' can exceed this threshold,
> theoretically, this is possible. To ensure robustness and maintainability,
> this patch casts only 'complete' to s64 (64-bit integer) before performing
> the multiplication. This guarantees that the arithmetic is conducted in
> 64-bit space, accommodating larger values without overflow.
> 
> This change enhances the reliability of the JFS filesystem when managing
> large volumes and preemptively addresses potential overflow issues.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Rand Deeb <rand.sec96@...il.com>
> ---
>  fs/jfs/jfs_dmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
> index edb22cf9521a..380e73c516ee 100644
> --- a/fs/jfs/jfs_dmap.c
> +++ b/fs/jfs/jfs_dmap.c
> @@ -4059,7 +4059,7 @@ s64 dbMapFileSizeToMapSize(struct inode * ipbmap)
>  		factor =
>  		    (i == 2) ? MAXL1PAGES : ((i == 1) ? MAXL0PAGES : 1);
>  		complete = (u32) npages / factor;

Can it really overflow the 32bit arithmetic considering how 'factor' and
'complete' values are calculated above?

/*
 * maximum number of map pages at each level including control pages
 */
#define MAXL0PAGES	(1 + LPERCTL)
#define MAXL1PAGES	(1 + LPERCTL * MAXL0PAGES)

> -		ndmaps += complete * ((i == 2) ? LPERCTL * LPERCTL :
> +		ndmaps += (s64)complete * ((i == 2) ? LPERCTL * LPERCTL :
>  				      ((i == 1) ? LPERCTL : 1));
>  
>  		/* pages in last/incomplete child */
> -- 
> 2.34.1