linux-ext4 - Re: [PATCH 2/2] ext4: fix potential use after free issue while fsresize

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]

Date:	Thu, 27 Nov 2014 16:06:35 +0300
From:	Dmitry Monakhov <dmonakhov@...nvz.org>
To:	Theodore Ts'o <tytso@....edu>
Cc:	linux-ext4@...r.kernel.org, cmm@...ibm.com
Subject: Re: [PATCH 2/2] ext4: fix potential use after free issue while fsresize

Theodore Ts'o <tytso@....edu> writes:

> On Fri, Nov 14, 2014 at 01:40:40PM +0400, Dmitry Monakhov wrote:
>> We need some sort of synchronization while updating ->s_group_desc because there
>> are a lot of users which can access old ->s_group_desc array after it was released.
>> It is reasonable to use lightweight seqcount_t here instead of RCU.
>> 
>> Signed-off-by: Dmitry Monakhov <dmonakhov@...nvz.org>
>
> I'm curious --- under what circumstances did you manage to hit this,
> or was this something that you noticed?
>
>> +	do {
>> +		seq = read_seqcount_begin(&sbi->s_group_desc_seq);
>> +		gd_bh = sbi->s_group_desc[group_desc];
>> +	} while (unlikely(read_seqcount_retry(&sbi->s_group_desc_seq, seq)));
>
> The problem is that s_group_desc is allocated using ext4_kvmalloc(),
> which means it's possible that it was allocated using vmalloc().
> Which means that it is possible (although unlikely) that the old
> s_group_desc address could become invalidated after the call to
> ext4_kvfree(o_group_desc).
>
> This will only happen on 32-bit platforms if we get unlucky and the
> kmap region gets recycled right after the vfree(); but we would only
> see the bug in practice if the memory gets kfree'ed gets reused
> immeidately afterwards, and we've been living with this with ext3 for
> a long time.  Don't get me wrong; I'm not saying we should ignore this
> bug, since I certainly agree with you that it is truly a bug.  But if
> we are going to fix it, we should probably fix it all the way, which I
> suspect means we may have to use RCU here.
>
> OTOH, if you are hitting this in live production workloads, then we
> can do the partial fix now, and then fix it all the way up later.  Is
> the RCU mechanism really going to be that ugly?  It's not like we are
> resizing all the time, so we don't need to worry about it being
> heavyweight from a performance point of view, no?
I use seqcount_t because Doc/RCU/array.txt state that it is lighter,
but indeed I've missed vmalloc case so I'll rewrite it to RCU.
>
> 	    	   	       	     	- Ted

Download attachment "signature.asc" of type "application/pgp-signature" (473 bytes)