linux-ext4 - Re: [PATCH, RFC V2] ext4: limit block allocations for indirect-block files to

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 for Android: free password hash cracker in your pocket

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

Date:	Sat, 05 Sep 2009 18:45:35 +0200
From:	Andreas Dilger <adilger@....com>
To:	Eric Sandeen <sandeen@...hat.com>
Cc:	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH, RFC V2] ext4: limit block allocations for	indirect-block
 files to < 2^32

On Sep 04, 2009  22:21 -0500, Eric Sandeen wrote:
> Today, the ext4 allocator will happily allocate blocks past
> 232 for indirect-block files, which results in the block
> numbers getting truncated, and corruption ensues.
>
> This patch limits such allocations to < 2^32, and adds
> WARN_ONs (maybe should be BUG_ONs) if we do get blocks
> larger than that.

Eric, thanks for making the patch.

> This should address RH Bug 519471, ext4 bitmap allocator must limit 
> blocks to < 2^32
>
> * ext4_find_goal() is modified to choose a goal < UINT_MAX,
>  so that our starting point is in an acceptable range.
>
> * ext4_xattr_block_set() is modified such that the goal block
>  is < UINT_MAX, as above.

Using UINT_MAX probably isn't wholly safe, as I know of systems
that have e.g. 64-bit ints (though I guess none that have Linux
kernel ports).  It should use (u32)~0 or ((1 << 32) - 1) directly.

> Perhaps an ext4-specific #define would be better than UINT_MAX?

I think yes, since we know the maximum value is tied specifically
to the u32 indirect block pointers, and not necessarily to an "int".

> static ext4_fsblk_t ext4_find_goal(struct inode *inode, ext4_lblk_t block,
> 				   Indirect *partial)
> {
> +	goal = ext4_find_near(inode, partial);
> +	goal = goal % UINT_MAX;
> +	return goal;

Using "% UINT_MAX" here will result in a 64-bit division on 32-bit
platforms, since ext4_fsblk_t is declared as an unsigned long long.
This should instead be "(u32)" or "& 0xffffffff".

> @@ -1943,6 +1943,11 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac)
> +	/* non-extent files are limited to low blocks/groups */
> +	if (!(EXT4_I(ac->ac_inode)->i_flags & EXT4_EXTENTS_FL))
> +		ngroups = min_t(unsigned long, ngroups,
> +				(UINT_MAX / EXT4_BLOCKS_PER_GROUP(sb)));

Since EXT4_BLOCKS_PER_GROUP() is a run-time variable, but is constant
for the life of the filesystem, this could be computed once and stored
in the superblock?

> +++ b/fs/ext4/xattr.c
> @@ -810,12 +810,22 @@ inserted:
> +			if (!(EXT4_I(inode)->i_flags & EXT4_EXTENTS_FL))
> +				goal = goal % UINT_MAX;

As above.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html