linux-kernel - Re: [PATCH] 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: <f1c5490e-8d14-416f-bfbc-0041263870f5@oracle.com>
Date: Thu, 20 Feb 2025 09:38:39 -0600
From: Dave Kleikamp <dave.kleikamp@...cle.com>
To: Rand Deeb <rand.sec96@...il.com>, jfs-discussion@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org
Cc: deeb.rand@...fident.ru, lvc-project@...uxtesting.org,
        voskresenski.stanislav@...fident.ru
Subject: Re: [PATCH] fs/jfs: cast inactags to s64 to prevent potential
 overflow

On 2/20/25 3:43AM, 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.

Looks good. I'll add this to jfs-next.

Shaggy

> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Rand Deeb <rand.sec96@...il.com>
> ---
>   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 f9009e4f9ffd..f89f07c9580e 100644
> --- a/fs/jfs/jfs_dmap.c
> +++ b/fs/jfs/jfs_dmap.c
> @@ -3666,8 +3666,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