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
| ||
|
Date: Thu, 7 Jul 2016 22:10:40 +0200
From: Vegard Nossum <vegard.nossum@...cle.com>
To: "Darrick J. Wong" <darrick.wong@...cle.com>,
"Theodore Ts'o" <tytso@....edu>
Cc: Ext4 Developers List <linux-ext4@...r.kernel.org>,
linux-fsdevel@...r.kernel.org
Subject: Re: [RFC PATCH] ext4: validate number of meta clusters in group
On 07/02/2016 09:49 AM, Darrick J. Wong wrote:
> On Fri, Jul 01, 2016 at 03:06:41PM +0200, Vegard Nossum wrote:
>> Hi,
>>
>> I've found that sbi->s_es->s_reserved_gdt_blocks is not validated before
>> being used, so e.g. a value of 25600 will overflow the buffer head and
>> corrupt random kernel memory (I've observed 20+ different stack traces
>> due to this bug! many of them long after the code has finished).
>
> This function merely initializes a bitmap block once ext4 decides to start
> using the block group, which could be a long time after mount...
>
>> The following patch fixes it for me:
>>
>> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
>> index 3020fd7..1ea5054 100644
>> --- a/fs/ext4/balloc.c
>> +++ b/fs/ext4/balloc.c
>> @@ -208,6 +208,9 @@ static int ext4_init_block_bitmap(struct super_block
>> *sb,
>> memset(bh->b_data, 0, sb->s_blocksize);
>>
>> bit_max = ext4_num_base_meta_clusters(sb, block_group);
>> + if ((bit_max >> 3) >= bh->b_size)
>> + return -EFSCORRUPTED;
>> +
>> for (bit = 0; bit < bit_max; bit++)
>> ext4_set_bit(bit, bh->b_data);
>>
>> However, I think there are potentially more bugs later in this function
>> where offsets are not validated so it needs to be reviewed carefully.
>>
>> Another question is whether we should do the validation earlier, e.g. in
>> ext4_fill_super(). I'm not too familiar with the code, but maybe
>> something like the attached patch would be better? It does seem to fix
>> the issue as well.
>
> ...whereas superblock fields such as s_reserved_gdt_blocks ought to be
> checked (and -EFSCORRUPTED'ed) at mount time. Since we know that BG 0
> has everything (superblock, old school gdt tables, inodes), maybe we
> should make mount check that ext4_num_base_meta_clusters() >
> NBBY * fs->block_size?
>
> (Kinda surprised ext4 doesn't already do this...)
I ran into a second problem (this time it was num_clusters_in_group()
returning a bogus value) with the same symptoms (random memory
corruptions), the new attached patch fixes both problems by checking the
values at mount time.
I'm using (bit_max >> 3) >= sb->s_blocksize which should be equivalent
to what you suggested (num_clusters() > NBBY * fs->block_size).
Vegard
View attachment "0001-ext4-validate-number-of-clusters-in-group.patch" of type "text/x-patch" (4426 bytes)
Powered by blists - more mailing lists