Re: fix to CVE-2009-4307

[Xi Wang <xi.wang () gmail com>]

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