From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: "zhangyi (F)" <yi.zhang@huawei.com>
Cc: linux-ext4@vger.kernel.org, Miao Xie <miaoxie@huawei.com>,
yangerkun <yangerkun@huawei.com>
Subject: Re: Question about commit "ext4: always initialize the crc32c checksum driver"
Date: Thu, 13 Dec 2018 22:40:52 -0500 [thread overview]
Message-ID: <20181214034052.GC20880@thunk.org> (raw)
In-Reply-To: <d686d167-9ad2-bfd6-3464-fb35c10ae63d@huawei.com>
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
next prev parent reply other threads:[~2018-12-14 3:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-13 7:56 Question about commit "ext4: always initialize the crc32c checksum driver" zhangyi (F)
2018-12-14 3:40 ` Theodore Y. Ts'o [this message]
2018-12-14 7:51 ` zhangyi (F)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181214034052.GC20880@thunk.org \
--to=tytso@mit.edu \
--cc=linux-ext4@vger.kernel.org \
--cc=miaoxie@huawei.com \
--cc=yangerkun@huawei.com \
--cc=yi.zhang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).