Bug 199183
| Summary: | Invalid pointer dereference in ext4_xattr_inode_hash when mounting and later operating on a crafted image | ||
|---|---|---|---|
| Product: | File System | Reporter: | Wen Xu (wen.xu) |
| Component: | ext4 | Assignee: | fs_ext4 (fs_ext4) |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | normal | CC: | tytso, vt, wen.xu |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 4.15.x | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
| Attachments: |
The crafted image which causes kernel panic
poc.c 0001-ext4-always-initialize-the-crc32c-checksum-driver.patch 0002-ext4-don-t-allow-r-w-mounts-if-metadata-blocks-overl.patch |
||
|
Description
Wen Xu
2018-03-22 20:46:22 UTC
Created attachment 274885 [details]
poc.c
I can't replicate the BUG with the latest version of linux. What makes you think inode->i_sb is NULL? That field is one of the first to be set up.... [ Sorry, it was my mistake. After debugging the kernel again, I found that it crashes here: https://elixir.bootlin.com/linux/v4.15/source/fs/ext4/ext4.h#L2005 static inline u32 ext4_chksum(struct ext4_sb_info *sbi, u32 crc, const void *address, unsigned int length) { struct { struct shash_desc shash; char ctx[4]; } desc; int err; BUG_ON(crypto_shash_descsize(sbi->s_chksum_driver)!=sizeof(desc.ctx)); <- sbi->s_chksum_driver may not always be set up and can be NULL, while static inline unsigned int crypto_shash_descsize(struct crypto_shash *tfm) { return tfm->descsize; } the function crypto_shash_descsize() directly dereferences the pointer. By the way, I reproduce it on Linux 4.15.0-12-generic used by latest Ubuntu 18.04. OK, I was able to replicate this on 4.15, but it's not replicating on 4.16-rc1. However, the underlying issue was NOT that inode->i_sb is NULL, but rather that s_chksum_driver (found in EXT4_SBI(inode->i_sb)->s_chksum_driver) was NULL. I'm not sure why we're not ending up calling ext4_chksum() in 4.16-rc1, but the xattr code can try to calculate a crc32c, and so we should just initialize the s_chksum_driver() unconditionally. It will make things simpler, and avoid a bunch of problems. In addition, the file system should have never been allowed to have been mounted in the first place, since one of the block allocation bitmaps overlapped with the superblock, and this means that as blocks got allocated, the superblock was getting corrupted. Created attachment 274935 [details]
0001-ext4-always-initialize-the-crc32c-checksum-driver.patch
Created attachment 274937 [details]
0002-ext4-don-t-allow-r-w-mounts-if-metadata-blocks-overl.patch
Theodore, your patch 0001-ext4-always-initialize-the-crc32c-checksum-driver.patch (which is v4.16-rc1-18-ga45403b51582) introduces strong dependency on having crc32c modules preloaded before ext4, otherwise it's goto failed_mount. This dependency is invisible for modinfo/depmod, thus some dependency resolvers for initrd fail to properly adjust crc32c modules for boot. Is it intentional or could be fixed? Thanks, |