All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] overlayfs forward compat index changes
@ 2017-07-18 18:07 Amir Goldstein
  2017-07-18 18:07 ` [PATCH v2 1/2] ovl: do not cleanup directory and whiteout index entries Amir Goldstein
  2017-07-18 18:07 ` [PATCH v2 2/2] ovl: check for bad and whiteout index on lookup Amir Goldstein
  0 siblings, 2 replies; 3+ messages in thread
From: Amir Goldstein @ 2017-07-18 18:07 UTC (permalink / raw
  To: Miklos Szeredi; +Cc: linux-unionfs

Miklos,

I did not get any NACK nor ACK from you on the patch:
[08/10] ovl: do not cleanup directory and whiteout index entries
from the v4.13 assorted fixes series.

So I've NACKed it myself, because we should not be allowing
rw mount with index entries from the future.

So patch 1 is resend of that patch which returns EROFS from
ovl_indexdir_cleanup() if entries from the future are found.
I'll leave it to you to decide if this error translates to
mount failure or fallback to read-only mount.

Patch 2 takes care of another forward compact issue related
index entries from the future in ovl_lookup_index(), but it
is really a sanity check that we should have had there (we
forgot to check that found index is of same file type as lower).

FYI, I have a WIP for whiteout index implementation at [1]
with these patched:
  ovl: verify directory and whiteout index entries
  ovl: whiteout index on last unlink

Thanks,
Amir.

[1] https://github.com/amir73il/linux/commits/ovl-index-wip

Amir Goldstein (2):
  ovl: do not cleanup directory and whiteout index entries
  ovl: check for bad and whiteout index on lookup

 fs/overlayfs/namei.c   | 41 ++++++++++++++++++++++++++++++++---------
 fs/overlayfs/readdir.c |  5 ++++-
 2 files changed, 36 insertions(+), 10 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v2 1/2] ovl: do not cleanup directory and whiteout index entries
  2017-07-18 18:07 [PATCH v2 0/2] overlayfs forward compat index changes Amir Goldstein
@ 2017-07-18 18:07 ` Amir Goldstein
  2017-07-18 18:07 ` [PATCH v2 2/2] ovl: check for bad and whiteout index on lookup Amir Goldstein
  1 sibling, 0 replies; 3+ messages in thread
From: Amir Goldstein @ 2017-07-18 18:07 UTC (permalink / raw
  To: Miklos Szeredi; +Cc: linux-unionfs

Directory index entries are going to be used for looking up
redirected upper dirs by lower dir fh when decoding an overlay
file handle of a merge dir.

Whiteout index entries are going to be used as an indication that
an exported overlay file handle should be treated as stale (i.e.
after unlink of the overlay inode).

We don't know the verification rules for directory and whiteout
index entries, because they have not been implemented yet, so fail
to mount overlay rw if those entries are found to avoid corrupting
an index that was created by a newer kernel.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/namei.c   | 19 +++++++++++++++----
 fs/overlayfs/readdir.c |  5 ++++-
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 9bc0e580a5b3..229a88ff335c 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -397,8 +397,19 @@ int ovl_verify_index(struct dentry *index, struct path *lowerstack,
 	if (!d_inode(index))
 		return 0;
 
-	err = -EISDIR;
-	if (d_is_dir(index))
+	/*
+	 * Directory index entries are going to be used for looking up
+	 * redirected upper dirs by lower dir fh when decoding an overlay
+	 * file handle of a merge dir. Whiteout index entries are going to be
+	 * used as an indication that an exported overlay file handle should
+	 * be treated as stale (i.e. after unlink of the overlay inode).
+	 * We don't know the verification rules for directory and whiteout
+	 * index entries, because they have not been implemented yet, so return
+	 * EROFS if those entries are found to avoid corrupting an index that
+	 * was created by a newer kernel.
+	 */
+	err = -EROFS;
+	if (d_is_dir(index) || ovl_is_whiteout(index))
 		goto fail;
 
 	err = -EINVAL;
@@ -436,8 +447,8 @@ int ovl_verify_index(struct dentry *index, struct path *lowerstack,
 	return err;
 
 fail:
-	pr_warn_ratelimited("overlayfs: failed to verify index (%pd2, err=%i)\n",
-			    index, err);
+	pr_warn_ratelimited("overlayfs: failed to verify index (%pd2, ftype=%x, err=%i)\n",
+			    index, d_inode(index)->i_mode & S_IFMT, err);
 	goto out;
 }
 
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 0298463cf9c3..3d424a51cabb 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -703,7 +703,10 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
 			err = PTR_ERR(index);
 			break;
 		}
-		if (ovl_verify_index(index, lowerstack, numlower)) {
+		err = ovl_verify_index(index, lowerstack, numlower);
+		if (err) {
+			if (err == -EROFS)
+				break;
 			err = ovl_cleanup(dir, index);
 			if (err)
 				break;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH v2 2/2] ovl: check for bad and whiteout index on lookup
  2017-07-18 18:07 [PATCH v2 0/2] overlayfs forward compat index changes Amir Goldstein
  2017-07-18 18:07 ` [PATCH v2 1/2] ovl: do not cleanup directory and whiteout index entries Amir Goldstein
@ 2017-07-18 18:07 ` Amir Goldstein
  1 sibling, 0 replies; 3+ messages in thread
From: Amir Goldstein @ 2017-07-18 18:07 UTC (permalink / raw
  To: Miklos Szeredi; +Cc: linux-unionfs

Index should always be of the same file type as origin, except for
the case of a whiteout index.  A whiteout index should only exist
if all lower aliases have been unlinked, which means that finding
a lower origin on lookup whose index is a whiteout should be treated
as a lookup error.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/namei.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 229a88ff335c..8aef2b304b2d 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -513,6 +513,7 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,
 		goto out;
 	}
 
+	inode = d_inode(index);
 	if (d_is_negative(index)) {
 		if (upper && d_inode(origin)->i_nlink > 1) {
 			pr_warn_ratelimited("overlayfs: hard link with origin but no index (ino=%lu).\n",
@@ -522,11 +523,22 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,
 
 		dput(index);
 		index = NULL;
-	} else if (upper && d_inode(index) != d_inode(upper)) {
-		inode = d_inode(index);
-		pr_warn_ratelimited("overlayfs: wrong index found (index ino: %lu, upper ino: %lu).\n",
-				    d_inode(index)->i_ino,
-				    d_inode(upper)->i_ino);
+	} else if (upper && d_inode(upper) != inode) {
+		pr_warn_ratelimited("overlayfs: wrong index found (index=%pd2, ino=%lu, upper ino=%lu).\n",
+				    index, inode->i_ino, d_inode(upper)->i_ino);
+		goto fail;
+	} else if (ovl_dentry_weird(index) || ovl_is_whiteout(index) ||
+		   ((inode->i_mode ^ d_inode(origin)->i_mode) & S_IFMT)) {
+		/*
+		 * Index should always be of the same file type as origin
+		 * except for the case of a whiteout index. A whiteout
+		 * index should only exist if all lower aliases have been
+		 * unlinked, which means that finding a lower origin on lookup
+		 * whose index is a whiteout should be treated as an error.
+		 */
+		pr_warn_ratelimited("overlayfs: bad index found (index=%pd2, ftype=%x, origin ftype=%x).\n",
+				    index, d_inode(index)->i_mode & S_IFMT,
+				    d_inode(origin)->i_mode & S_IFMT);
 		goto fail;
 	}
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-07-18 18:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-18 18:07 [PATCH v2 0/2] overlayfs forward compat index changes Amir Goldstein
2017-07-18 18:07 ` [PATCH v2 1/2] ovl: do not cleanup directory and whiteout index entries Amir Goldstein
2017-07-18 18:07 ` [PATCH v2 2/2] ovl: check for bad and whiteout index on lookup Amir Goldstein

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.