On 2018/2/25 2:46, Amir Goldstein wrote:
> On Sat, Feb 24, 2018 at 11:36 AM, yangerkun <yangerkun@xxxxxxxxxx> wrote:
>>
>>
>> On 2/24/2018 3:43 PM, Amir Goldstein wrote:
>>>
>>> On Fri, Feb 23, 2018 at 2:01 PM, yangerkun <yangerkun@xxxxxxxxxx> wrote:
>>>>
>>>> For origin xattr, use fid in ovl_fh to consume file_handle, and use
>>>> open_by_handle_at to check validity of the origin ovl_fh. In auto mode,
>>>> the invalid origin xattr will remove automatic.
>>>>
>>>> Signed-off-by: yangerkun <yangerkun@xxxxxxxxxx>
>>>> ---
> [...]
>>>> +
>>>> +/*
>>>> + * Check the validity of ovl_fh.
>>>> + */
>>>> +int ovl_check_fh_len(struct ovl_fh *fh, int fh_len)
>>>> +{
>>>> + if (fh_len < sizeof(struct ovl_fh) || fh_len < fh->len)
>>>> + return -EINVAL;
>>>> +
>>>> + if (fh->magic != OVL_FH_MAGIC)
>>>> + return -EINVAL;
>>>> +
>>>> + if (fh->version > OVL_FH_VERSION || fh->flags & ~OVL_FH_FLAG_ALL)
>>>> + return -ENODATA;
>>>> +
>>>> + if (!(fh->flags & OVL_FH_FLAG_ANY_ENDIAN) &&
>>>> + (fh->flags & OVL_FH_FLAG_BIG_ENDIAN) !=
>>>> OVL_FH_FLAG_CPU_ENDIAN)
>>>> + return -ENODATA;
>>>> +
>>>
>>>
>>> Overlayfs approach to unrecognized ovl_fh on-disk format is very
>>> naiive borderline buggy.
>>> When endian/flags/version check fails, origin is treated as "unknown"
>>> and that is fine as
>>> long as the only implication is change of inode number.
>>> But when checking that origin of upper root matches lower root for
>>> index feature, treating
>>> unrecognized ovl_fh format as unknown will result in overwriting the
>>> root origin and as
>>> far as I can tell, this is a bug.
>>>
>>> The situation with fsck.overlayfs is even worse IMO, because running
>>> an old fsck tools
>>> is even more easy to get wrong than mounting an overlayfs with an old
>>> kernel.
>>> In general fsck.* tools, like e2fsck, are more prudent about fixing a
>>> filesystem
>>> with unknown on-disk format than the kernel about mounting filesystem
>>> with unknown
>>> on-disk format features.
>>>
>>> This patch specifically does not seem to do any harm when encountering
>>> unknown
>>> on-disk format of ovl_fh, but I think fsck.overlay should take some
>>> extra steps for
>>> safety, before fixing overlayfs with features that fsck.overlay does
>>> not support.
>>>
>>> I suggest at least the following safely checks on start of fsck.overlay:
>>> - Check if index dir exists in workdir
>>> - Check if upperdir root has an unrecognized origin xattr format
>>> - Check if index dir root has an unrecognized origin xattr format
>>>
>>> If any of the checks above is positive, fsck.overlay should issue a
>>> warning
>>> and refuse to fix fielsystem. Running with -n -f is allowed.
>>> See if index dir exists, then removing origin xattr can corrupt the index.
>>> When fsck.overlay gains the knowledge about checking and fixing index,
>>> user would opt-in to checking an overlayfs with index by -o index=on
>>
>>
>> When compile the kernel, we can choose the default config about overlay,
>> which user won't get. So, user may choose the fault arguments like
>> "fsck.overlay -olower..." while index xattr is on when there is really
>> overlayfs. Other filesystems can get xattr from superblock in disk, but we
>> cannot use this way in overlay. So, how to deal with this？
>>
>>
>
> The kernel default options for overlayfs mount really does not matter for
> fsck.overlay. The only thing that matters is to detect whether the layers
> were *ever* mounted by a kernel which enabled features that fsck.overlay
> does not support.
>
> The current state of fsck.overlay is:
> - supports redirect feature
> - does not support any other feature including upstream features index and
> nfs_export and future features like metacopy.
>
> A filesystem repair tool should NOT fix the filesystem if it detects on-disk
> format that it does not recognize.
>
> That means among other things for overlayfs that if an xattr if found on any
> file by the name of trusted.overlay.<something> and <something> is not a
> an expected name, then fsck.overlay should shout that it does not
> recognize the on-disk format (print out the unrecognized xattr name) and
> refuse to continue.
>
> The problem with overlayfs compares to traditional filesystems is the lack
> of "feature bits" in the "super block", so there is no O(1) test for unsupported
> features, but they still can be detected.
>
> Contrary to the example above, there is a simple O(1) test to check if
> overlayfs was mounted with index feature enabled - the existence of a
> non empty $workdir/index dir is de-facto a "feature bit" and until fsck.overlay
> knows how to verify and fix overlayfs without corrupting the index it should
> refuse to fix an overlay filesystem with an index.
>
Maybe we can store "feature bit" in upper root xattr or somewhere else in workdir
after we implement mkfs.overlay, and classify them into compat and incompat
features, we can use old fsck.overlay to fix the filesystem if there is no
unrecognized incompat feature.
Before this, the index dir can be treated as a kind of "unrecognized incompat
feature", so the "simple O(1) test" can works well, but how can we handle
another incompat feature tomorrow? Should we implement mkfs.overlay before the
first release version of fsck.overlay?
Thanks,
Yi.
> The options that will be passed to fsck.overlay (i.e.
> redirect_dir=off, index-on)
> will not mean that fsck.overlay should or should not check redirects/index.
> They will mean something like:
> index=on - reconstruct index for copied up hardlinks
> redirect_dir=off - remove all redirects and recursively "move" all
> copied up files.
>
> Thanks,
> Amir.
>
> .
>
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
