linux-kernel - Re: [PATCH 5.10.y] fs/jfs: cast inactags to s64 to prevent potential overflow

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: <v3rsldvzq4ujgcxamjwcmmfm62wgdgjscg25c6t6xph3itjedp@sbuayguprw2i>
Date: Wed, 19 Feb 2025 13:38:49 +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 5.10.y] fs/jfs: cast inactags to s64 to prevent potential
 overflow

Hi,

On Wed, 19. Feb 10:25, Rand Deeb wrote:
> The expression "inactags << bmp->db_agl2size" in the function
> dbFinalizeBmap() is computed using int operands. Although the
> values (inactags and db_agl2size) are derived from filesystem
> parameters and are usually small, there is a theoretical risk that
> the shift could overflow a 32-bit int if extreme values occur.
> 
> According to the C standard, shifting a signed 32-bit int can lead
> to undefined behavior if the result exceeds its range. In our
> case, an overflow could miscalculate free blocks, potentially
> leading to erroneous filesystem accounting.
> 
> To ensure the arithmetic is performed in 64-bit space, we cast
> "inactags" to s64 before shifting. This defensive fix prevents any
> risk of overflow and complies with kernel coding best practices.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Rand Deeb <rand.sec96@...il.com>
> ---

Why is the patch targeted only to 5.10.y? It should go to the mainline
first, no?

Please check https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

>  fs/jfs/jfs_dmap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
> index eedea23d70ff..3cc10f9bf9f8 100644
> --- a/fs/jfs/jfs_dmap.c
> +++ b/fs/jfs/jfs_dmap.c
> @@ -3728,8 +3728,8 @@ void dbFinalizeBmap(struct inode *ipbmap)
>  	 * system size is not a multiple of the group size).
>  	 */
>  	inactfree = (inactags && ag_rem) ?
> -	    ((inactags - 1) << bmp->db_agl2size) + ag_rem
> -	    : inactags << bmp->db_agl2size;
> +	    (((s64)inactags - 1) << bmp->db_agl2size) + ag_rem
> +	    : ((s64)inactags << bmp->db_agl2size);
>  
>  	/* determine how many free blocks are in the active
>  	 * allocation groups plus the average number of free blocks
> -- 
> 2.34.1