Thanks for your deep explanation, I get it. Thanks, Yi. On 2018/12/14 11:40, Theodore Y. Ts'o Wrote: > On Thu, Dec 13, 2018 at 03:56:04PM +0800, zhangyi (F) wrote: >> I am checking a CVE patch a45403b515 "ext4: always initialize the crc32c checksum driver"[1] >> in CVE-2018-1094[2] recently, and have a question about the commit log in this patch. >> >> The patch commit log said: >> >>> The extended attribute code now uses the crc32c checksum for hashing >>> purposes, so we should just always always initialize it. We also want >>> to prevent NULL pointer dereferences if one of the metadata checksum >>> features is enabled after the file sytsem is originally mounted. >> >> This first fix is clear. But I don't understand the second fix. IIUC, the kernel does not call >> ext4_set_feature_metadata_csum() to enable metadata checksum, and this feature can only be enabled >> by mkfs,turn2fs or change the image directly. So this feature bit will never change once the >> file system is mounted, the second case could never happen ? > > This was triggered by a maliciously created file system where the > journal contained a superblock which had the metadata checksum feature > enabled (although the superblock which was visible to the kernel when > it was initially mounted did not have the metadata checksum field > set). > > So the file system would get mounted, with metadata_csum not enabled, > so the crc32c checksum was not initialized. Then the journal replay > would overwrite the superblock with a version that had the > metadata_csum feature set. And then the next time the kernel tried to > fetch an inode, it would try to check the inode's metadata checksum, > and dereference a NULL pointer.... and boom. > > This was found by a researcher that was investigating file system > fuzzing techniques. So if you have a system with automount enabled, > this is one more way that someone with access to the USB port could > plug in a maliciously crafted file system, and cause the system to > crash, or at least oops. I don't think *this* particular one could be > exploited to cause a remote execution attack, just a DOS, but it's why > it was assigned a CVE. > >> BTW, does this patch need on the old kernel before dec214d00e "ext4: xattr inode deduplication" ? > > It's needed on any old kernel which supports the metadata checksum > feature. > > Cheers, > > - Ted > > . >
