Bug 45631 - Caught 32-bit read from uninitialized memory in ext4_da_get_block_prep
Caught 32-bit read from uninitialized memory in ext4_da_get_block_prep
Status: RESOLVED CODE_FIX
Product: File System
Classification: Unclassified
Component: ext4
All Linux
: P1 normal
Assigned To: fs_ext4@kernel-bugs.osdl.org
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-05 21:35 UTC by Christian Casteyde
Modified: 2012-08-26 18:55 UTC (History)
2 users (show)

See Also:
Kernel Version: 3.6-rc1
Tree: Mainline
Regression: Yes


Attachments
Kernel config file used. (75.58 KB, text/plain)
2012-08-05 21:35 UTC, Christian Casteyde
Details

Description Christian Casteyde 2012-08-05 21:35:23 UTC
Created attachment 76841 [details]
Kernel config file used.

Kernel 3.6-rc1
Slackware 64 current (gcc 4.7.0)
Core i7, 6GB

Since 3.6-rc1, I get the following warning when booting with debug options (check memory use):
WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (ffff8801b6a6d950)
ffffffff000000000000000000000000002c0400000000001c00000000000000
 i i i i i i i i i i i i i i i i u u u u i i i i i i u u u u u u
                                 ^
Pid: 7839, comm: kdm Not tainted 3.6.0-rc1 #1 Acer Aspire 7750G/JE70_HR
RIP: 0010:[<ffffffff811b3093>]  [<ffffffff811b3093>] ext4_da_get_block_prep+0x243/0x2b0
RSP: 0018:ffff8801c29eb958  EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff8801b6a6d4d0 RCX: 0000000000000000
RDX: ffffffff811b3085 RSI: ffff8801b6a6d9e8 RDI: 0000000000000001
RBP: ffff8801c29eb9d8 R08: ffffffff820f4660 R09: ffffffff820c5ce0
R10: 0000000000000001 R11: 0000000000000000 R12: ffff8801a9606d00
R13: ffff8801c1109800 R14: 0000000000000000 R15: 0000000000000000
FS:  00007fd6e5908740(0000) GS:ffff8801c7e00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: ffff8801c6be0dd0 CR3: 00000001c29da000 CR4: 00000000000407f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff4ff0 DR7: 0000000000000400
 [<ffffffff8116efc1>] __block_write_begin+0x1b1/0x540
 [<ffffffff811b62f1>] ext4_da_write_begin+0xc1/0x1a0
 [<ffffffff810f4c09>] generic_file_buffered_write+0x119/0x2a0
 [<ffffffff810f6c3c>] __generic_file_aio_write+0x1bc/0x3c0
 [<ffffffff810f6ec5>] generic_file_aio_write+0x85/0x110
 [<ffffffff811afb24>] ext4_file_write+0xa4/0x4b0
 [<ffffffff8113d2a7>] do_sync_write+0xa7/0xe0
 [<ffffffff8113daf6>] vfs_write+0xa6/0x160
 [<ffffffff8113ddf8>] sys_write+0x48/0xa0
 [<ffffffff817e1aa2>] system_call_fastpath+0x16/0x1b
 [<ffffffffffffffff>] 0xffffffffffffffff

In gdb, I get the following:
(gdb) l *0xffffffff811b3093
0xffffffff811b3093 is in ext4_da_get_block_prep (fs/ext4/inode.c:1217).
1212            /*
1213             * ext4_calc_metadata_amount() has side effects, which we have
1214             * to be prepared undo if we fail to claim space.
1215             */
1216            save_len = ei->i_da_metadata_calc_len;
1217            save_last_lblock = ei->i_da_metadata_calc_last_lblock;
1218            md_needed = EXT4_NUM_B2C(sbi,
1219                                     ext4_calc_metadata_amount(inode, lblock));
1220            trace_ext4_da_reserve_space(inode, md_needed);
1221

This is new to 3.6-rc1, hence a regression.
Comment 1 Theodore Tso 2012-08-05 22:21:04 UTC
It's a false positive in that this isn't going to actually cause an actual bug; we're just saving and restoring an uninitialized value if we have a error.  The fact that we do this is no big deal, since if i_da_metadata_calc_len is zero, the value of i_data_metadata_calc_last_lblock is never consulted.  So if we save an uninitialized value, and then later restore it, it really doesn't matter from a correctness point of view.

That being said, triggering a kmemcheck failure is sad, even if it's not a real bug, so I agree we should fix it.  The most expedient and most efficient fix is to simply initialize i_data_metadata_calc_last_lblock to zero in ext4_alloc_inode(); this is where we initialize i_data_metadata_calc_len to zero, and if we initialize the last_lblock to zero, it won't matter, and it saves us from having to adding an extra conditional in ext4_da_get_block_prep().

- Ted
Comment 2 Florian Mickler 2012-08-26 10:48:54 UTC
A patch referencing this bug report has been merged in Linux v3.6-rc3:

commit 7e731bc9a12339f344cddf82166b82633d99dd86
Author: Theodore Ts'o <tytso@mit.edu>
Date:   Sun Aug 5 23:28:16 2012 -0400

    ext4: avoid kmemcheck complaint from reading uninitialized memory
Comment 3 Theodore Tso 2012-08-26 18:55:58 UTC
Fixed with commit 7e731bc9a123

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