[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4F7BB827.6020501@redhat.com>
Date: Tue, 03 Apr 2012 20:55:35 -0600
From: Kurt Seifried <kseifried@...hat.com>
To: oss-security@...ts.openwall.com
CC: akuster <akuster@...sta.com>, "Steven M. Christey" <coley@...us.mitre.org>
Subject: Re: fix to CVE-2009-4307
On 04/03/2012 04:32 PM, akuster wrote:
> Hello,
>
> Was there a CVE assigned to commit d50f2ab6f050311dbf7b8f5501b25f0bf64a439b?
>
> Commit 503358ae01b70ce6909d19dd01287093f6b6271c ("ext4: avoid divide by
> zero when trying to mount a corrupted file system") fixes CVE-2009-4307
> by performing a sanity check on s_log_groups_per_flex, since it can be
> set to a bogus value by an attacker.
>
> - Armin
I assume you are talking about this:
http://git.kernel.org/?p=virt/kvm/kvm.git;a=commitdiff;h=d50f2ab6f050311dbf7b8f5501b25f0bf64a439b
================================================
Commit 503358ae01b70ce6909d19dd01287093f6b6271c ("ext4: avoid divide by
zero when trying to mount a corrupted file system") fixes CVE-2009-4307
by performing a sanity check on s_log_groups_per_flex, since it can be
set to a bogus value by an attacker.
sbi->s_log_groups_per_flex = sbi->s_es->s_log_groups_per_flex;
groups_per_flex = 1 << sbi->s_log_groups_per_flex;
if (groups_per_flex < 2) { ... }
This patch fixes two potential issues in the previous commit.
1) The sanity check might only work on architectures like PowerPC.
On x86, 5 bits are used for the shifting amount. That means, given a
large s_log_groups_per_flex value like 36, groups_per_flex = 1 << 36
is essentially 1 << 4 = 16, rather than 0. This will bypass the check,
leaving s_log_groups_per_flex and groups_per_flex inconsistent.
2) The sanity check relies on undefined behavior, i.e., oversized shift.
A standard-confirming C compiler could rewrite the check in unexpected
ways. Consider the following equivalent form, assuming groups_per_flex
is unsigned for simplicity.
groups_per_flex = 1 << sbi->s_log_groups_per_flex;
if (groups_per_flex == 0 || groups_per_flex == 1) {
We compile the code snippet using Clang 3.0 and GCC 4.6. Clang will
completely optimize away the check groups_per_flex == 0, leaving the
patched code as vulnerable as the original. GCC keeps the check, but
there is no guarantee that future versions will do the same.
Signed-off-by: Xi Wang <xi.wang@...il.com>
Signed-off-by: "Theodore Ts'o" <tytso@....edu>
Cc: stable@...r.kernel.org
================================================
What specific do you want a CVE assigned for?
For #1 I can see a CVE of the "a previous patch didn't completely fix
the issue, yada yada" type.
For #2 I'm not sure how we handle something like a compiler possibly
mangling code so that an issue is introduced (is that a compiler
problem? a code problem? the intersection of both? Steve: can I get a
comment/referees decision here =)
--
Kurt Seifried Red Hat Security Response Team (SRT)
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.
