[PATCH 0/8] overlayfs: fix ro/rw fd data inconsistecies

[PATCH 0/8] overlayfs: fix ro/rw fd data inconsistecies

[Miklos Szeredi]

Hi Al,

I'd like to hear your opinion on this patchset before queuing up for 4.10.

Here's the problem:

A file is opened for read-only, opened read-write (resulting in a copy up)
and modified.  The data read back from the the read-only fd will be stale
in this case (the read-only file descriptor still refers to the lower,
unmodified file).

This patchset fixes issues related to this corner case.

The VFS impact is minimal and performance in the non-corner cases shouldn't
suffer.

Git tree is here:

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

Pathset is against #overlayfs-next.

Thanks,
Miklos

---
Miklos Szeredi (8):
  vfs: allow overlayfs to intercept file ops
  vfs: export filp_clone_open()
  mm: ovl: copy-up on MAP_SHARED
  ovl: add infrastructure for intercepting file ops
  ovl: intercept read_iter
  ovl: intercept mmap
  ovl: intercept fsync
  Revert "ovl: Warn on copy up if a process has a R/O fd open to the
    lower file"

 fs/internal.h            |   1 -
 fs/open.c                |   2 +-
 fs/overlayfs/copy_up.c   |  34 -------
 fs/overlayfs/inode.c     | 227 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/overlayfs/overlayfs.h |   2 +
 fs/overlayfs/super.c     |   1 +
 include/linux/fs.h       |   1 +
 mm/util.c                |  22 +++++
 8 files changed, 254 insertions(+), 36 deletions(-)

-- 
2.5.5

[PATCH 1/8] vfs: allow overlayfs to intercept file ops

[Miklos Szeredi]

Almost all of the time overlayfs just wants to let underlying filesystems
handle file operations on regular files.

There is a rare corner case, namely when a file residing a read-only
(lower) layer is:

 - opened for O_RDONLY
 - opened for O_WRONLY
 - contents modified
 - contents read back though O_RDONLY fd

Currently overlayfs gives inconsistent result in this case, since the
O_RDONLY file refers to the lower, unmodified file, even after the copy up
has happened.

This happens very rarely, so

a) we want the normal cases (when no copy up happens) still go as fast as
   possible;

b) we can let the corner case go slow.

The proposed solution is to allow overlayfs to intercept file operations if
it wants (O_RDONLY open of file on lower layer), but still let the file be
owned by the underlying layer.  This means everything in filp, including
the private_data, is controlled by the underlying layer (as it has been
until now) with the exception of f_op, which comes from overlayfs.

This patch doesn't change the actual behavior, it just adds the vfs change
necessary and then unconditionally sets the f_op back to the underlying
file's ops.

For non-overlayfs, this doesn't change anything.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/open.c            |  2 +-
 fs/overlayfs/inode.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/fs/open.c b/fs/open.c
index 8a0254b56780..453ef5581389 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -734,7 +734,7 @@ static int do_dentry_open(struct file *f,
 	if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode))
 		f->f_mode |= FMODE_ATOMIC_POS;
 
-	f->f_op = fops_get(inode->i_fop);
+	f->f_op = fops_get(f->f_path.dentry->d_inode->i_fop);
 	if (unlikely(WARN_ON(!f->f_op))) {
 		error = -ENODEV;
 		goto cleanup_all;
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index a10e948d24fa..1981a5514f51 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 #include <linux/xattr.h>
 #include <linux/posix_acl.h>
+#include <linux/module.h>
 #include "overlayfs.h"
 
 static int ovl_copy_up_truncate(struct dentry *dentry)
@@ -333,6 +334,22 @@ static const struct inode_operations ovl_symlink_inode_operations = {
 	.update_time	= ovl_update_time,
 };
 
+static int ovl_open(struct inode *inode, struct file *file)
+{
+	int ret = 0;
+
+	/* Want fops from real inode */
+	replace_fops(file, inode->i_fop);
+	if (file->f_op->open)
+		ret = file->f_op->open(inode, file);
+
+	return ret;
+}
+
+static const struct file_operations ovl_file_operations = {
+	.open		= ovl_open,
+};
+
 static void ovl_fill_inode(struct inode *inode, umode_t mode, dev_t rdev)
 {
 	inode->i_ino = get_next_ino();
@@ -345,6 +362,7 @@ static void ovl_fill_inode(struct inode *inode, umode_t mode, dev_t rdev)
 	switch (mode & S_IFMT) {
 	case S_IFREG:
 		inode->i_op = &ovl_file_inode_operations;
+		inode->i_fop = &ovl_file_operations;
 		break;
 
 	case S_IFDIR:
-- 
2.5.5

[PATCH 2/8] vfs: export filp_clone_open()

[Miklos Szeredi]

Already exported to modules, just needs to move declaration from
"fs/internal.h" to <linux/fs.h>.

This will be used by overlayfs to clone a read-only file that has been
copied up after being opened.  The clone will refer to the copied-up uppper
file instead of the original, lower file.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/internal.h      | 1 -
 include/linux/fs.h | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/internal.h b/fs/internal.h
index f4da3341b4a3..361ba1c12698 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -112,7 +112,6 @@ extern long do_handle_open(int mountdirfd,
 			   struct file_handle __user *ufh, int open_flag);
 extern int open_check_o_direct(struct file *f);
 extern int vfs_open(const struct path *, struct file *, const struct cred *);
-extern struct file *filp_clone_open(struct file *);
 
 /*
  * inode.c
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 87a32f999812..e3bc970214e1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2355,6 +2355,7 @@ extern struct file *filp_open(const char *, int, umode_t);
 extern struct file *file_open_root(struct dentry *, struct vfsmount *,
 				   const char *, int, umode_t);
 extern struct file * dentry_open(const struct path *, int, const struct cred *);
+extern struct file *filp_clone_open(struct file *);
 extern int filp_close(struct file *, fl_owner_t id);
 
 extern struct filename *getname_flags(const char __user *, int, int *);
-- 
2.5.5

[PATCH 3/8] mm: ovl: copy-up on MAP_SHARED

[Miklos Szeredi]

A corner case of a corner case is when

 - file opened for O_RDONLY
 - which is then memory mapped SHARED
 - file opened for O_WRONLY
 - contents modified
 - contents read back though the shared mapping

Unfortunately it looks very difficult to do anything about the established
shared map after the file is copied up.  Instead when a read-only file is
mapped shared overlayfs copies up the file before actually doing the map.
This may result in unnecessary copy-ups (but so may copy-up on open(O_RDWR)
for exampe).

We can revisit this later if it turns out to be a performance problem in
real life.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 mm/util.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/mm/util.c b/mm/util.c
index 1a41553db866..449272271840 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -300,6 +300,28 @@ unsigned long vm_mmap_pgoff(struct file *file, unsigned long addr,
 
 	ret = security_mmap_file(file, prot, flag);
 	if (!ret) {
+		/*
+		 * Special treatment for overlayfs:
+		 *
+		 * Take MAP_SHARED/PROT_READ as hint about future writes to the
+		 * file (through another file descriptor).  Caller might not
+		 * have had such an intent, but we hope MAP_PRIVATE will be used
+		 * in most such cases.
+		 *
+		 * If we don't copy up now and the file is modified, it becomes
+		 * really difficult to change the mapping to match that of the
+		 * file's content later.
+		 *
+		 * Copy up needs to be done without mmap_sem since it takes vfs
+		 * locks which would potentially deadlock under mmap_sem.
+		 */
+		if ((flag & MAP_SHARED) && !(prot & PROT_WRITE) && file) {
+			void *p = d_real(file->f_path.dentry, NULL, O_WRONLY);
+
+			if (IS_ERR(p))
+				return PTR_ERR(p);
+		}
+
 		if (down_write_killable(&mm->mmap_sem))
 			return -EINTR;
 		ret = do_mmap_pgoff(file, addr, len, prot, flag, pgoff,
-- 
2.5.5

[PATCH 4/8] ovl: add infrastructure for intercepting file ops

[Miklos Szeredi]

Overlayfs needs to intercept a few file operations in order to properly
handle the following corner case:

 - file X is in overlayfs;

 - X resides on a lower (read-only) layer
	the lower file is L;

 - X is opened with O_RDONLY ->
	L is opened -> 'rofd';

 - X is opened with O_WRONLY ->
	this results in L being copied up to the upper layer file U
	U is opened -> 'rwfd';

 - write to 'rwfd' modifying U;

 - read from 'rofd' gets data from unmodified L;

While this is a rare occurrence, it has been known to cause problems.  To
prevent such inconsistencies, allow intercepting read operations and fix
them up in the unlikely case that it's needed.

This patch adds an ovl_fops structure that is going to contain the pieces
necessary for intercepting file operations:

 a) a new file_operations structure, based on the original but changing
    those operations that need to be intercepted;

 b) a pointer to the origin f_op.  Intercepted operations will normally
    just call the original, unless the file was copied up;

 c) a hash table entry to hook this up for reuse in subsequent opens.

The hash table is small (32 buckets) since there will only be a few
different file operations used (basically the number of different
filesystems used as lower layer of overlay).  The key to the has table is
the original fops pointer.

Insertion is done under a global mutex.  There will only be one insertion
event for each unique underlying file_operations structure, so this is
going to be rare.

The hash table is accessed using rcu, so this will add minimal overhead to
opens.  The override fops are removed only at module exit time, so we don't
even have to be worry about read-side rcu locking.

There are a few assumption this scheme makes about file operation
structures supplied by filesystems:

 1) the lifetime of the structures is equal to the lifetime of the
    filesystem module; ensure this by holding a ref to the
    sb->s_type->owner (normal filesystems don't fill in f_ops->owner).
    This means that once a filesystem has been used as lower layer of
    overlayfs its module cannot be removed until the overlay module itself
    is removed.  I don't believe this will be a problem.

 2) Filesystems should not make use of f_op in any way except possibly
    replacing it in ->open.  Overlayfs itself breaks this assumption, but
    recursion is handled by ->d_real so this is not an issue.  Add a
    ->magic field to the override fops structure to verify that the
    filesystem didn't mess with file->f_op.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/overlayfs/inode.c     | 146 ++++++++++++++++++++++++++++++++++++++++++++++-
 fs/overlayfs/overlayfs.h |   2 +
 fs/overlayfs/super.c     |   1 +
 3 files changed, 148 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 1981a5514f51..e98e3323c330 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -12,6 +12,7 @@
 #include <linux/xattr.h>
 #include <linux/posix_acl.h>
 #include <linux/module.h>
+#include <linux/hashtable.h>
 #include "overlayfs.h"
 
 static int ovl_copy_up_truncate(struct dentry *dentry)
@@ -334,16 +335,159 @@ static const struct inode_operations ovl_symlink_inode_operations = {
 	.update_time	= ovl_update_time,
 };
 
+static DEFINE_READ_MOSTLY_HASHTABLE(ovl_fops_htable, 5);
+static DEFINE_MUTEX(ovl_fops_mutex);
+
+#define OVL_FOPS_MAGIC 0x73706f462a6c764f
+
+struct ovl_fops {
+	struct module *owner;
+	struct file_operations fops;
+	u64 magic;
+	const struct file_operations *orig_fops;
+	struct hlist_node entry;
+};
+
+void ovl_cleanup_fops_htable(void)
+{
+	int bkt;
+	struct hlist_node *tmp;
+	struct ovl_fops *ofop;
+
+	hash_for_each_safe(ovl_fops_htable, bkt, tmp, ofop, entry) {
+		module_put(ofop->owner);
+		fops_put(ofop->orig_fops);
+		kfree(ofop);
+	}
+}
+
+#define OVL_CALL_REAL_FOP(file, call) \
+	({ struct ovl_fops *__ofop =					 \
+			container_of(file->f_op, struct ovl_fops, fops); \
+	   WARN_ON(__ofop->magic != OVL_FOPS_MAGIC) ? -EIO :		 \
+		   __ofop->orig_fops->call;				 \
+	})
+
+static struct ovl_fops *ovl_fops_find(const struct file_operations *orig)
+{
+	struct ovl_fops *ofop;
+
+	hash_for_each_possible_rcu(ovl_fops_htable, ofop, entry, (long) orig) {
+		if (ofop->orig_fops == orig)
+			return ofop;
+	}
+	return NULL;
+}
+
+static struct ovl_fops *ovl_fops_get(struct file *file)
+{
+	const struct file_operations *orig = file->f_op;
+	struct ovl_fops *ofop = ovl_fops_find(orig);
+
+	if (likely(ofop))
+		return ofop;
+
+	mutex_lock(&ovl_fops_mutex);
+	ofop = ovl_fops_find(orig);
+	if (ofop)
+		goto out_unlock;
+
+	ofop = kzalloc(sizeof(struct ovl_fops), GFP_KERNEL);
+	if (!ofop)
+		goto out_unlock;
+
+	/*
+	 * FS don't usually fill in fops->owner, so grab ref to filesystem's
+	 * module as well.
+	 */
+	ofop->owner = file_inode(file)->i_sb->s_type->owner;
+	__module_get(ofop->owner);
+
+	ofop->magic = OVL_FOPS_MAGIC;
+	ofop->orig_fops = fops_get(orig);
+
+	/* These will need to be intercepted: */
+	ofop->fops.read_iter = orig->read_iter;
+	ofop->fops.mmap = orig->mmap;
+	ofop->fops.fsync = orig->fsync;
+
+	/*
+	 * These should be intercepted, but they are very unlikely to be
+	 * a problem in practice.  Leave them alone for now.
+	 */
+	ofop->fops.copy_file_range = orig->copy_file_range;
+	ofop->fops.clone_file_range = orig->clone_file_range;
+	ofop->fops.dedupe_file_range = orig->dedupe_file_range;
+
+	/* Don't intercept these: */
+	ofop->fops.llseek = orig->llseek;
+	ofop->fops.unlocked_ioctl = orig->unlocked_ioctl;
+	ofop->fops.compat_ioctl = orig->compat_ioctl;
+	ofop->fops.flush = orig->flush;
+	ofop->fops.release = orig->release;
+	ofop->fops.get_unmapped_area = orig->get_unmapped_area;
+	ofop->fops.check_flags = orig->check_flags;
+
+	/* splice_read should be generic_file_splice_read */
+	WARN_ON(orig->splice_read != generic_file_splice_read);
+	ofop->fops.splice_read = generic_file_splice_read;
+
+	/* These make no sense for "normal" files: */
+	WARN_ON(orig->read);
+	WARN_ON(orig->iterate);
+	WARN_ON(orig->iterate_shared);
+	WARN_ON(orig->poll);
+	WARN_ON(orig->fasync);
+	WARN_ON(orig->show_fdinfo);
+
+	/*
+	 * Don't add those which are unneeded for O_RDONLY:
+	 *
+	 *  write
+	 *  write_iter
+	 *  splice_write
+	 *  sendpage
+	 *  fallocate
+	 *
+	 * Locking operations are already intercepted by vfs for ovl:
+	 *
+	 *  lock
+	 *  flock
+	 *  setlease
+	 */
+
+	hash_add_rcu(ovl_fops_htable, &ofop->entry, (long) orig);
+
+out_unlock:
+	mutex_unlock(&ovl_fops_mutex);
+
+	return ofop;
+}
+
 static int ovl_open(struct inode *inode, struct file *file)
 {
 	int ret = 0;
+	struct ovl_fops *ofop;
+	bool isupper = OVL_TYPE_UPPER(ovl_path_type(file->f_path.dentry));
 
 	/* Want fops from real inode */
 	replace_fops(file, inode->i_fop);
 	if (file->f_op->open)
 		ret = file->f_op->open(inode, file);
 
-	return ret;
+	/* No need to override fops for upper */
+	if (isupper || ret)
+		return ret;
+
+	ofop = ovl_fops_get(file);
+	if (unlikely(!ofop)) {
+		if (file->f_op->release)
+			file->f_op->release(inode, file);
+		return -ENOMEM;
+	}
+	replace_fops(file, &ofop->fops);
+
+	return 0;
 }
 
 static const struct file_operations ovl_file_operations = {
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index a83de5d5b8a0..a305083112f9 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -209,6 +209,8 @@ static inline void ovl_copyattr(struct inode *from, struct inode *to)
 	to->i_ctime = from->i_ctime;
 }
 
+void ovl_cleanup_fops_htable(void);
+
 /* dir.c */
 extern const struct inode_operations ovl_dir_inode_operations;
 struct dentry *ovl_lookup_temp(struct dentry *workdir, struct dentry *dentry);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 4bd1e9c7246f..f9a2021c9abb 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -963,6 +963,7 @@ static int __init ovl_init(void)
 static void __exit ovl_exit(void)
 {
 	unregister_filesystem(&ovl_fs_type);
+	ovl_cleanup_fops_htable();
 }
 
 module_init(ovl_init);
-- 
2.5.5

[PATCH 5/8] ovl: intercept read_iter

[Miklos Szeredi]

...in order to handle the corner case when the file is copyied up after
being opened read-only.

Can be verified with the following script:

 - 8< - - - - - 8< - - - - - 8< - - - - - 8< - - - -
cd /
rm -rf /tmp/ovl-rorw-test
mkdir /tmp/ovl-rorw-test
cd /tmp/ovl-rorw-test
mkdir -p mnt lower upper work
echo baba > lower/foo
mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work mnt
exec 3< mnt/foo
echo bubu > mnt/foo
cat <&3
exec 3>&-
umount mnt
 - 8< - - - - - 8< - - - - - 8< - - - - - 8< - - - -

Correct output is "bubu", incorrect output is "baba".

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/overlayfs/inode.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index e98e3323c330..a2d5b3d03e14 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -12,6 +12,7 @@
 #include <linux/xattr.h>
 #include <linux/posix_acl.h>
 #include <linux/module.h>
+#include <linux/file.h>
 #include <linux/hashtable.h>
 #include "overlayfs.h"
 
@@ -368,6 +369,29 @@ void ovl_cleanup_fops_htable(void)
 		   __ofop->orig_fops->call;				 \
 	})
 
+static bool ovl_file_is_lower(struct file *file)
+{
+	return !OVL_TYPE_UPPER(ovl_path_type(file->f_path.dentry));
+}
+
+static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *to)
+{
+	struct file *file = iocb->ki_filp;
+	ssize_t ret;
+
+	if (likely(ovl_file_is_lower(file)))
+		return OVL_CALL_REAL_FOP(file, read_iter(iocb, to));
+
+	file = filp_clone_open(file);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	ret = vfs_iter_read(file, to, &iocb->ki_pos);
+	fput(file);
+
+	return ret;
+}
+
 static struct ovl_fops *ovl_fops_find(const struct file_operations *orig)
 {
 	struct ovl_fops *ofop;
@@ -406,8 +430,11 @@ static struct ovl_fops *ovl_fops_get(struct file *file)
 	ofop->magic = OVL_FOPS_MAGIC;
 	ofop->orig_fops = fops_get(orig);
 
+	/* Intercept these: */
+	if (orig->read_iter)
+		ofop->fops.read_iter = ovl_read_iter;
+
 	/* These will need to be intercepted: */
-	ofop->fops.read_iter = orig->read_iter;
 	ofop->fops.mmap = orig->mmap;
 	ofop->fops.fsync = orig->fsync;
 
-- 
2.5.5

[PATCH 6/8] ovl: intercept mmap

[Miklos Szeredi]

... in order to handle the corner case when the file is copied up after
being opened read-only and mapped shared.

Can be verified with the following script:

 - 8< - - - - - 8< - - - - - 8< - - - - - 8< - - - -
cd /
rm -rf /tmp/ovl-rorw-test
mkdir /tmp/ovl-rorw-test
cd /tmp/ovl-rorw-test
cat << EOF > rorw-map.c
#include <fcntl.h>
#include <unistd.h>
#include <string.h>
#include <err.h>
#include <sys/mman.h>

int main(int argc, char *argv[])
{
        int rofd, rwfd;
        int ret;
        char buf[4];
        char *addr;

        rofd = open(argv[1], O_RDONLY);
        if (rofd == -1)
                err(1, "ro open");

        addr = mmap(NULL, 4, PROT_READ, MAP_SHARED, rofd, 0);
        if (addr == MAP_FAILED)
                err(1, "ro mmap");

        if (memcmp(addr, "bubu", 4) == 0)
                errx(1, "identical startup data");

        rwfd = open(argv[1], O_WRONLY);
        if (rwfd == -1)
                err(1, "rw open");

        ret = write(rwfd, "bubu", 4);
        if (ret == -1)
                err(1, "write");
        if (ret < 4)
                errx(1, "short write");

        if (memcmp(addr, "bubu", 4) != 0)
                errx(1, "bad mmap data");

        ret = read(rofd, buf, 4);
        if (ret == -1)
                err(1, "read");
        if (ret < 4)
                errx(1, "short read");
        if (memcmp(buf, "bubu", 4) != 0)
                errx(1, "bad read data");

        return 0;
}
EOF
gcc -o rorw-map rorw-map.c
mkdir -p mnt lower upper work
echo baba > lower/foo
mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work mnt
./rorw-map mnt/foo
umount mnt
 - 8< - - - - - 8< - - - - - 8< - - - - - 8< - - - -

No output means success, "rorw-map: bad mmap data" means failure.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/overlayfs/inode.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index a2d5b3d03e14..210838f97777 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 #include <linux/xattr.h>
 #include <linux/posix_acl.h>
+#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/file.h>
 #include <linux/hashtable.h>
@@ -392,6 +393,25 @@ static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	return ret;
 }
 
+static int ovl_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	if (likely(ovl_file_is_lower(file)))
+		return OVL_CALL_REAL_FOP(file, mmap(file, vma));
+
+	file = filp_clone_open(file);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	fput(vma->vm_file);
+	/* transfer ref: */
+	vma->vm_file = file;
+
+	if (!file->f_op->mmap)
+		return -ENODEV;
+
+	return file->f_op->mmap(file, vma);
+}
+
 static struct ovl_fops *ovl_fops_find(const struct file_operations *orig)
 {
 	struct ovl_fops *ofop;
@@ -433,9 +453,10 @@ static struct ovl_fops *ovl_fops_get(struct file *file)
 	/* Intercept these: */
 	if (orig->read_iter)
 		ofop->fops.read_iter = ovl_read_iter;
+	if (orig->mmap)
+		ofop->fops.mmap = ovl_mmap;
 
 	/* These will need to be intercepted: */
-	ofop->fops.mmap = orig->mmap;
 	ofop->fops.fsync = orig->fsync;
 
 	/*
-- 
2.5.5

[PATCH 7/8] ovl: intercept fsync

[Miklos Szeredi]

Fsync is really an inode operation (AFAICS) so a doing it on a O_RDONLY
file descriptor should flush any data written through an O_WRONLY file
descriptor for example.

To make this work correctly in case the file is copied up after being
opened for read intercept the fsync fop, similarly to read_iter and mmap.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/overlayfs/inode.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 210838f97777..d861aa7462c5 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -412,6 +412,24 @@ static int ovl_mmap(struct file *file, struct vm_area_struct *vma)
 	return file->f_op->mmap(file, vma);
 }
 
+static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
+{
+	int ret;
+
+	if (likely(ovl_file_is_lower(file))) {
+		return OVL_CALL_REAL_FOP(file,
+					 fsync(file, start, end, datasync));
+	}
+	file = filp_clone_open(file);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	ret = vfs_fsync_range(file, start, end, datasync);
+	fput(file);
+
+	return ret;
+}
+
 static struct ovl_fops *ovl_fops_find(const struct file_operations *orig)
 {
 	struct ovl_fops *ofop;
@@ -455,9 +473,8 @@ static struct ovl_fops *ovl_fops_get(struct file *file)
 		ofop->fops.read_iter = ovl_read_iter;
 	if (orig->mmap)
 		ofop->fops.mmap = ovl_mmap;
-
-	/* These will need to be intercepted: */
-	ofop->fops.fsync = orig->fsync;
+	if (orig->fsync)
+		ofop->fops.fsync = ovl_fsync;
 
 	/*
 	 * These should be intercepted, but they are very unlikely to be
-- 
2.5.5

[PATCH 8/8] Revert "ovl: Warn on copy up if a process has a R/O fd open to the lower file"

[Miklos Szeredi]

This reverts commit fb5bb2c3b73df060d588b6521de5ab03589283f7.

The check is no longer needed since the effected corner case is now handled
correctly by overlayfs.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/overlayfs/copy_up.c | 34 ----------------------------------
 1 file changed, 34 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 6c3aaf45e9cf..7bf7167ba61e 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -7,7 +7,6 @@
  * the Free Software Foundation.
  */
 
-#include <linux/module.h>
 #include <linux/fs.h>
 #include <linux/slab.h>
 #include <linux/file.h>
@@ -17,41 +16,10 @@
 #include <linux/uaccess.h>
 #include <linux/sched.h>
 #include <linux/namei.h>
-#include <linux/fdtable.h>
-#include <linux/ratelimit.h>
 #include "overlayfs.h"
 
 #define OVL_COPY_UP_CHUNK_SIZE (1 << 20)
 
-static bool __read_mostly ovl_check_copy_up;
-module_param_named(check_copy_up, ovl_check_copy_up, bool,
-		   S_IWUSR | S_IRUGO);
-MODULE_PARM_DESC(ovl_check_copy_up,
-		 "Warn on copy-up when causing process also has a R/O fd open");
-
-static int ovl_check_fd(const void *data, struct file *f, unsigned int fd)
-{
-	const struct dentry *dentry = data;
-
-	if (f->f_inode == d_inode(dentry))
-		pr_warn_ratelimited("overlayfs: Warning: Copying up %pD, but open R/O on fd %u which will cease to be coherent [pid=%d %s]\n",
-				    f, fd, current->pid, current->comm);
-	return 0;
-}
-
-/*
- * Check the fds open by this process and warn if something like the following
- * scenario is about to occur:
- *
- *	fd1 = open("foo", O_RDONLY);
- *	fd2 = open("foo", O_RDWR);
- */
-static void ovl_do_check_copy_up(struct dentry *dentry)
-{
-	if (ovl_check_copy_up)
-		iterate_fd(current->files, 0, ovl_check_fd, dentry);
-}
-
 int ovl_copy_xattr(struct dentry *old, struct dentry *new)
 {
 	ssize_t list_size, size, value_size = 0;
@@ -339,8 +307,6 @@ int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
 	if (WARN_ON(!workdir))
 		return -EROFS;
 
-	ovl_do_check_copy_up(lowerdentry);
-
 	ovl_path_upper(parent, &parentpath);
 	upperdir = parentpath.dentry;
 
-- 
2.5.5