Re: [PATCH v4] erofs: accept source file descriptor via fsconfig

[Gao Xiang <hsiangkao@linux.alibaba.com>]

Hi Christian,

On 2026/7/23 22:46, Christian Brauner wrote:
>> I'm not quite sure if I catched the point, I think Giuseppe's patch here
>> tried to record `file` into `sbi->dif0.file` (which indicates the primary
>> "device" later.)
>>
>> And if `sbi->dif0.file` is set up by erofs_fc_parse_source(),
>> erofs_fc_get_tree() will just use `sbi->dif0.file` instead of
>> `fc->source` according to this patch.
> 
> Oh, so you only do it for file-backed mounts. Do you only allow regular
> files or do you also support block devices with
> CONFIG_EROFS_FS_BACKED_BY_FILE?

Block devices with CONFIG_EROFS_FS_BACKED_BY_FILE are supported,
but with only `fc->source` (not this way.)

That is the limitation I see in Giuseppe's patch. I'd hoped
bdev-backed mounts could work the same way, but that would require
changes to the VFS flow.

Since this is a side improvement, I think it's fine as long as
it's documented somewhere, and I do hope Giuseppe can at least
address the documentation.

I'd also like to make sure the way fc->source is filled
out of fd passing follows common practice, so that if fd-based
bdev-backed mounts land in the VFS later, they can keep
the same fc->source convention, otherwise it will cause
a userspace behavior change.

> 
> Do you document the expected behavior for the file you're consuming?
> Meaning, are concurrent modifications supported and what type of
> behavior does this exhibit?


As I perhaps mentioned, EROFS itself (or many EROFS) won't do any
modification to the underlayfs bdev or files by design so the
standard behavior is the blob devices / files won't get any change.
Beyond that, both the on-disk format and the implementation are
designed to tolerate unexpected external modifications (or storage
media damage). Even in the worst case, where the underlying storage
(block device or backing filesystem) is malicious, corrupted on-disk
(meta)data will not lead to the kind of complex, hard-to-resolve
inconsistencies you see in general-purpose writable filesystems,
whose ondisk/in-memory cached metadata is much harder to reconcile.

I'm not sure whether you'll agree, but I want to emphasize that
again this is one of EROFS core design goals: the on-disk and
implementation design ensure that. If there is any human bug, it
will be addressed and fixed as long as it discloses: it won't be
hard to fixed.

But if you really want to avoid concurrent modifications or keep
the image golden, I think dmverity or fsverify should be enforced
to ensure the filesystem won't be modified unexpectedly or expectedly.

> 
>> The reason why `fc->source` is set was discussed in the
>> thread of the previous version suggested by Aleksa.
>>
>>>
>>> If they close it before this means you can mount something completely
>>> different. The other thing is even if they keep the fd open someone
>>> could just rename the damn thing and fc->source ends up pointing
>>> somwhere completely different. The could switch namespaces as well in
>>> some circumstances and then it points again into wherever.
>>>
>>> I've played with that fd idea before. The only way to make this work
>>> correctly is if you plumb this down into get_tree_nodev()
>>
>> fc->source in this case has no use in erofs_fc_get_tree() (`fc->source`
>> is just used for mountinfo for example), `sbi->dif0.file` works instead
>> I hope I don't misunderstand something.
> 
> No, I misunderstood this.

Thanks,
Gao Xiang