Bug 156471
| Summary: | Undefined behavior in crc32c.c | ||
|---|---|---|---|
| Product: | File System | Reporter: | Lukas Lueg (lukas.lueg) |
| Component: | btrfs | Assignee: | Josef Bacik (josef) |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | normal | CC: | dsterba |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 4.6.6-300.fc24-x86_64 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
| Attachments: |
Image causing undefined behavior in crc32c.c
UBSAN-enabled log |
||
Created attachment 232941 [details]
UBSAN-enabled log
The crc32 code is copied from kernel and I don't see any difference regarding alignment handling (btrfs-progs: crc32c_intel, arch/x86/crypto/crc32c-intel-glue.c: crc32c_intel_le_hw).
The buffer data come from extent_buffer::data, which is not specifically aligned:
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 */
};
I'll make the alignment explicit and remove the 4B gap after len as well.
Patch in devel, image added to the testsuite. |
Created attachment 232931 [details] Image causing undefined behavior in crc32c.c 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.