Bug 66661 - Possible size overflow in tree-log.c
Summary: Possible size overflow in tree-log.c
Status: RESOLVED CODE_FIX
Alias: None
Product: File System
Classification: Unclassified
Component: btrfs (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Josef Bacik
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-05 22:28 UTC by Jens Binnewies
Modified: 2013-12-18 13:37 UTC (History)
2 users (show)

See Also:
Kernel Version: 3.12.2
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Jens Binnewies 2013-12-05 22:28:50 UTC
hey guys,

i tried to use the current 3.12.2 linux kernel with the latest grsecurity patchset available for this kernel.

i've built my vanilla kernel + grsecurity patchset and rebooted into my debian system. After issuing an 'apt-get update' im getting the following error, triggered by pax, a memory protection:


http://www.rootfabrik.com/c03a3cec5f47910ef4bc4204d8a8c443/20131203_182543.jpg


The guys at grsecurity let me modify some code to debug this issue in detail and they think that the core-developers of the btrfs should take a closer look on this issue, because it could be a real overflow and not an false positive.

The main-statement of grsecurity for this issue:



"
ok, this is something you'll have to work out with upstream because it looks like some logic error. what happened was that fs/btrfs/tree-log.c:log_one_extent was called with an extent_map whose block_start field had a special value (EXTENT_MAP_HOLE/ffff_ffff_ffff_fffd) instead of some real block number yet btrfs_lookup_csums_range ended up getting called with this value and the overflow plugin triggered on the resulting overflow when block_start+csum_offset+csum_len was calculated. now i know nothing about btrfs internals but my gut feeling is that btrfs_lookup_csums_range expects valid ranges, not something computed from the magic values of EXTENT_MAP_*.
"


The whole discussion, including logfiles and screenshots, is here: 

http://forums.grsecurity.net/viewtopic.php?f=1&t=3887


Maybe this will help you guys :)



kind regards and many thanks, Jens
Comment 1 Jens Binnewies 2013-12-11 18:48:01 UTC
The bug also exists in the latest kernel 3.12.4.

please take a look at this bug. the system is unusable.
Comment 2 David Sterba 2013-12-12 15:09:29 UTC
I don't see how it gets to btrfs_lookup_csums_range, there's skip_csum = true set if block_start is EXTENT_MAP_HOLE:

3373         if (test_bit(EXTENT_FLAG_PREALLOC, &em->flags)) {
3374                 skip_csum = true;
3375                 btrfs_set_token_file_extent_type(leaf, fi,
3376                                                  BTRFS_FILE_EXTENT_PREALLOC,
3377                                                  &token);
3378         } else {
3379                 btrfs_set_token_file_extent_type(leaf, fi,
3380                                                  BTRFS_FILE_EXTENT_REG,
3381                                                  &token);
3382                 if (em->block_start == EXTENT_MAP_HOLE)
3383                         skip_csum = true;
3384         }
...
3421         if (skip_csum)
3422                 return 0;

There's no goto that would miss that check, em->block_start is not modified nor is 'em' passed to a function that could modify it.

...
3515         /* block start is already adjusted for the file extent offset. */
3516         ret = btrfs_lookup_csums_range(log->fs_info->csum_root,
3517                                        em->block_start + csum_offset,
3518                                        em->block_start + csum_offset +
3519                                        csum_len - 1, &ordered_sums, 0);
Comment 3 PaX Team 2013-12-12 17:19:51 UTC
David,

it seems that this was indeed a real bug and got only fixed after 3.12 with commit ed9e8af88e2551aaa6bf51d8063a2493e2d71597 in vanilla. It was however never CC'd to -stable...
Comment 4 David Sterba 2013-12-18 13:37:47 UTC
The flow of btrfs patches to stable is not established, usually only the patches that fix serious or visible problems end up there. This bug falls into that category, I'll submit it to stable. Thanks for the report.

Note You need to log in before you can comment on or make changes to this bug.