[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <301ccbea-4140-3816-a1b3-5018ffb4036c@gmx.com>
Date: Tue, 20 Aug 2019 14:04:46 +0800
From: Qu Wenruo <quwenruo.btrfs@....com>
To: miaoxie@...wei.com, Gao Xiang <hsiangkao@....com>,
"Darrick J. Wong" <darrick.wong@...cle.com>
Cc: Christoph Hellwig <hch@...radead.org>,
"Theodore Y. Ts'o" <tytso@....edu>,
Eric Biggers <ebiggers@...nel.org>,
Richard Weinberger <richard@....at>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jan Kara <jack@...e.cz>, Chao Yu <yuchao0@...wei.com>,
Dave Chinner <david@...morbit.com>,
David Sterba <dsterba@...e.cz>,
devel <devel@...verdev.osuosl.org>,
Stephen Rothwell <sfr@...b.auug.org.au>,
Amir Goldstein <amir73il@...il.com>,
linux-erofs <linux-erofs@...ts.ozlabs.org>,
Al Viro <viro@...iv.linux.org.uk>,
Jaegeuk Kim <jaegeuk@...nel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
Li Guifu <bluce.liguifu@...wei.com>,
Fang Wei <fangwei1@...wei.com>, Pavel Machek <pavel@...x.de>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] erofs: move erofs out of staging
[...]
>> The same tool exists for btrfs, although lacks the write ability, but
>> that dump is more comprehensive and a great tool to learn the on-disk
>> format.
>>
>>
>> And for the fuzzing defending part, just a few kernel releases ago,
>> there is none for btrfs, and now we have a full static verification
>> layer to cover (almost) all on-disk data at read and write time.
>> (Along with enhanced runtime check)
>>
>> We have covered from vague values inside tree blocks and invalid/missing
>> cross-ref find at runtime.
>>
>> Currently the two layered check works pretty fine (well, sometimes too
>> good to detect older, improper behaved kernel).
>> - Tree blocks with vague data just get rejected by verification layer
>> So that all members should fit on-disk format, from alignment to
>> generation to inode mode.
>>
>> The error will trigger a good enough (TM) error message for developer
>> to read, and if we have other copies, we retry other copies just as
>> we hit a bad copy.
>>
>> - At runtime, we have much less to check
>> Only cross-ref related things can be wrong now. since everything
>> inside a single tree block has already be checked.
>>
>> In fact, from my respect of view, such read time check should be there
>> from the very beginning.
>> It acts kinda of a on-disk format spec. (In fact, by implementing the
>> verification layer itself, it already exposes a lot of btrfs design
>> trade-offs)
>>
>> Even for a fs as complex (buggy) as btrfs, we only take 1K lines to
>> implement the verification layer.
>> So I'd like to see every new mainlined fs to have such ability.
>
> It is a good idea. In fact, we already have a verification layer which was implemented
> as a device mapper sub-module. I think it is enough for a read-only filesystem because
> it is simple, flexible and independent(we can modify the filesystem layout without
> verification module modification).
If you're talking about dm-verity, then IMHO they are with completely
different objective.
For dm-verity it's more like authentication. Without proper key
(authentication), no one can modify the data without being caught.
That's why I hate such thing, it's not open at all, *as bad as locked
bootloader*.
While the tree-checker (the layer in btrfs) is more like a sensitive and
sometimes overreacting detector, find anything wrong, then reject the
offending tree block.
The original objective of tree-checker is to free coder from defensive
coding, providing a centralized verification service, thus we don't need
to verify tree blocks randomly using ugly BUG_ON()s.
(But unfortunately, a lot of BUG_ON()s are still kept as is, as it takes
more time to persuade reviewers that those BUG_ON()s are impossible to
hit anymore)
Tree-checker can not only detect suspicious metadata (either caused by
mem bit flip or poorly crafted image), but also bad *kernel* behavior or
even runtime bitflip. (Well, it only works for RW fs, so not really
helpful for a RO fs).
And performance is another point.
That tree-checker in btrfs is as fast/slow as CRC32.
Not sure how it would be for dm-verity, but I guess it's slower than
CRC32 if using any strong hash.
Anyway, for a RO fs, if it's relying on dm-verify then that's OK for
real-world usage.
But as a standalone fs, even it's RO, a verification layer would be a
great plus.
At least when new student developers try fuzzed images on the fs, it
would be a good surprise other than tons of new bug reports.
>
>
>>>
[...]
>>>
>>> Yes, we will do such a debugging tool of course. Actually Li Guifu is now
>>> developping a erofs-fuse to support old linux versions or other OSes for
>>> archiveing only use, we will base on that code to develop a better fuzzer
>>> tool as well.
>>
>> Personally speaking, debugging tool is way more important than a running
>> kernel module/fuse.
>> It's human trying to write the code, most of time is spent educating
>> code readers, thus debugging tool is way more important than dead cold code.
>
> Agree, Xiang and I have no time to developing this feature now, we are glad very much if you could help
> us to do it ;)
In fact, since the fs is a RO fs, it could be pretty good educational
example for any fs newbies. Thus a debug tool which can show the full
metadata of the fs can really be helpful.
In fact, btrfs-debug-tree (now "btrfs ins dump-tree") leads my way to
btrfs, and still one of my favourite tool to debug.
Thanks,
Qu
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists