All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miklos Szeredi <miklos@szeredi.hu>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Vivek Goyal <vgoyal@redhat.com>,
	overlayfs <linux-unionfs@vger.kernel.org>,
	Fabian <godi.beat@gmx.net>
Subject: Re: overlayfs: issue with a replaced lower squashfs with export-table
Date: Wed, 8 Jul 2020 10:00:27 +0200	[thread overview]
Message-ID: <CAJfpegv9h7ubuGy_6K4OCdZd3R7Z4HGmCDB2L7mO5bVoGd6MSA@mail.gmail.com> (raw)
In-Reply-To: <CAOQ4uxhd+kYzaDmndCV5rgiswfHnyLjZokmUa+BVk9t31C=HWg@mail.gmail.com>

On Wed, Jul 8, 2020 at 8:55 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Wed, Jul 8, 2020 at 12:53 AM Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > On Tue, Jul 07, 2020 at 08:41:20PM +0300, Amir Goldstein wrote:
> > > > > Miklos,
> > > > >
> > > > > At first glance I did not understand how changing lower file handles causes
> > > > > failure to ovl_verify_inode().
> > > > > To complete the picture, here is the explanation.
> > > > >
> > > > > Upper file A was copied up from lower file with inode 10 in old squashfs
> > > > > and the "origin" file handle composed of the inode number 10 is recorded
> > > > > in upper file A.
> > > > >
> > > > > With newly formatted lower, lower A has inode 11 and lower B has inode 10.
> > > > > Upper file B is copied from lower file B with inode 10 in new squashfs and
> > > > > the "origin" file handle composed of the inode number 10 is recorded
> > > > > in upper file B.
> > > > > Now we have two upper files with the same "origin" that are not hardlinks.
> > > > >
> > > > > On lookup of both overlay files A and B, ovl_check_origin() decodes lower
> > > > > file B (inode 10) as the lower inode.
> > > > > This lower inode is used to get the overlay inode number (10) and as
> > > > > the key to hash overlay inode in inode cache.
> > > > >
> > > > > Suppose A is looked up first and it's inode is hashed.
> > > > > Then B is looked up and in ovl_get_inode() it finds the inode hashed
> > > > > by the same lower inode in inode cache, but fails ovl_verify_inode()
> > > > > because:
> > > > > d_inode(upperdentry) /* B */ != ovl_inode_upper(inode) /* A */
> > > > >
> > > > > This can also happen when copying overlay layers to a new
> > > > > fs tree and carrying over the old "origin" xattr.
> > > > > In practice, the UUID part of the stored "origin" xattr is meant to
> > > > > protect against decoding lower fh when migrating to another
> > > > > filesystem, but layers could be migrated inside the same filesystem.
> > > > > Since squashfs does not have a UUID, re-creating sqhashfs is similar
> > > > > to migrating layers inside the same filesystem.

No, I think copying layers is a different issue: it's not possible to
get the same file handle for A and B since they are

 - either on different filesystems and uuid is different
 - on the same filesystem, hence fh must be unique for the lifetime of the fs

> Now, suggestions for work arounds:
>
> 1. Don't follow with lower null uuid (patch posted) - no caveats

We could add some sort of "overlay version"  to origin to be able to
trust null uuid, but only if created in *this* instance of overlayfs.
That way we retain some of the advantages without any of the
drawbacks.   This does not seem too complex theoretically, but not
sure if it's worth it.

> 2. Opt-out of following origin with explicit option e.g. "index=nofollow"
> 3. Don't follow origin unless one of the following opt-in features:
>     metacopy,index,xino

Maybe, if 1) is problematic for some reason.

Thanks,
Miklos

  reply	other threads:[~2020-07-08  8:00 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-06 13:27 overlayfs: issue with a replaced lower squashfs with export-table Fabian
2020-07-06 14:29 ` Amir Goldstein
2020-07-06 15:14   ` Fabian
2020-07-06 15:33     ` Amir Goldstein
2020-07-06 16:10       ` Fabian
2020-07-06 17:14         ` Amir Goldstein
2020-07-16 13:29           ` Fabian Godehardt
2020-07-07  5:51     ` Amir Goldstein
2020-07-07 15:51       ` Vivek Goyal
2020-07-07 17:41         ` Amir Goldstein
2020-07-07 21:53           ` Vivek Goyal
2020-07-08  6:55             ` Amir Goldstein
2020-07-08  8:00               ` Miklos Szeredi [this message]
2020-07-08  8:31                 ` Amir Goldstein
2020-07-08  8:37                   ` Miklos Szeredi
2020-07-08  8:50                     ` Amir Goldstein
2020-07-08 14:23                       ` Vivek Goyal
2020-07-08 14:26                         ` Vivek Goyal
2020-07-08 17:26                           ` Amir Goldstein
2020-07-08 17:36                             ` Vivek Goyal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJfpegv9h7ubuGy_6K4OCdZd3R7Z4HGmCDB2L7mO5bVoGd6MSA@mail.gmail.com \
    --to=miklos@szeredi.hu \
    --cc=amir73il@gmail.com \
    --cc=godi.beat@gmx.net \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=vgoyal@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.