Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Sat, 2 Apr 2016 22:32:53 -0400
From: Theodore Ts'o <tytso@....edu>
To: Ben Hutchings <ben@...adent.org.uk>
Cc: Yves-Alexis Perez <corsac@...ian.org>, oss-security@...ts.openwall.com,
	Johannes Segitz <jsegitz@...e.com>
Subject: Re: ext4 data corruption due to punch hole races

On Sun, Apr 03, 2016 at 02:37:45AM +0100, Ben Hutchings wrote:
> Regardless of how difficult it is, we probably need to fix the bugs
> somehow in Debian stable.  It looks like the commits are:
> 
> ea3d7209ca01 fix for PUNCH_HOLE (3.0+)
> 17048e8a083f fix for default fallocate (all) and ZERO_RANGE (3.15+)
> 32ebffd3bbb4 fix for COLLAPSE_RANGE (3.15+) and INSERT_RANGE (4.2+)
> 011278485ecc fix for PUNCH_HOLE (3.0+) and ZERO_RANGE (3.15+)
> 
> So the third would not be needed for stable branches up to 3.14 but
> otherwise they're all needed (at least in part) for all live stable
> branches - right?
> 
> (As there are clearly multiple bugs here; why only one CVE ID?)

Commit ea3d7209ca01 fixes the only bug that could result in the
overwriting of another file or directory (which could belong to
someone else, or in the case of a directory could lead to a file
system corruption.)

The other bugs fixes races which could lead to the user's file being
corrupted (some data regions getting replaced by zeros), but it's
strictly speaking not a vulnerability per se, since the user would
have to be trying very hard to hit the race, and it would only result
in the user's own file being corrupted.  Hence, it's probably fair to
consider them as not being vulernabilities, and hence not needing a
CVE.

Furthermore, very few applications call ZERO_RANGE, COLLAPSE_RANGE, or
INSERT_RANGE at all, and those that do are not likely to be trying to
issue racing writes or truncates while they operate on the file using
these fallocate modes.  Hence, I'd consider them low priority bug
fixes to backport.

To the extent that ea3d7209ca01 could result in file system corruption
or another user's file being corrupted, it's clearly the higher
priority one to backport.  Using this as a viable exploit would be
tricky, since you would have to try to hit the race while the other
user was trying to allocate blocks to the file or directory that you
are trying to corrupt.

Still, in a highly specialized circumstance where you had some ACL
that was getting regularly updated at a predictable time interval
(say, /root/.ssh/authorized_keys, from some contralized databased), if
you could manage to arrange to allocate and punch holes in the same
block group as the ACL file would be written to, it probably could be
leveraged into a privilege escalation attack.  While I suspect that
most cyber criminals and the NSA have zero days stocked up that would
allow them to escalate a non-privileged shell up to root access that
would be far easier and less noisy to exploit, it would be good to
backport this commit.

> > If anyone is interested, please contact me.  Otherwise, I'll get to it
> > eventually.
> 
> Since I do most of the security backports for Debian, of course I am
> interested.

That would be great, thanks!  It's been two months since the last time
I've done a comprehensive test of the stable kernels.  Typically
xfstests gets updated with repros of the more critical bugs, so I try
to rerun xfstests on the stable kernels to make sure we haven't missed
any critical backports.  The last time I did the survey, there is one
upstream commit that still needs to be backported to the stable
kernel:

commit 3da40c7b089810ac9cf2bb1e59633f619f3a7312
Author: Josef Bacik <jbacik@...com>
Date:   Mon Jun 22 00:31:26 2015 -0400

    ext4: only call ext4_truncate when size <= isize
    
    At LSF we decided that if we truncate up from isize we shouldn't trim
    fallocated blocks that were fallocated with KEEP_SIZE and are past the
    new i_size.  This patch fixes ext4 to do this.
    
    [ Completely reworked patch so that i_disksize would actually get set
      when truncating up.  Also reworked the code for handling truncate so
      that it's easier to handle. -- tytso ]
    
    Signed-off-by: Josef Bacik <jbacik@...com>
    Signed-off-by: Theodore Ts'o <tytso@....edu>
    Reviewed-by: Lukas Czerner <lczerner@...hat.com>

This is not security critical, but it makes ext4's behavior consistent
with the other file systems, and it fixes an xfstest failure.  With
this exception, the regression test runs were quite clean the last
time I checked, about two months ago.

Cheers,

						 - Ted

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.