[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <C4778AD0-1BE5-4AA9-A939-08123334574D@gmail.com>
Date: Wed, 11 Apr 2012 17:15:09 -0400
From: Xi Wang <xi.wang@...il.com>
To: Petr Matousek <pmatouse@...hat.com>
Cc: oss-security@...ts.openwall.com,
Kurt Seifried <kseifried@...hat.com>,
akuster <akuster@...sta.com>,
"Steven M. Christey" <coley@...us.mitre.org>,
vuln@...unia.com
Subject: Re: fix to CVE-2009-4307
On Apr 11, 2012, at 7:07 AM, Petr Matousek wrote:
> Is there any compiler that is used to compile the kernel that turns the
> CVE-2009-4307 fix not working (the groups_per_flex < 2 check)? I
> see that in your commit description you mention equivalent form where
> Clang optimizes away the "groups_per_flex == 0" check. Does Clang
> optimize/change also the "groups_per_flex < 2" check in a similar way?
For current version, no.
> If not, I would not call it a incomplete fix as the issue with zero
> division was fixed. But yes, we'd still want to include the Xi's commit.
I agree. Future compilers might break that, but it's ok for now.
> This is not only compiler specific but also architecture specific if I'm
> not mistaken - on x86 the 1 << x shift can never become zero, whereas on
> for example powerpc it can (for example slw instruction will give a zero
> result when the shift amounts from 32 to 63).
You are right. Actually the bug was found on s930/ppc with fsfuzzer.
https://bugzilla.kernel.org/show_bug.cgi?id=14287
If fsfuzzer were running on x86, it would not have tiggered this
bug. ;-)
You can also find the original patch there.
groups_per_flex = 1 << sbi->s_log_groups_per_flex;
+ /* There are some situations, after shift the value of
+ 'groups_per_flex' can become zero and division with 0
+ will result in fixpoint divide exception
+ */
+ if (groups_per_flex == 0)
+ return 1;+
The check "groups_per_flex == 0" would be optimized away by Clang
since it involves undefined behavior. Fortunately, ext4 developers
changed the original patch a little bit.
http://www.spinics.net/lists/linux-ext4/msg16218.html
The revised patch combines
- an existing check "s_log_groups_per_flex == 0" (that is,
"groups_per_flex == 1") and
- the proposed check "groups_per_flex == 0"
into "groups_per_flex < 2", which current compilers won't kill. ;-)
- xi
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
