linux-kernel - Re: [PATCH 1/1] btrfs: add integer overflow protection to flush_dir_items

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <53686e91-5822-4137-9f79-e4f4d98ff6fb@wdc.com>
Date: Wed, 30 Jul 2025 06:35:54 +0000
From: Johannes Thumshirn <Johannes.Thumshirn@....com>
To: "kmpfqgdwxucqz9@...il.com" <kmpfqgdwxucqz9@...il.com>, David Sterba
	<dsterba@...e.com>
CC: "linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, KernelKraze
	<admin@...l.free-proletariat.dpdns.org>
Subject: Re: [PATCH 1/1] btrfs: add integer overflow protection to
 flush_dir_items_batch allocation

On 7/30/25 6:44 AM, kmpfqgdwxucqz9@...il.com wrote:
> iff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 9f05d454b9df..19b443314db0 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -3655,14 +3655,35 @@ static int flush_dir_items_batch(struct btrfs_trans_handle *trans,
>   	} else {
>   		struct btrfs_key *ins_keys;
>   		u32 *ins_sizes;
> +		size_t keys_size, sizes_size, total_size;
>   
> -		ins_data = kmalloc(count * sizeof(u32) +
> -				   count * sizeof(struct btrfs_key), GFP_NOFS);
> +		/*
> +		 * Prevent integer overflow when calculating allocation size.
> +		 * We use the same reasonable limit as log_delayed_insertion_items()
> +		 * to prevent excessive memory allocation and potential DoS.
> +		 */
> +		if (count > 195) {
> +			btrfs_warn(inode->root->fs_info,
> +				   "dir items batch size %d exceeds safe limit, truncating",
> +				   count);
> +			count = 195;
> +		}

Where does this number come from?


> +		/* Check for overflow in size calculations */
> +		if (check_mul_overflow(count, sizeof(u32), &sizes_size) ||
> +		    check_mul_overflow(count, sizeof(struct btrfs_key), &keys_size) ||
> +		    check_add_overflow(sizes_size, keys_size, &total_size)) {
> +			btrfs_err(inode->root->fs_info,
> +				  "integer overflow in batch allocation size calculation");
> +			return -EOVERFLOW;
> +		}
> +
> +		ins_data = kmalloc(total_size, GFP_NOFS);

Wouldn't kcalloc()  or kmalloc_array() be the better choice here? 
kcalloc() calls  kmalloc_array() which in term does overflow checking.