From da64ae3a0f7498e356a72e7f1b65a704ddfbb6bc Mon Sep 17 00:00:00 2001 From: David Sterba Date: Mon, 12 Sep 2016 11:13:24 +0200 Subject: [PATCH] btrfs-progs: reorganize extent_buffer and fix alignment of data Reported by UBSAN, the checksum code tries to access unaligned data that come from the extent_buffer. struct extent_buffer { struct cache_extent cache_node; /* 0 48 */ u64 start; /* 48 8 */ u64 dev_bytenr; /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ u32 len; /* 64 4 */ /* XXX 4 bytes hole, try to pack */ struct extent_io_tree * tree; /* 72 8 */ struct list_head lru; /* 80 16 */ struct list_head recow; /* 96 16 */ int refs; /* 112 4 */ u32 flags; /* 116 4 */ int fd; /* 120 4 */ char data[0]; /* 124 0 */ /* size: 128, cachelines: 2, members: 11 */ /* sum members: 120, holes: 1, sum holes: 4 */ /* padding: 4 */ }; Add explicit alignment to data. Reported-by: Lukas Lueg Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=156471 Signed-off-by: David Sterba --- extent_io.h | 2 +- ...471-ubsan-trigger-crc32c-unaligned.raw.txt | 62 ++++++++++++++++++ ...6471-ubsan-trigger-crc32c-unaligned.raw.xz | Bin 0 -> 3764 bytes 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 tests/fuzz-tests/images/bko-156471-ubsan-trigger-crc32c-unaligned.raw.txt create mode 100644 tests/fuzz-tests/images/bko-156471-ubsan-trigger-crc32c-unaligned.raw.xz diff --git a/extent_io.h b/extent_io.h index d9594c32..208c4fea 100644 --- a/extent_io.h +++ b/extent_io.h @@ -97,7 +97,7 @@ struct extent_buffer { int refs; u32 flags; int fd; - char data[]; + char data[] __attribute__((aligned(8))); }; static inline void extent_buffer_get(struct extent_buffer *eb) diff --git a/tests/fuzz-tests/images/bko-156471-ubsan-trigger-crc32c-unaligned.raw.txt b/tests/fuzz-tests/images/bko-156471-ubsan-trigger-crc32c-unaligned.raw.txt new file mode 100644 index 00000000..c8279633 --- /dev/null +++ b/tests/fuzz-tests/images/bko-156471-ubsan-trigger-crc32c-unaligned.raw.txt @@ -0,0 +1,62 @@ +URL: https://bugzilla.kernel.org/show_bug.cgi?id=156471 +Lukas Lueg 2016-09-09 18:58:27 UTC + +More news from the fuzzer and (up to now) the only news from UBSAN using +btrfs-progs v4.7-42-g56e9586. The attached image causes btrfsck to trigger +undefined behavior by dereferencing a ptr to a long unsigned int that was cast +from an uchar with no alignment guarantees. + +UBSAN complains: +crc32c.c:75:19: runtime error: load of misaligned address 0x000001b3736c for +type 'long unsigned int', which requires 8 byte alignment + +I've attached an image and a log, the behavior is triggered all the time and +unspecific, though. + +AFAIC the problem is that *ptmp is cast from *data. This may actually not cause +the CPU to fault due to how *data is de-facto aligned by it's callers. The code +may still cause nose demons as the pure act of having *ptmp is undefined +behavior. + +crc32c.c:75:19: runtime error: load of misaligned address 0x000001b3736c for type 'long unsigned int', which requires 8 byte alignment +0x000001b3736c: note: pointer points here + 00 00 00 00 b7 0e 65 6c 64 61 40 4b a5 0d 0f ba 33 0c 75 27 00 00 02 00 00 00 00 00 01 00 00 00 + ^ + #0 0x4f4308 in crc32c_intel /home/lukas/dev/btrfsfuzz/src-ubsan/crc32c.c:75 + #1 0x4f43f3 in crc32c_le /home/lukas/dev/btrfsfuzz/src-ubsan/crc32c.c:221 + #2 0x486c39 in __csum_tree_block_size /home/lukas/dev/btrfsfuzz/src-ubsan/disk-io.c:139 + #3 0x486c39 in csum_tree_block_size /home/lukas/dev/btrfsfuzz/src-ubsan/disk-io.c:159 + #4 0x486d48 in csum_tree_block_fs_info /home/lukas/dev/btrfsfuzz/src-ubsan/disk-io.c:174 + #5 0x48ba29 in read_tree_block_fs_info /home/lukas/dev/btrfsfuzz/src-ubsan/disk-io.c:348 + #6 0x48d48d in read_tree_block /home/lukas/dev/btrfsfuzz/src-ubsan/disk-io.h:112 + #7 0x48d48d in btrfs_setup_chunk_tree_and_device_map /home/lukas/dev/btrfsfuzz/src-ubsan/disk-io.c:1210 + #8 0x48d95b in __open_ctree_fd /home/lukas/dev/btrfsfuzz/src-ubsan/disk-io.c:1322 + #9 0x48dd80 in open_ctree_fs_info /home/lukas/dev/btrfsfuzz/src-ubsan/disk-io.c:1381 + #10 0x45011a in cmd_check /home/lukas/dev/btrfsfuzz/src-ubsan/cmds-check.c:11449 + #11 0x40a799 in main /home/lukas/dev/btrfsfuzz/src-ubsan/btrfs.c:243 + #12 0x7fdf11c96730 in __libc_start_main (/lib64/libc.so.6+0x20730) + #13 0x40a1e8 in _start (/home/lukas/dev/btrfsfuzz/bin-ubsan/bin/btrfs+0x40a1e8) + +key (3472328296227680304 INODE_EXTREF 3472328296227680304)slot end outside of leaf 12360 > 3995 +parent transid verify failed on 4227072 wanted 3472328296227680304 found 4 +Ignoring transid failure +checking extents +Checking filesystem on ubsan_logs/id:000990,src:000816,op:flip1,pos:3845.img +UUID: b70e656c-6461-404b-a50d-0fba330c7527 +key (3472328296227680304 INODE_EXTREF 3472328296227680304)slot end outside of leaf 12360 > 3995 +Invalid key type(ROOT_ITEM) found in root(3472328296227680304) +ignoring invalid key +Invalid key type(ROOT_ITEM) found in root(3472328296227680304) +ignoring invalid key +Invalid key type(ROOT_ITEM) found in root(3472328296227680304) +ignoring invalid key +Invalid key type(ROOT_ITEM) found in root(3472328296227680304) +ignoring invalid key +key (3472328296227680304 INODE_EXTREF 3472328296227680304)slot end outside of leaf 12360 > 3995 +key (3472328296227680304 INODE_EXTREF 3472328296227680304)slot end outside of leaf 12360 > 3995 +key (3472328296227680304 INODE_EXTREF 3472328296227680304)slot end outside of leaf 12360 > 3995 +key (3472328296227680304 INODE_EXTREF 3472328296227680304)slot end outside of leaf 12360 > 3995 +key (3472328296227680304 INODE_EXTREF 3472328296227680304)slot end outside of leaf 12360 > 3995 +bad block 4202496 +Errors found in extent allocation tree or chunk allocation +key (3472328296227680304 INODE_EXTREF 3472328296227680304)slot end outside of leaf 12360 > 3995 diff --git a/tests/fuzz-tests/images/bko-156471-ubsan-trigger-crc32c-unaligned.raw.xz b/tests/fuzz-tests/images/bko-156471-ubsan-trigger-crc32c-unaligned.raw.xz new file mode 100644 index 0000000000000000000000000000000000000000..ee5778a56e70114578c170a1ee94173c23ec73f7 GIT binary patch literal 3764 zcmeH~X*3&%7RM87T1%QyMeU^{)E-+B1El}}z_B6s0S+gOyTua#2s_|#%4i%b6QpPa{IXM14)k4FGXEZXNhV=LqubCJ zXj2#0OC;K9lnG7UtXxc2fS?yJ;@vpn+vwdVKi(7!V$MZb`*={zMAtq~z)p6i3+syV zQs4mvG0D5PEfwOHz zUNvjV3Y*bPPw_`dfv&Did}hNI=)G>dU1FNn{7_L@^(YmAs>eH=j-3Im!hZ0EqfMV7 z34wkSTjJKpNbUztPZGdUG**bFen1^po{a0hC8UR4;2&7xn>wu-FjX6R!cca2!TR)J z?YV(8f8WfbQnH&SJ(J}6WHrqkK0~Zh5a{|bkZ@kw>e#xvpY{{R2CPxkV>D8fXS-04 z8ywq!-TRbBtF5$Cl;M$xDEXKp*!9Zx{3E=nyus|{o#)*iqIZ6&eNo)|@!ciD;V_vg zle55AX8}`ut~Hjh<*@`OOKq_Uom#Wv2Aj*e{42FP-mhBWiHAg z_GZy%ca?pP7!mqaLgpKdHoNHRm#V=gHhzdK5n;};+}*1&m|cNEP9D?}TpqS0f(;kU z4n9q9POFl5!>?v8lzu^(`&xrmKZbP&bm2NBcy@$~Ew75?ML0hkB>%iTP;&B9PC;_t zw7Ge7R&jiDzW7WfS>?E99C2kzUzL%so!mdvWQ+s7iZ1ncAu$|jvyD_ijr%&ccMFQ9 zS3@HDM%rHv?1^m9*UY>Ggf2$on(adKf8S9qv7W@5&JABo(WfN^DZYVmZB)0do%6jpP069ZOu#ve| zlnlhR^F&4;@}gB@ALCxZ&ZuaAXTMHskak_%Rm-a&1>hACHysBVD&rwu>m-#%)(Q&B@<*abu1eh!#R4JjAJSZU7qN-Dl!xy8Ni=#;P7W%ck4<9XpS_l=C$~rWzD|fYv3f7c3!BKY-A;fK%2|rC`rgyZ9 zWLsYmoL@Yrv5qT2AB-0R0d8EBeP!uJ-!KC2&Jebeb-hKu*yWJd;!=^F7kp;16qz=c z`@BgpYDYnsusmu0L6_~*ByAvFJiho?p#F^JV%qJ+KRl8#D%qIFdVxRCfD|%ZRaJ!qf4lmdC$44P;=5th1aA7esuLp#nH=jM8DNw)3Q!xF2d#Zyr3{u zC~^_tzm#5*+8(U-oC9H`l%Ok|)k)glj%qacMF!M_JCCEz##OF*3=cWnw(n*npAiZV zx7K6-v}pM(Twm(cAwqzCE9hyg&36%mMurj*R?1kC{r_Ha4&_m>&b6OIHo=xQo#&#C;L9#hP}rS<5|ZySi(_#oN{)Ii`yyqdfUo@ zw)9{Iik0xYD*nT_`>$^NKa|a1+8WFDgTj8btuui2NOaY~f6*cTCTyL+ruAQe{Z=aZ zH#+zV?Dt~aS73iK_kIQT*8n5%nE`kK<;zJMX8(LY0fv4!Q2M|S6#yhl69@#5%2`up PU6=nCu3w8h80