[PATCH 0/3] overlayfs fixes for v3.8 and later base

[PATCH 0/3] overlayfs fixes for v3.8 and later base

[Andy Whitcroft]

Following this email are three patches against overlayfs.v17 which allow
it to work against v3.10-rc6 and later.  With these applied overlayfs
passes basic testing.

overlayfs -- ovl_path_open should not take path reference

    Found this in testing on Ubuntu raring, testing against loopback
    mounted files.  Without this change we were unable to release the
    loopback device for reuse.  Looking at it actually we were also leaking
    references on the root filesystem, but these are not as obvious.

vfs: export do_splice_direct() to modules -- fix
overlayfs -- follow change to do_splice_direct interface

    This pair switch us to the new do_splice_direct interface.  Note that
    the first one is effectivly a fix for the existing export of this
    interface and likely should be merged down into it.

Please consider for your overlayfs branch.

-apw

[PATCH 1/1] overlayfs -- ovl_path_open should not take path reference

[Andy Whitcroft]

Since the commit below dentry_open now takes its own references
as required.  We therefore should no longer take path references in
ovl_path_open.  Doing so leaves stray mount references to the underlying
devices preventing them being released:

    commit 765927b2d508712d320c8934db963bbe14c3fcec
    Author: Al Viro <viro@zeniv.linux.org.uk>
    Date:   Tue Jun 26 21:58:53 2012 +0400

        switch dentry_open() to struct path, make it grab references itself

BugLink: http://bugs.launchpad.net/bugs/1098378
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
 fs/overlayfs/super.c | 1 -
 1 file changed, 1 deletion(-)


    Found this in testing on Ubuntu raring, testing against loopback
    mounted files.  Without this change we were unable to release the
    loopback device for reuse.  Looking at it actually we were also leaking
    references on the root filesystem, but these are not as obvious.
    Applies against overlayfs.v17 as rebased to 3.8 and later.

    -apw


diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 482c26f..9473e79 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -385,7 +385,6 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 
 struct file *ovl_path_open(struct path *path, int flags)
 {
-	path_get(path);
 	return dentry_open(path, flags, current_cred());
 }
 
-- 
1.8.3.1

[PATCH 1/2] vfs: export do_splice_direct() to modules -- fix

[Andy Whitcroft]

The commit below moved the prototype for do_splice_direct to
fs/internal.h expose it:

  commit 7995bd287134f6c8f80d94bebe7396f05a9bc42b
  Author: Al Viro <viro@zeniv.linux.org.uk>
  Date:   Thu Jun 20 18:58:36 2013 +0400

    splice: don't pass the address of ->f_pos to methods

[This should be merged down into:
 vfs: export do_splice_direct() to modules.]

Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
 fs/internal.h      | 6 ------
 include/linux/fs.h | 6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 6dd0ffd..6c9ec69 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -127,12 +127,6 @@ extern struct dentry *__d_alloc(struct super_block *, const struct qstr *);
 extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
 
 /*
- * splice.c
- */
-extern long do_splice_direct(struct file *in, loff_t *ppos, struct file *out,
-		loff_t *opos, size_t len, unsigned int flags);
-
-/*
  * pipe.c
  */
 extern const struct file_operations pipefifo_fops;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e623f11..baf1175 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2703,4 +2703,10 @@ static inline void inode_has_no_xattr(struct inode *inode)
 		inode->i_flags |= S_NOSEC;
 }
 
+/*
+ * splice.c
+ */
+extern long do_splice_direct(struct file *in, loff_t *ppos, struct file *out,
+		loff_t *opos, size_t len, unsigned int flags);
+
 #endif /* _LINUX_FS_H */
-- 
1.8.3.1

[PATCH 2/2] overlayfs -- follow change to do_splice_direct interface

[Andy Whitcroft]

The commit below changed the interface to do_splice_direct, follow that
change in copy_up:

  commit 7995bd287134f6c8f80d94bebe7396f05a9bc42b
  Author: Al Viro <viro@zeniv.linux.org.uk>
  Date:   Thu Jun 20 18:58:36 2013 +0400

    splice: don't pass the address of ->f_pos to methods

Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
 fs/overlayfs/copy_up.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index eef85e0..8e1b09f 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -90,9 +90,10 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
 
 	/* FIXME: copy up sparse files efficiently */
 	while (len) {
-		loff_t offset = new_file->f_pos;
 		size_t this_len = OVL_COPY_UP_CHUNK_SIZE;
 		long bytes;
+		loff_t pos = old_file->f_pos;
+		loff_t out_pos = new_file->f_pos;
 
 		if (len < this_len)
 			this_len = len;
@@ -102,7 +103,7 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
 			break;
 		}
 
-		bytes = do_splice_direct(old_file, &offset, new_file, this_len,
+		bytes = do_splice_direct(old_file, &pos, new_file, &out_pos, this_len,
 				 SPLICE_F_MOVE);
 		if (bytes <= 0) {
 			error = bytes;
-- 
1.8.3.1

Re: [PATCH 2/2] overlayfs -- follow change to do_splice_direct interface

[Miklos Szeredi]

Thanks for the patches, Andy.  I already pushed a similar set of fixes
to overlayfs.current (.v18) and to .v17 for the dentry_open() refcount
fix.

On Thu, Jun 27, 2013 at 6:26 PM, Andy Whitcroft <apw@canonical.com> wrote:
> The commit below changed the interface to do_splice_direct, follow that
> change in copy_up:
>
>   commit 7995bd287134f6c8f80d94bebe7396f05a9bc42b
>   Author: Al Viro <viro@zeniv.linux.org.uk>
>   Date:   Thu Jun 20 18:58:36 2013 +0400
>
>     splice: don't pass the address of ->f_pos to methods
>
> Signed-off-by: Andy Whitcroft <apw@canonical.com>
> ---
>  fs/overlayfs/copy_up.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index eef85e0..8e1b09f 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -90,9 +90,10 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
>
>         /* FIXME: copy up sparse files efficiently */
>         while (len) {
> -               loff_t offset = new_file->f_pos;
>                 size_t this_len = OVL_COPY_UP_CHUNK_SIZE;
>                 long bytes;
> +               loff_t pos = old_file->f_pos;
> +               loff_t out_pos = new_file->f_pos;
>
>                 if (len < this_len)
>                         this_len = len;
> @@ -102,7 +103,7 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
>                         break;
>                 }
>
> -               bytes = do_splice_direct(old_file, &offset, new_file, this_len,
> +               bytes = do_splice_direct(old_file, &pos, new_file, &out_pos, this_len,


It is interesting how many people end up fixing this the wrong way ;)

Hint: how will the file offsets advance?

Thanks,
Miklos

Re: [PATCH 2/2] overlayfs -- follow change to do_splice_direct interface

[Andy Whitcroft]

On Thu, Jun 27, 2013 at 06:37:45PM +0200, Miklos Szeredi wrote:
> Thanks for the patches, Andy.  I already pushed a similar set of fixes
> to overlayfs.current (.v18) and to .v17 for the dentry_open() refcount
> fix.

Bah so you did, that'll teach me for waiting for them to be tested.

> It is interesting how many people end up fixing this the wrong way ;)
> 
> Hint: how will the file offsets advance?

Interesting I got bluffed in by the original implementation which
reloaded the source pos each time.

-apw