On Fri, Nov 17, 2017 at 6:18 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> On Fri, Nov 17, 2017 at 01:23:58PM +0200, Amir Goldstein wrote:
>
> [..]
>> > @@ -1164,6 +1187,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>> >
>> > ofs->config.redirect_dir = ovl_redirect_dir_def;
>> > ofs->config.index = ovl_index_def;
>> > + ofs->config.metacopy = ovl_metacopy_def;
>> > err = ovl_parse_opt((char *) data, &ofs->config);
>> > if (err)
>> > goto out_err;
>> > @@ -1209,6 +1233,16 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>> > else if (ofs->upper_mnt->mnt_sb != ofs->same_sb)
>> > ofs->same_sb = NULL;
>> >
>> > + if (!(ovl_force_readonly(ofs)) && ofs->config.metacopy) {
>> > + /* Verify lower root is upper root origin */
>> > + err = ovl_verify_origin(upperpath.dentry,
>> > + oe->lowerstack[0].dentry, false, true);
>> > + if (err) {
>> > + pr_err("overlayfs: failed to verify upper root origin\n");
>> > + goto out_free_oe;
>> > + }
>> > + }
>> > +
>> >
>>
>> Fs can have upper but no workdir and you still need to verify lower
>> If metacopy is enabled
>
> Hmm..., trying to understand this. This probably is more involved.
>
> So first use case is that if metacopy is being enabled, verify lower
> root is upper root origin. (Even if it is read-only fs).
>
> if (ofs->upper_mnt && ofs->config.metacopy)
> ovl_verify_origin().
Yes, this is needed in case workdir/work dir creation fails and fs is mounted
read-only. You should not allow it with metacopy on unverified lower.
>
> But this does not cover the case of same fs being remounted with
> metacopy=off. In that case we will not do metacopy only copy ups but
> existing metacopy inodes will still require that lower does not change.
>
> Will it make sense to set OVL_METACOPY xattr on upper when metacopy=on
> is done first time on mount. And later in subsequent mounts, if METACOPY
> is set on upper, make sure to verify origin and make sure lower supports
> file handles etc.
>
This is an related to what we discussed at length on the ovl-features
patches.
Yes, we SHOULD mark that the metacopy feature was enabled, at least
on first meta_copy_up().
No, we should not do that by marking root upper with metacopy xattr.
this would be very confusing and inconsistent.
We can either set trusted.overlay.features on upper root as Miklos suggested
or incompat_feature/ directory as I suggested, but we need to decide on
a consistent scheme we can use for other features as well.
I guess metacopy must have this infrastructure in place, because as you
correctly noted, we should not mount an overlay with metacopied inodes
without verifying lower and file handle support.
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
