Re: [PATCH] erofs: move erofs out of staging

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4557 bytes --]

[...]
>> 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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]