[GIT PULL] multi-layer support for overlay filesystem

[GIT PULL] multi-layer support for overlay filesystem

[Miklos Szeredi]

Al,

Please pull from

  git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next

This adds support for multiple read-only layers to overlayfs.  It also makes the
writable upper layer optional.

This is a highly requested feature and has been in -next for the last cycle.

Thanks,
Miklos

---
Miklos Szeredi (16):
      ovl: check whiteout while reading directory
      ovl: make path-type a bitmap
      ovl: dont replace opaque dir
      ovl: add mutli-layer infrastructure
      ovl: helper to iterate layers
      ovl: multi-layer readdir
      ovl: multi-layer lookup
      ovl: check whiteout on lowest layer as well
      ovl: lookup ENAMETOOLONG on lower means ENOENT
      ovl: allow statfs if no upper layer
      ovl: mount: change order of initialization
      ovl: improve mount helpers
      ovl: make upperdir optional
      ovl: support multiple lower layers
      ovl: add testsuite to docs
      ovl: document lower layer ordering

Seunghun Lee (1):
      ovl: Prevent rw remount when it should be ro mount

hujianyang (5):
      ovl: Cleanup redundant blank lines
      ovl: Use macros to present ovl_xattr
      ovl: Fix kernel panic while mounting overlayfs
      ovl: Fix opaque regression in ovl_lookup
      ovl: discard independent cursor in readdir()

---
 Documentation/filesystems/overlayfs.txt |  28 ++
 fs/overlayfs/copy_up.c                  |   5 +-
 fs/overlayfs/dir.c                      |  28 +-
 fs/overlayfs/inode.c                    |  12 +-
 fs/overlayfs/overlayfs.h                |  18 +-
 fs/overlayfs/readdir.c                  | 181 +++++-----
 fs/overlayfs/super.c                    | 564 +++++++++++++++++++++-----------
 7 files changed, 517 insertions(+), 319 deletions(-)

Re: [GIT PULL] multi-layer support for overlay filesystem

[Miklos Szeredi]

On Wed, Feb 11, 2015 at 10:30:39AM +0100, Miklos Szeredi wrote:
> Al,
> 
> Please pull from
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next
> 
> This adds support for multiple read-only layers to overlayfs.  It also makes the
> writable upper layer optional.
> 
> This is a highly requested feature and has been in -next for the last cycle.

Ping.

Not sure whom I should send overlayfs pull requests to.  Would direct to Linus
be better?

Thanks,
Miklos


> ---
> Miklos Szeredi (16):
>       ovl: check whiteout while reading directory
>       ovl: make path-type a bitmap
>       ovl: dont replace opaque dir
>       ovl: add mutli-layer infrastructure
>       ovl: helper to iterate layers
>       ovl: multi-layer readdir
>       ovl: multi-layer lookup
>       ovl: check whiteout on lowest layer as well
>       ovl: lookup ENAMETOOLONG on lower means ENOENT
>       ovl: allow statfs if no upper layer
>       ovl: mount: change order of initialization
>       ovl: improve mount helpers
>       ovl: make upperdir optional
>       ovl: support multiple lower layers
>       ovl: add testsuite to docs
>       ovl: document lower layer ordering
> 
> Seunghun Lee (1):
>       ovl: Prevent rw remount when it should be ro mount
> 
> hujianyang (5):
>       ovl: Cleanup redundant blank lines
>       ovl: Use macros to present ovl_xattr
>       ovl: Fix kernel panic while mounting overlayfs
>       ovl: Fix opaque regression in ovl_lookup
>       ovl: discard independent cursor in readdir()
> 
> ---
>  Documentation/filesystems/overlayfs.txt |  28 ++
>  fs/overlayfs/copy_up.c                  |   5 +-
>  fs/overlayfs/dir.c                      |  28 +-
>  fs/overlayfs/inode.c                    |  12 +-
>  fs/overlayfs/overlayfs.h                |  18 +-
>  fs/overlayfs/readdir.c                  | 181 +++++-----
>  fs/overlayfs/super.c                    | 564 +++++++++++++++++++++-----------
>  7 files changed, 517 insertions(+), 319 deletions(-)

Re: [GIT PULL] multi-layer support for overlay filesystem

[Al Viro]

On Wed, Feb 18, 2015 at 11:59:03AM +0100, Miklos Szeredi wrote:
> On Wed, Feb 11, 2015 at 10:30:39AM +0100, Miklos Szeredi wrote:
> > Al,
> > 
> > Please pull from
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next
> > 
> > This adds support for multiple read-only layers to overlayfs.  It also makes the
> > writable upper layer optional.
> > 
> > This is a highly requested feature and has been in -next for the last cycle.
> 
> Ping.

Pong.  I'm still looking through a large pile of assorted stuff, that pull
request sitting in the queue.  Should be done with that tomorrow (as in "pull
or reply with last-minute-found $SOMETHING_DIRE"; FWIW, the last time I looked
at multilayer stuff was a couple of weeks ago and nothing dire had been spotted,
so the former is more likely).

ObSomethingDire^H^H^H^HOddElsewhere: what happens if libfuse
fuse_lowlevel_notify_store() is called with SPLICE_F_MOVE in flags on an
inode that happens to be mmaped with MAP_SHARED?  Cache coherency isn't the
main concern; I'm looking at
        /*
         * This is a new and locked page, it shouldn't be mapped or
         * have any special flags on it
         */
        if (WARN_ON(page_mapped(oldpage)))
                goto out_fallback_unlock;
        if (WARN_ON(page_has_private(oldpage)))
                goto out_fallback_unlock;
        if (WARN_ON(PageDirty(oldpage) || PageWriteback(oldpage)))
                goto out_fallback_unlock;
        if (WARN_ON(PageMlocked(oldpage)))
                goto out_fallback_unlock;
in fuse_try_move_page() and I don't see anything to prevent those
WARN_ON getting triggered in that case.  The call chain is
fuse_try_move_page() <- fuse_copy_page() <- fuse_notify_store() <-
fuse_notify() <- fuse_dev_do_write() <- fuse_dev_splice_write(),
oldpage is obtained by
                page = find_or_create_page(mapping, index,
                                           mapping_gfp_mask(mapping));
in the loop in fuse_notify_store() and that ought to be able to pick
the pages present in MAP_SHARED mappings...

What am I missing here?  FWIW, I'd been trying to resurrect SPLICE_F_MOVE
for local filesystems, and with FUSE being the only place in the kernel
still trying to support it...

[GIT PULL] multi-layer support for overlay filesystem

[Miklos Szeredi]

Al,

Please pull from

  git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next

This adds support for multiple read-only layers to overlayfs.  It also makes the
writable upper layer optional.

Thanks,
Miklos

---
Miklos Szeredi (12):
      ovl: check whiteout while reading directory
      ovl: make path-type a bitmap
      ovl: dont replace opaque dir
      ovl: add mutli-layer infrastructure
      ovl: helper to iterate layers
      ovl: multi-layer readdir
      ovl: multi-layer lookup
      ovl: allow statfs if no upper layer
      ovl: mount: change order of initialization
      ovl: improve mount helpers
      ovl: make upperdir optional
      ovl: support multiple lower layers

---
 Documentation/filesystems/overlayfs.txt |  12 +
 fs/overlayfs/copy_up.c                  |   4 +-
 fs/overlayfs/dir.c                      |  24 +-
 fs/overlayfs/inode.c                    |   9 +-
 fs/overlayfs/overlayfs.h                |  14 +-
 fs/overlayfs/readdir.c                  | 120 +++-----
 fs/overlayfs/super.c                    | 521 ++++++++++++++++++++------------
 7 files changed, 427 insertions(+), 277 deletions(-)

Re: [GIT PULL] multi-layer support for overlay filesystem

[Al Viro]

On Tue, Dec 09, 2014 at 11:37:45AM +0100, Miklos Szeredi wrote:
> Al,
> 
> Please pull from
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next
> 
> This adds support for multiple read-only layers to overlayfs.  It also makes the
> writable upper layer optional.

First of all, your checks in ovl_mount_dir_noesc() have no effect besides
spamming the logs.  In
        if (err) 
                pr_err("overlayfs: failed to resolve '%s': %i\n", name, err);
        else if (!ovl_is_allowed_fs_type(path->dentry))
                pr_err("overlayfs: filesystem on '%s' not supported\n", name);
        else if (!S_ISDIR(path->dentry->d_inode->i_mode))
                pr_err("overlayfs: '%s' not a directory\n", name);
        else   
                err = 0;
the fourth alternative is completely pointless and both the second and the
third leave you with err being zero.

That opens all kinds of unpleasant holes, obviously.  If you fix that by
setting err in the 2nd and thr 3rd alternatives, you get the next problem -
leaks on such failures.  Suppose the first ovl_mount_dir() call fails on
those checks.  You go to out_free_config and voila - upperpath has leaked.
The same in the second call means leaked workpath.  And those failures in
ovl_lower_dir(), as well as failures of vfs_getattr() there, end up leaking
vfsmount/dentry in stack[...].

Next piece of fun: suppose you have in one of the lower layers a filesystem
with ->lookup()-enforced upper limit on name length.  Pretty much every
local fs has one, but... they are not all equal.  255 characters is the
common upper limit, but e.g. jffs2 stops at 254, minixfs upper limit is
somewhere from 14 to 60, depending upon version, etc.  You are doing a lookup
for something that is present in upper layer, but happens to be too long for
one of the lower layers.  Too bad - ENAMETOOLONG for you...

BTW, that sucker really needs cleaning up.  I seriously suspect that
quite a few conditions in there might be redundant, but after several
hours of staring at it I'm still not sure that I hadn't missed some
paths in there ;-/  AFAICS, it would get cleaner if you unrolled the idx == 0
pass out of that loop - as in "deal with ovl_path_upper() if present, then
simple for (lower = 0; lower < oe->numlower; lower++)".  TBH, ovl_path_next()
seems to be a bad primitive for that use and the other caller doesn't give
a damn about upper vs. lower, so I wonder if the games with reporting that
make any sense.  They certainly make ovl_path_next() much uglier...

Another thing: in theory, you can get up to about 2000 (identical)
single-letter names in lowerdirs.  Resulting arrays of struct path (i.e.
pairs of pointers) will make allocators unhappy - 32Kb kmalloc() is
not nice.  IMO that needs more or less sane limit enforced at mount time -
relying on "mount data is at most one page, can't fit too much there"
is not enough.

Logics in ovl_dir_read_merged() looks odd - why is the lowermost one special
and not the uppermost?  BTW, what if you find a whiteout in the lowermost
layer?  And while we are at it, what happens when the _upper_ layer is an
overlayfs - how do you create whiteouts there?  That, AFAICS, applies to
the current mainline as well...

Anyway, more after I get some sleep - it's nearly 5am here ;-/

Re: [GIT PULL] multi-layer support for overlay filesystem

[Miklos Szeredi]

Thanks for the review.

On Fri, Dec 12, 2014 at 09:47:39AM +0000, Al Viro wrote:
> First of all, your checks in ovl_mount_dir_noesc() have no effect besides
> spamming the logs.  In
>         if (err) 
>                 pr_err("overlayfs: failed to resolve '%s': %i\n", name, err);
>         else if (!ovl_is_allowed_fs_type(path->dentry))
>                 pr_err("overlayfs: filesystem on '%s' not supported\n", name);
>         else if (!S_ISDIR(path->dentry->d_inode->i_mode))
>                 pr_err("overlayfs: '%s' not a directory\n", name);
>         else   
>                 err = 0;
> the fourth alternative is completely pointless and both the second and the
> third leave you with err being zero.

Fixed.

> 
> That opens all kinds of unpleasant holes, obviously.  If you fix that by
> setting err in the 2nd and thr 3rd alternatives, you get the next problem -
> leaks on such failures.  Suppose the first ovl_mount_dir() call fails on
> those checks.  You go to out_free_config and voila - upperpath has leaked.
> The same in the second call means leaked workpath.  And those failures in
> ovl_lower_dir(), as well as failures of vfs_getattr() there, end up leaking
> vfsmount/dentry in stack[...].

Fixed.

> 
> Next piece of fun: suppose you have in one of the lower layers a filesystem
> with ->lookup()-enforced upper limit on name length.  Pretty much every
> local fs has one, but... they are not all equal.  255 characters is the
> common upper limit, but e.g. jffs2 stops at 254, minixfs upper limit is
> somewhere from 14 to 60, depending upon version, etc.  You are doing a lookup
> for something that is present in upper layer, but happens to be too long for
> one of the lower layers.  Too bad - ENAMETOOLONG for you...

Fixed.

> 
> BTW, that sucker really needs cleaning up.  I seriously suspect that
> quite a few conditions in there might be redundant, but after several
> hours of staring at it I'm still not sure that I hadn't missed some
> paths in there ;-/  AFAICS, it would get cleaner if you unrolled the idx == 0
> pass out of that loop - as in "deal with ovl_path_upper() if present, then
> simple for (lower = 0; lower < oe->numlower; lower++)".  TBH, ovl_path_next()
> seems to be a bad primitive for that use and the other caller doesn't give
> a damn about upper vs. lower, so I wonder if the games with reporting that
> make any sense.  They certainly make ovl_path_next() much uglier...

Done cleanup.

> 
> Another thing: in theory, you can get up to about 2000 (identical)
> single-letter names in lowerdirs.  Resulting arrays of struct path (i.e.
> pairs of pointers) will make allocators unhappy - 32Kb kmalloc() is
> not nice.  IMO that needs more or less sane limit enforced at mount time -
> relying on "mount data is at most one page, can't fit too much there"
> is not enough.

Fixed.

> 
> Logics in ovl_dir_read_merged() looks odd - why is the lowermost one special
> and not the uppermost?  BTW, what if you find a whiteout in the lowermost
> layer?
Yeah, it was supposed to be an optimization (no need to check for whiteouts on
lower, since whiteouts never exist on lower) and is present in the lookup code
as well.

I switched to checking whiteouts in lowest layer as well, for consitency.

>   And while we are at it, what happens when the _upper_ layer is an
> overlayfs - how do you create whiteouts there?  That, AFAICS, applies to
> the current mainline as well...

It really doesn't make sense to make upper layer an overlayfs.  It won't work,
but it shouldn't crash.  Should we blacklist it, so any such stupidity is found
out at mount time?

Fixes pushed to

   git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next

Thanks,
Miklos

---
Miklos Szeredi (15):
      ovl: check whiteout while reading directory
      ovl: make path-type a bitmap
      ovl: dont replace opaque dir
      ovl: add mutli-layer infrastructure
      ovl: helper to iterate layers
      ovl: multi-layer readdir
      ovl: multi-layer lookup
      ovl: check whiteout on lowest layer as well
      ovl: lookup ENAMETOOLONG on lower means ENOENT
      ovl: allow statfs if no upper layer
      ovl: mount: change order of initialization
      ovl: improve mount helpers
      ovl: make upperdir optional
      ovl: support multiple lower layers
      ovl: add testsuite to docs

hujianyang (2):
      ovl: Cleanup redundant blank lines
      ovl: Use macros to present ovl_xattr

---
 Documentation/filesystems/overlayfs.txt |  24 ++
 fs/overlayfs/copy_up.c                  |   5 +-
 fs/overlayfs/dir.c                      |  28 +-
 fs/overlayfs/inode.c                    |  12 +-
 fs/overlayfs/overlayfs.h                |  18 +-
 fs/overlayfs/readdir.c                  | 145 ++++-----
 fs/overlayfs/super.c                    | 546 +++++++++++++++++++++-----------
 7 files changed, 482 insertions(+), 296 deletions(-)