[PATCH 0/4] assorted overlayfs fixes for v4.14-rc3

[PATCH 0/4] assorted overlayfs fixes for v4.14-rc3

[Amir Goldstein]

Miklos,

Following are 1 fix for a commit in v4.14-rc1
and 3 fixes for index error paths in v4.13.

Amir.

Amir Goldstein (4):
  ovl: fix may_write_real() for overlayfs directories
  ovl: fix error value printed in ovl_lookup_index()
  ovl: fix dput() of ERR_PTR in ovl_cleanup_index()
  ovl: fix dentry leak in ovl_indexdir_cleanup()

 fs/namespace.c         | 4 +++-
 fs/overlayfs/namei.c   | 1 +
 fs/overlayfs/readdir.c | 6 ++++--
 fs/overlayfs/util.c    | 9 ++++++---
 4 files changed, 14 insertions(+), 6 deletions(-)

-- 
2.7.4

[PATCH 1/4] ovl: fix may_write_real() for overlayfs directories

[Amir Goldstein]

Overlayfs directory file_inode() is the overlay inode whether the real
inode is upper or lower.

This fixes a regression in xfstest generic/158.

Fixes: 7c6893e3c9ab ("ovl: don't allow writing ioctl on lower layer")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/namespace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 54059b142d6b..3b601f115b6c 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -468,7 +468,9 @@ static inline int may_write_real(struct file *file)
 
 	/* File refers to upper, writable layer? */
 	upperdentry = d_real(dentry, NULL, 0, D_REAL_UPPER);
-	if (upperdentry && file_inode(file) == d_inode(upperdentry))
+	if (upperdentry &&
+	    (file_inode(file) == d_inode(upperdentry) ||
+	     file_inode(file) == d_inode(dentry)))
 		return 0;
 
 	/* Lower layer: can't write to real file, sorry... */
-- 
2.7.4

[PATCH 2/4] ovl: fix error value printed in ovl_lookup_index()

[Amir Goldstein]

Fixes: 359f392ca53e ("ovl: lookup index entry for copy up origin")
Cc: <stable@vger.kernel.org> # v4.13
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/namei.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index c3addd1114f1..654bea1a5ac9 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -506,6 +506,7 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,
 
 	index = lookup_one_len_unlocked(name.name, ofs->indexdir, name.len);
 	if (IS_ERR(index)) {
+		err = PTR_ERR(index);
 		pr_warn_ratelimited("overlayfs: failed inode index lookup (ino=%lu, key=%*s, err=%i);\n"
 				    "overlayfs: mount with '-o index=off' to disable inodes index.\n",
 				    d_inode(origin)->i_ino, name.len, name.name,
-- 
2.7.4

[PATCH 3/4] ovl: fix dput() of ERR_PTR in ovl_cleanup_index()

[Amir Goldstein]

Fixes: caf70cb2ba5d ("ovl: cleanup orphan index entries")
Cc: <stable@vger.kernel.org> # v4.13
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/util.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 117794582f9f..a1bf81d352b4 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -430,7 +430,7 @@ void ovl_inuse_unlock(struct dentry *dentry)
 	}
 }
 
-/* Called must hold OVL_I(inode)->oi_lock */
+/* Caller must hold OVL_I(inode)->lock */
 static void ovl_cleanup_index(struct dentry *dentry)
 {
 	struct inode *dir = ovl_indexdir(dentry->d_sb)->d_inode;
@@ -466,9 +466,12 @@ static void ovl_cleanup_index(struct dentry *dentry)
 	inode_lock_nested(dir, I_MUTEX_PARENT);
 	/* TODO: whiteout instead of cleanup to block future open by handle */
 	index = lookup_one_len(name.name, ovl_indexdir(dentry->d_sb), name.len);
-	err = PTR_ERR(index);
-	if (!IS_ERR(index))
+	if (!IS_ERR(index)) {
 		err = ovl_cleanup(dir, index);
+	} else {
+		err = PTR_ERR(index);
+		index = NULL;
+	}
 	inode_unlock(dir);
 	if (err)
 		goto fail;
-- 
2.7.4

[PATCH 4/4] ovl: fix dentry leak in ovl_indexdir_cleanup()

[Amir Goldstein]

index dentry was not released when breaking out of the loop
due to index verification error.

Fixes: 415543d5c64f ("ovl: cleanup bad and stale index entries on mount")
Cc: <stable@vger.kernel.org> # v4.13
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/readdir.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 62e9b22a2077..0f85ee9c3268 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -988,6 +988,7 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
 			 struct path *lowerstack, unsigned int numlower)
 {
 	int err;
+	struct dentry *index = NULL;
 	struct inode *dir = dentry->d_inode;
 	struct path path = { .mnt = mnt, .dentry = dentry };
 	LIST_HEAD(list);
@@ -1007,8 +1008,6 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
 
 	inode_lock_nested(dir, I_MUTEX_PARENT);
 	list_for_each_entry(p, &list, l_node) {
-		struct dentry *index;
-
 		if (p->name[0] == '.') {
 			if (p->len == 1)
 				continue;
@@ -1018,6 +1017,7 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
 		index = lookup_one_len(p->name, dentry, p->len);
 		if (IS_ERR(index)) {
 			err = PTR_ERR(index);
+			index = NULL;
 			break;
 		}
 		err = ovl_verify_index(index, lowerstack, numlower);
@@ -1029,7 +1029,9 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
 				break;
 		}
 		dput(index);
+		index = NULL;
 	}
+	dput(index);
 	inode_unlock(dir);
 out:
 	ovl_cache_free(&list);
-- 
2.7.4

[PATCH 5/5] ovl: fix missing unlock_rename() in ovl_do_copy_up()

[Amir Goldstein]

Use the ovl_lock_rename_workdir() helper which requires
unlock_rename() only on lock success.

Fixes: ("fd210b7d67ee ovl: move copy up lock out")
Cc: <stable@vger.kernel.org> # v4.13
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/copy_up.c   |  6 ++----
 fs/overlayfs/dir.c       | 20 --------------------
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/util.c      | 19 +++++++++++++++++++
 4 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index aad97b30d5e6..c441f9387a1b 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -561,10 +561,8 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 		c->tmpfile = true;
 		err = ovl_copy_up_locked(c);
 	} else {
-		err = -EIO;
-		if (lock_rename(c->workdir, c->destdir) != NULL) {
-			pr_err("overlayfs: failed to lock workdir+upperdir\n");
-		} else {
+		err = ovl_lock_rename_workdir(c->workdir, c->destdir);
+		if (!err) {
 			err = ovl_copy_up_locked(c);
 			unlock_rename(c->workdir, c->destdir);
 		}
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 3309b1912241..cc961a3bd3bd 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -216,26 +216,6 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
 	return err;
 }
 
-static int ovl_lock_rename_workdir(struct dentry *workdir,
-				   struct dentry *upperdir)
-{
-	/* Workdir should not be the same as upperdir */
-	if (workdir == upperdir)
-		goto err;
-
-	/* Workdir should not be subdir of upperdir and vice versa */
-	if (lock_rename(workdir, upperdir) != NULL)
-		goto err_unlock;
-
-	return 0;
-
-err_unlock:
-	unlock_rename(workdir, upperdir);
-err:
-	pr_err("overlayfs: failed to lock workdir+upperdir\n");
-	return -EIO;
-}
-
 static struct dentry *ovl_clear_empty(struct dentry *dentry,
 				      struct list_head *list)
 {
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index d4e8c1a08fb0..c706a6f99928 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -235,6 +235,7 @@ bool ovl_inuse_trylock(struct dentry *dentry);
 void ovl_inuse_unlock(struct dentry *dentry);
 int ovl_nlink_start(struct dentry *dentry, bool *locked);
 void ovl_nlink_end(struct dentry *dentry, bool locked);
+int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir);
 
 static inline bool ovl_is_impuredir(struct dentry *dentry)
 {
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index a1bf81d352b4..904c60b4e759 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -560,3 +560,22 @@ void ovl_nlink_end(struct dentry *dentry, bool locked)
 		mutex_unlock(&OVL_I(d_inode(dentry))->lock);
 	}
 }
+
+int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir)
+{
+	/* Workdir should not be the same as upperdir */
+	if (workdir == upperdir)
+		goto err;
+
+	/* Workdir should not be subdir of upperdir and vice versa */
+	if (lock_rename(workdir, upperdir) != NULL)
+		goto err_unlock;
+
+	return 0;
+
+err_unlock:
+	unlock_rename(workdir, upperdir);
+err:
+	pr_err("overlayfs: failed to lock workdir+upperdir\n");
+	return -EIO;
+}
-- 
2.7.4

Re: [PATCH 0/4] assorted overlayfs fixes for v4.14-rc3

[Amir Goldstein]

On Mon, Sep 25, 2017 at 12:36 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> Miklos,
>
> Following are 1 fix for a commit in v4.14-rc1
> and 3 fixes for index error paths in v4.13.

Make that 4 fixes to v4.13..
Available at https://github.com/amir73il/linux/commits/ovl-fixes

>
> Amir.
>
> Amir Goldstein (5):
>   ovl: fix may_write_real() for overlayfs directories
>   ovl: fix error value printed in ovl_lookup_index()
>   ovl: fix dput() of ERR_PTR in ovl_cleanup_index()
>   ovl: fix dentry leak in ovl_indexdir_cleanup()

+   ovl: fix missing unlock_rename() in ovl_do_copy_up()

>
>  fs/namespace.c         | 4 +++-
>  fs/overlayfs/namei.c   | 1 +
>  fs/overlayfs/readdir.c | 6 ++++--
>  fs/overlayfs/util.c    | 9 ++++++---
>  4 files changed, 14 insertions(+), 6 deletions(-)
>
> --
> 2.7.4
>