linux-ext4 - Re: [PATCH] ext4: fix null pointer deref on mount

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]

Date:	Mon, 05 Jan 2009 23:50:39 +0100
From:	Thiemo Nagel <thiemo.nagel@...tum.de>
To:	Theodore Tso <tytso@....edu>
CC:	Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] ext4: fix null pointer deref on mount

Theodore Tso wrote:
> On Mon, Jan 05, 2009 at 09:50:13PM +0100, Thiemo Nagel wrote:
>> I have chosen unsigned long for the sole reason to avoid truncation in  
>> the assignment
>>
>> db_count = (sbi->s_groups_count + EXT4_DESC_PER_BLOCK(sb) - 1) /
>> 	   EXT4_DESC_PER_BLOCK(sb);
>>
>> where the operands on the right side are of type unsigned long and  
>> ext4_group_t (which is typedef unsigned long), so I don't think to make  
>> db_count an unsigned long is hurting anything.
> 
> Err, no.  ext4_group_t is typedef'ed to be an unsigned int.  

fs/ext4/ext4_i.h, line 34:
typedef unsigned long ext4_group_t;

> And there
> are plenty of places in both the kernel and userspace code where the
> number of groups is assumed to a quantity that can be held in a 2**32
> bit field.  This isn't a problem, because normally the number of
> blocks per group is fs->blocksize*8.  So for a 4k block filesystem,
> the number of blocks per group is 32768, or 2**15.  So that means an
> effective limit of 2**47 blocks before we overflow 2**32 block group
> type width, and with 4k blocks, that means a max volume size of 512
> petabytes.   
> 
>> But maybe it's not desireable to allow filesystems which are mountable  
>> on x86_64 but not on x86_32?  Then a different solution would be to  
>> enforce s_groups_count < (1<<31).
> 
> I'd say enforce s_groups_count < 2**32, because that's the limit we
> have everywhere else.

Fine.

I had another look at the kmalloc() call:  It's not a problem because 
s_groups_count is divided by s_desc_per_block, which always is larger 
than the pointer size.

I have updated the

Patch: fix null pointer dereference on mount

Enforce 1 <= s_groups_count <= 2**32 - s_desc_per_block to avoid invalid 
memory accesses later in the code.

Signed-off-by: Thiemo Nagel <thiemo.nagel@...tum.de>


View attachment "null_deref.patch2" of type "text/plain" (1650 bytes)