All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] Implement overlayfs index=all mount option
@ 2017-10-17 16:00 Amir Goldstein
  2017-10-17 16:00 ` [PATCH 01/11] ovl: fix EIO from lookup of non-indexed upper Amir Goldstein
                   ` (10 more replies)
  0 siblings, 11 replies; 13+ messages in thread
From: Amir Goldstein @ 2017-10-17 16:00 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

Miklos,

This is the second of two prep patch sets [1][2] for NFS export [3].
NFS export is enabled with the 'verify_dir' and 'index=all'
mount options.

The requirement for 'index=all' can be relaxed if we create index
for redirected dirs on lookup and for renamed non-dirs when encoding
file handles. Decoding file handle will be more complex without
index=all, so per your suggestion, this was not implemented for
first version of NFS export.

The first two patches in this series are tagged for stable v4.13.
The first patch I already posted, relaxes some index lookup constrains
that we can do without.
The second patch allows kernel v4.13 to mount an overlay with whiteout
index entries, which are being introduced by this patch set.

To sanity test index=all, I have implemented _overlay_check_fs()
helper for xfstests [4], which is a very basic "fsck" of the index dir.
This helper is called at the end of each overlay xfstest to verify the
sanity of the index entries.

Amir.

[1] https://github.com/amir73il/linux/commits/ovl-verify-dir
[2] https://github.com/amir73il/linux/commits/ovl-index-all
[3] https://github.com/amir73il/linux/commits/ovl-nfs-export
[4] https://github.com/amir73il/xfstests/commits/ovl-index-all

Amir Goldstein (11):
  ovl: fix EIO from lookup of non-indexed upper
  ovl: verify whiteout index entries on mount
  ovl: create ovl_need_index() helper
  ovl: add support for mount option index=all
  ovl: lookup index for directories
  ovl: verify directory index entries on mount
  ovl: index directories on copy up
  ovl: cleanup dir index when dir nlink drops to zero
  ovl: whiteout index when union nlink drops to zero
  ovl: whiteout orphan index entries on mount
  ovl: cleanup stale whiteout index entries on mount

 fs/overlayfs/copy_up.c   | 130 +++++++++++++++++---
 fs/overlayfs/dir.c       |  58 +++++----
 fs/overlayfs/inode.c     |  20 ++-
 fs/overlayfs/namei.c     | 314 +++++++++++++++++++++++++++++++++--------------
 fs/overlayfs/overlayfs.h |  29 ++++-
 fs/overlayfs/ovl_entry.h |   2 +-
 fs/overlayfs/readdir.c   |  48 ++++++--
 fs/overlayfs/super.c     |  28 +++--
 fs/overlayfs/util.c      |  55 ++++++---
 9 files changed, 505 insertions(+), 179 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread
* [PATCH 01/11] ovl: fix EIO from lookup of non-indexed upper
  2017-10-17 16:00 [PATCH 00/11] Implement overlayfs index=all mount option Amir Goldstein
@ 2017-10-17 16:00 ` Amir Goldstein
  2017-10-17 16:00 ` [PATCH 02/11] ovl: verify whiteout index entries on mount Amir Goldstein
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2017-10-17 16:00 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs, # v4 . 13

Commit fbaf94ee3cd5 ("ovl: don't set origin on broken lower hardlink")
attempt to avoid the condition of non-indexed upper inode with lower
hardlink as origin. If this condition is found, lookup returns EIO.

The protection of commit mentioned above does not cover the case of lower
that is not a hardlink when it is copied up (with either index=off/on)
and then lower is hardlinked while overlay is offline.

Changes to lower layer while overlayfs is offline should not result in
unexpected behavior, so a permanent EIO error after creating a link in
lower layer should not be considered as correct behavior.

This fix replaces EIO error with a warning in cases where upper has
origin but no index is found, or index is found that does not match upper
inode. In those cases, lookup will not fail and the returned overlay
inode will be hashed by upper inode instead of by lower origin inode.

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/inode.c     | 20 ++++++++++++++++----
 fs/overlayfs/namei.c     | 26 ++++++++++++++++----------
 fs/overlayfs/overlayfs.h |  3 ++-
 3 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index a619addecafc..321511ed8c42 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -598,18 +598,30 @@ static bool ovl_verify_inode(struct inode *inode, struct dentry *lowerdentry,
 	return true;
 }
 
-struct inode *ovl_get_inode(struct dentry *dentry, struct dentry *upperdentry)
+struct inode *ovl_get_inode(struct dentry *dentry, struct dentry *upperdentry,
+			    struct dentry *index)
 {
 	struct dentry *lowerdentry = ovl_dentry_lower(dentry);
 	struct inode *realinode = upperdentry ? d_inode(upperdentry) : NULL;
 	struct inode *inode;
+	/* Already indexed or could be indexed on copy up? */
+	bool indexed = (index || (ovl_indexdir(dentry->d_sb) && !upperdentry));
+
+	if (WARN_ON(upperdentry && indexed && !lowerdentry))
+		return ERR_PTR(-EIO);
 
 	if (!realinode)
 		realinode = d_inode(lowerdentry);
 
-	if (!S_ISDIR(realinode->i_mode) &&
-	    (upperdentry || (lowerdentry && ovl_indexdir(dentry->d_sb)))) {
-		struct inode *key = d_inode(lowerdentry ?: upperdentry);
+	/*
+	 * Copy up origin (lower) may exist for non-indexed upper, but we must
+	 * not use lower as hash key in that case.
+	 * Hash inodes that are or could be indexed by origin inode and
+	 * non-indexed upper inodes that could be hard linked by upper inode.
+	 */
+	if (!S_ISDIR(realinode->i_mode) && (upperdentry || indexed)) {
+		struct inode *key = d_inode(indexed ? lowerdentry :
+						      upperdentry);
 		unsigned int nlink;
 
 		inode = iget5_locked(dentry->d_sb, (unsigned long) key,
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 5de7aa60a7a6..6a8e0156d40d 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -519,6 +519,10 @@ 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);
+		if (err == -ENOENT) {
+			index = NULL;
+			goto out;
+		}
 		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,
@@ -529,17 +533,14 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,
 	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",
-					    d_inode(origin)->i_ino);
-			goto fail;
+			pr_warn_ratelimited("overlayfs: hard link with origin but no index (%pd2).\n",
+					    upper);
 		}
-
-		dput(index);
-		index = NULL;
+		goto out_dput;
 	} 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;
+		pr_warn_ratelimited("overlayfs: broken hard link - ignoring index (%pd2).\n",
+				    upper);
+		goto out_dput;
 	} else if (ovl_dentry_weird(index) || ovl_is_whiteout(index) ||
 		   ((inode->i_mode ^ d_inode(origin)->i_mode) & S_IFMT)) {
 		/*
@@ -559,6 +560,11 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,
 	kfree(name.name);
 	return index;
 
+out_dput:
+	dput(index);
+	index = NULL;
+	goto out;
+
 fail:
 	dput(index);
 	index = ERR_PTR(-EIO);
@@ -755,7 +761,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		upperdentry = dget(index);
 
 	if (upperdentry || ctr) {
-		inode = ovl_get_inode(dentry, upperdentry);
+		inode = ovl_get_inode(dentry, upperdentry, index);
 		err = PTR_ERR(inode);
 		if (IS_ERR(inode))
 			goto out_free_oe;
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index ba455642152f..df605368700d 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -288,7 +288,8 @@ int ovl_update_time(struct inode *inode, struct timespec *ts, int flags);
 bool ovl_is_private_xattr(const char *name);
 
 struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, dev_t rdev);
-struct inode *ovl_get_inode(struct dentry *dentry, struct dentry *upperdentry);
+struct inode *ovl_get_inode(struct dentry *dentry, struct dentry *upperdentry,
+			    struct dentry *index);
 static inline void ovl_copyattr(struct inode *from, struct inode *to)
 {
 	to->i_uid = from->i_uid;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 02/11] ovl: verify whiteout index entries on mount
  2017-10-17 16:00 [PATCH 00/11] Implement overlayfs index=all mount option Amir Goldstein
  2017-10-17 16:00 ` [PATCH 01/11] ovl: fix EIO from lookup of non-indexed upper Amir Goldstein
@ 2017-10-17 16:00 ` Amir Goldstein
  2017-10-17 16:00 ` [PATCH 03/11] ovl: create ovl_need_index() helper Amir Goldstein
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2017-10-17 16:00 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs, # v4 . 13

Whiteout index entries are used as an indication that
an exported overlay file handle should be treated as stale
(i.e. after unlink/rmdir of the overlay inode).
These entries contain no xattr.

Currently we only verify that whiteout index have a sane name,
but we do not try to decode the index name as file handle to
verify it is not stale.

This allows mounting overlayfs that has whiteout index entries
generated by newer kernel.

Fixes: 61b674710cd9 ("ovl: do not cleanup directory and whiteout index")
Cc: <stable@vger.kernel.org> # v4.13
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/namei.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 6a8e0156d40d..c73f490695c6 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -412,16 +412,13 @@ int ovl_verify_index(struct dentry *index, struct path *lowerstack,
 	/*
 	 * 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.
+	 * file handle of a merge dir.  We don't know the verification rules
+	 * for directory 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))
+	if (d_is_dir(index))
 		goto fail;
 
 	err = -EINVAL;
@@ -438,6 +435,17 @@ int ovl_verify_index(struct dentry *index, struct path *lowerstack,
 	if (hex2bin((u8 *)fh, index->d_name.name, len) || len != fh->len)
 		goto fail;
 
+	/*
+	 * Whiteout index entries are used as an indication that
+	 * an exported overlay file handle should be treated as stale
+	 * (i.e. after unlink/rmdir of the overlay inode).
+	 * These entries contain no xattr.
+	 * TODO: cleanup stale whiteout index (try to decode index name).
+	 */
+	err = 0;
+	if (ovl_is_whiteout(index))
+		goto out;
+
 	err = ovl_verify_origin_fh(index, fh);
 	if (err)
 		goto fail;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 03/11] ovl: create ovl_need_index() helper
  2017-10-17 16:00 [PATCH 00/11] Implement overlayfs index=all mount option Amir Goldstein
  2017-10-17 16:00 ` [PATCH 01/11] ovl: fix EIO from lookup of non-indexed upper Amir Goldstein
  2017-10-17 16:00 ` [PATCH 02/11] ovl: verify whiteout index entries on mount Amir Goldstein
@ 2017-10-17 16:00 ` Amir Goldstein
  2017-10-17 16:00 ` [PATCH 04/11] ovl: add support for mount option index=all Amir Goldstein
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2017-10-17 16:00 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

The helper determines which lower file needs to be indexed
on copy up and before nlink changes.

For index=on, the helper evaluates to true for lower hardlinks.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/copy_up.c   |  6 +-----
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/util.c      | 25 +++++++++++++++++++++----
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index c441f9387a1b..6729ea2a5a1c 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -532,11 +532,7 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 {
 	int err;
 	struct ovl_fs *ofs = c->dentry->d_sb->s_fs_info;
-	bool indexed = false;
-
-	if (ovl_indexdir(c->dentry->d_sb) && !S_ISDIR(c->stat.mode) &&
-	    c->stat.nlink > 1)
-		indexed = true;
+	bool indexed = ovl_need_index(c->dentry);
 
 	if (S_ISDIR(c->stat.mode) || c->stat.nlink == 1 || indexed)
 		c->origin = true;
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index df605368700d..17457483ccf6 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -235,6 +235,7 @@ void ovl_clear_flag(unsigned long flag, struct inode *inode);
 bool ovl_test_flag(unsigned long flag, struct inode *inode);
 bool ovl_inuse_trylock(struct dentry *dentry);
 void ovl_inuse_unlock(struct dentry *dentry);
+bool ovl_need_index(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);
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 7355c51ae05f..6b3af32ab0e9 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -506,6 +506,24 @@ static void ovl_cleanup_index(struct dentry *dentry)
 }
 
 /*
+ * Does this overlay dentry need to be indexed on copy up?
+ */
+bool ovl_need_index(struct dentry *dentry)
+{
+	struct dentry *lower = ovl_dentry_lower(dentry);
+
+	if (!lower)
+		return false;
+
+	/* Index only lower hardlinks on copy up */
+	if (ovl_indexdir(dentry->d_sb) &&
+	    !d_is_dir(lower) && d_inode(lower)->i_nlink > 1)
+		return true;
+
+	return false;
+}
+
+/*
  * Operations that change overlay inode and upper inode nlink need to be
  * synchronized with copy up for persistent nlink accounting.
  */
@@ -520,11 +538,11 @@ int ovl_nlink_start(struct dentry *dentry, bool *locked)
 
 	/*
 	 * With inodes index is enabled, we store the union overlay nlink
-	 * in an xattr on the index inode. When whiting out lower hardlinks
+	 * in an xattr on the index inode. When whiting out an indexed lower,
 	 * we need to decrement the overlay persistent nlink, but before the
 	 * first copy up, we have no upper index inode to store the xattr.
 	 *
-	 * As a workaround, before whiteout/rename over of a lower hardlink,
+	 * As a workaround, before whiteout/rename over an indexed lower,
 	 * copy up to create the upper index. Creating the upper index will
 	 * initialize the overlay nlink, so it could be dropped if unlink
 	 * or rename succeeds.
@@ -532,8 +550,7 @@ int ovl_nlink_start(struct dentry *dentry, bool *locked)
 	 * TODO: implement metadata only index copy up when called with
 	 *       ovl_copy_up_flags(dentry, O_PATH).
 	 */
-	if (ovl_indexdir(dentry->d_sb) && !ovl_dentry_has_upper_alias(dentry) &&
-	    d_inode(ovl_dentry_lower(dentry))->i_nlink > 1) {
+	if (ovl_need_index(dentry) && !ovl_dentry_has_upper_alias(dentry)) {
 		err = ovl_copy_up(dentry);
 		if (err)
 			return err;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 04/11] ovl: add support for mount option index=all
  2017-10-17 16:00 [PATCH 00/11] Implement overlayfs index=all mount option Amir Goldstein
                   ` (2 preceding siblings ...)
  2017-10-17 16:00 ` [PATCH 03/11] ovl: create ovl_need_index() helper Amir Goldstein
@ 2017-10-17 16:00 ` Amir Goldstein
  2017-10-17 16:00 ` [PATCH 05/11] ovl: lookup index for directories Amir Goldstein
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2017-10-17 16:00 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

With mount option index=all, all files are indexed on copy up.

The default boolean module/config var can only be used to configure
indexing of lower hardlinks (true) or to turn off indexing (false).

This is going to be used for overlayfs snapshots and NFS export.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/overlayfs.h | 14 ++++++++++++++
 fs/overlayfs/ovl_entry.h |  2 +-
 fs/overlayfs/super.c     | 23 +++++++++++++++--------
 fs/overlayfs/util.c      |  8 ++++++--
 4 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 17457483ccf6..35f5452e61e0 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -33,6 +33,20 @@ enum ovl_flag {
 };
 
 /*
+ * Which files should be indexed on copy up.
+ * The default is casted from the boolean module config var, which can
+ * only be used to configure indexing of lower hardlinks or to turn off
+ * indexing. Other values can only be configures via mount option
+ * (e.g. index=all).
+ */
+enum ovl_index {
+	OVL_INDEX_OFF = (int)false,
+	OVL_INDEX_ON = (int)true,
+	OVL_INDEX_NLINK = OVL_INDEX_ON,
+	OVL_INDEX_ALL,
+};
+
+/*
  * The tuple (fh,uuid) is a universal unique identifier for a copy up origin,
  * where:
  * origin.fh	- exported file handle of the lower file
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 4c3d5ff1a30c..45dc8077d959 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -15,7 +15,7 @@ struct ovl_config {
 	bool default_permissions;
 	bool redirect_dir;
 	bool verify_dir;
-	bool index;
+	int index;
 };
 
 /* private information held for overlayfs's superblock */
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index aa230efc1afe..24fac7d987a3 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -302,7 +302,8 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 			   ufs->config.redirect_dir ? "on" : "off");
 	if (ufs->config.index != ovl_index_def)
 		seq_printf(m, ",index=%s",
-			   ufs->config.index ? "on" : "off");
+			   ufs->config.index == OVL_INDEX_ALL ? "all" :
+			   (ufs->config.index ? "on" : "off"));
 	if (ufs->config.verify_dir)
 		seq_puts(m, ",verify_dir");
 	return 0;
@@ -338,6 +339,7 @@ enum {
 	OPT_REDIRECT_DIR_OFF,
 	OPT_INDEX_ON,
 	OPT_INDEX_OFF,
+	OPT_INDEX_ALL,
 	OPT_VERIFY_DIR,
 	OPT_ERR,
 };
@@ -351,6 +353,7 @@ static const match_table_t ovl_tokens = {
 	{OPT_REDIRECT_DIR_OFF,		"redirect_dir=off"},
 	{OPT_INDEX_ON,			"index=on"},
 	{OPT_INDEX_OFF,			"index=off"},
+	{OPT_INDEX_ALL,			"index=all"},
 	{OPT_VERIFY_DIR,		"verify_dir"},
 	{OPT_ERR,			NULL}
 };
@@ -425,11 +428,15 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 			break;
 
 		case OPT_INDEX_ON:
-			config->index = true;
+			config->index = OVL_INDEX_NLINK;
 			break;
 
 		case OPT_INDEX_OFF:
-			config->index = false;
+			config->index = 0;
+			break;
+
+		case OPT_INDEX_ALL:
+			config->index = OVL_INDEX_ALL;
 			break;
 
 		case OPT_VERIFY_DIR:
@@ -664,7 +671,7 @@ static int ovl_lower_dir(const char *name, struct path *path,
 	 */
 	if (ofs->config.upperdir && !ovl_can_decode_fh(path->dentry->d_sb) &&
 	    (ofs->config.index || ofs->config.verify_dir)) {
-		ofs->config.index = false;
+		ofs->config.index = 0;
 		ofs->config.verify_dir = false;
 		pr_warn("overlayfs: fs on '%s' does not support file handles, falling back to index=off and no verify_dir.\n",
 			name);
@@ -867,7 +874,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		goto out;
 
 	ufs->config.redirect_dir = ovl_redirect_dir_def;
-	ufs->config.index = ovl_index_def;
+	ufs->config.index = (enum ovl_index) ovl_index_def;
 	err = ovl_parse_opt((char *) data, &ufs->config);
 	if (err)
 		goto out_free_config;
@@ -1031,7 +1038,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 			if (err) {
 				ufs->noxattr = true;
 				ufs->config.redirect_dir = false;
-				ufs->config.index = false;
+				ufs->config.index = 0;
 				ufs->config.verify_dir = false;
 				pr_warn("overlayfs: upper fs does not support xattr, falling back to redirect_dir=off, index=off, no verify_dir and no opaque dir.\n");
 			} else {
@@ -1041,7 +1048,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 			/* Check if upper/work fs supports file handles */
 			if (ufs->config.index &&
 			    !ovl_can_decode_fh(ufs->workdir->d_sb)) {
-				ufs->config.index = false;
+				ufs->config.index = 0;
 				pr_warn("overlayfs: upper fs does not support file handles, falling back to index=off.\n");
 			}
 		}
@@ -1105,7 +1112,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 
 	/* Show index=off/on in /proc/mounts for any of the reasons above */
 	if (!ufs->indexdir)
-		ufs->config.index = false;
+		ufs->config.index = 0;
 
 	if (ufs->config.verify_dir || ufs->indexdir) {
 		/* Verify lower root is upper root origin */
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 6b3af32ab0e9..fb501eb5e75c 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -510,13 +510,17 @@ static void ovl_cleanup_index(struct dentry *dentry)
  */
 bool ovl_need_index(struct dentry *dentry)
 {
+	struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
 	struct dentry *lower = ovl_dentry_lower(dentry);
 
-	if (!lower)
+	if (!lower || !ofs->indexdir)
 		return false;
 
+	if (!d_is_dir(lower) && ofs->config.index == OVL_INDEX_ALL)
+		return true;
+
 	/* Index only lower hardlinks on copy up */
-	if (ovl_indexdir(dentry->d_sb) &&
+	if ((ofs->config.index == OVL_INDEX_NLINK) &&
 	    !d_is_dir(lower) && d_inode(lower)->i_nlink > 1)
 		return true;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 05/11] ovl: lookup index for directories
  2017-10-17 16:00 [PATCH 00/11] Implement overlayfs index=all mount option Amir Goldstein
                   ` (3 preceding siblings ...)
  2017-10-17 16:00 ` [PATCH 04/11] ovl: add support for mount option index=all Amir Goldstein
@ 2017-10-17 16:00 ` Amir Goldstein
  2017-10-17 16:00 ` [PATCH 06/11] ovl: verify directory index entries on mount Amir Goldstein
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2017-10-17 16:00 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

A directory index is a directory type entry in index dir
with a "trusted.overlay.origin" xattr containing an encoded ovl_fh
of the merge directory upper dir.

On lookup of non-dir files, lower file is followed by origin file handle.
On lookup of dir entries, lower dir is found by name and then compared
to origin file handle. We only trust dir index if we verified that lower
dir matches origin file handle, otherwise index is inconsistent and we
ignore it.

Directory index entries are going to be used for NFS export.

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

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index c73f490695c6..1afc6d6e99b9 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -518,6 +518,7 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,
 	struct dentry *index;
 	struct inode *inode;
 	struct qstr name;
+	bool is_dir = d_is_dir(origin);
 	int err;
 
 	err = ovl_get_index_name(origin, &name);
@@ -540,15 +541,11 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,
 
 	inode = d_inode(index);
 	if (d_is_negative(index)) {
-		if (upper && d_inode(origin)->i_nlink > 1) {
+		if (!is_dir && upper && d_inode(origin)->i_nlink > 1) {
 			pr_warn_ratelimited("overlayfs: hard link with origin but no index (%pd2).\n",
 					    upper);
 		}
 		goto out_dput;
-	} else if (upper && d_inode(upper) != inode) {
-		pr_warn_ratelimited("overlayfs: broken hard link - ignoring index (%pd2).\n",
-				    upper);
-		goto out_dput;
 	} else if (ovl_dentry_weird(index) || ovl_is_whiteout(index) ||
 		   ((inode->i_mode ^ d_inode(origin)->i_mode) & S_IFMT)) {
 		/*
@@ -562,8 +559,16 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,
 				    index, d_inode(index)->i_mode & S_IFMT,
 				    d_inode(origin)->i_mode & S_IFMT);
 		goto fail;
+	} else if (upper && is_dir) {
+		/* Verify that dir index origin points to upper dir */
+		err = ovl_verify_origin(index, upper, true, false);
+		if (err)
+			goto fail;
+	} else if (upper && d_inode(upper) != inode) {
+		pr_warn_ratelimited("overlayfs: broken hard link - ignoring index (%pd2).\n",
+				    upper);
+		goto out_dput;
 	}
-
 out:
 	kfree(name.name);
 	return index;
@@ -613,6 +618,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 	struct ovl_entry *roe = dentry->d_sb->s_root->d_fsdata;
 	struct path *stack = NULL;
 	struct dentry *upperdir, *upperdentry = NULL;
+	struct dentry *origin = NULL;
 	struct dentry *index = NULL;
 	unsigned int ctr = 0;
 	struct inode *inode = NULL;
@@ -659,6 +665,8 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			 */
 			err = ovl_check_origin(upperdentry, roe->lowerstack,
 					       roe->numlower, &stack, &ctr);
+			if (ctr)
+				origin = stack[0].dentry;
 			if (err)
 				goto out;
 		}
@@ -692,15 +700,26 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		if (!this)
 			continue;
 
+		if (!ctr)
+			origin = this;
+
 		/*
 		 * Verify that uppermost lower matches the copy up origin fh.
 		 * If no origin fh is stored in upper, store fh of lower dir.
+		 * If origin fh exists and does not match lower dir, depending
+		 * on verify_dir config, we either not merge this lower or we
+		 * merge it, but we do not use it as index key.
 		 */
 		if (this && upperdentry && !ctr) {
 			err = ovl_verify_origin(upperdentry, this, false, true);
-			if (err && ovl_verify_dir(dentry->d_sb)) {
-				dput(this);
-				break;
+			if (err) {
+				if (ovl_verify_dir(dentry->d_sb)) {
+					/* Don't merge with unverified lower */
+					dput(this);
+					break;
+				}
+				/* Don't use unverified lower as index key */
+				origin = NULL;
 			}
 		}
 
@@ -733,6 +752,8 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 				       roe->numlower, &stack, &ctr);
 		if (err)
 			goto out_put;
+
+		origin = stack[0].dentry;
 		/*
 		 * XXX: We do not continue layers lookup from decoded origin for
 		 * more than a single lower layer. This would require setting
@@ -742,10 +763,15 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		 */
 	}
 
-	/* Lookup index by lower inode and verify it matches upper inode */
-	if (ctr && !d.is_dir && ovl_indexdir(dentry->d_sb)) {
-		struct dentry *origin = stack[0].dentry;
-
+	/*
+	 * Lookup index by lower inode and verify it matches upper inode.
+	 * We only trust dir index if we verified that lower dir matches
+	 * origin, otherwise index is inconsistent and we ignore it.
+	 *
+	 * TODO: update origin and index in case lower dir has changed and
+	 *       store new generation number xattr in index for NFS export.
+	 */
+	if (ctr && ovl_indexdir(dentry->d_sb) && origin) {
 		index = ovl_lookup_index(dentry, upperdentry, origin);
 		if (IS_ERR(index)) {
 			err = PTR_ERR(index);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 06/11] ovl: verify directory index entries on mount
  2017-10-17 16:00 [PATCH 00/11] Implement overlayfs index=all mount option Amir Goldstein
                   ` (4 preceding siblings ...)
  2017-10-17 16:00 ` [PATCH 05/11] ovl: lookup index for directories Amir Goldstein
@ 2017-10-17 16:00 ` Amir Goldstein
  2017-10-19 10:35   ` Amir Goldstein
  2017-10-17 16:00 ` [PATCH 07/11] ovl: index directories on copy up Amir Goldstein
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 13+ messages in thread
From: Amir Goldstein @ 2017-10-17 16:00 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

Directory index entries should have origin xattr pointing to the
real upper dir. Non-dir index entries are hardlinks to the upper
real inode. For non-dir index, we can read the copy up origin xattr
directly from the index dentry, but for dir index we first need to
decode the upper directory.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/namei.c     | 60 +++++++++++++++++++++++++++++++++---------------
 fs/overlayfs/overlayfs.h |  4 ++--
 fs/overlayfs/readdir.c   |  2 +-
 3 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 1afc6d6e99b9..aba57d31d850 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -391,36 +391,45 @@ int ovl_verify_origin(struct dentry *dentry, struct dentry *origin,
 	goto out;
 }
 
+/* Get upper dentry from index */
+struct dentry *ovl_index_upper(struct dentry *index, struct vfsmount *mnt)
+{
+	struct path upperpath = { .mnt = mnt };
+	struct path *stack = &upperpath;
+	unsigned int ctr = 0;
+	int err;
+
+	if (!d_is_dir(index))
+		return dget(index);
+
+	err = ovl_check_origin(index, stack, 1, &stack, &ctr);
+	if (!err && !ctr)
+		err = -ENODATA;
+	if (err)
+		return ERR_PTR(err);
+
+	return upperpath.dentry;
+}
+
 /*
  * Verify that an index entry name matches the origin file handle stored in
  * OVL_XATTR_ORIGIN and that origin file handle can be decoded to lower path.
  * Return 0 on match, -ESTALE on mismatch or stale origin, < 0 on error.
  */
-int ovl_verify_index(struct dentry *index, struct path *lowerstack,
-		     unsigned int numlower)
+int ovl_verify_index(struct dentry *index, struct vfsmount *mnt,
+		     struct path *lowerstack, unsigned int numlower)
 {
 	struct ovl_fh *fh = NULL;
 	size_t len;
 	struct path origin = { };
 	struct path *stack = &origin;
 	unsigned int ctr = 0;
+	struct dentry *upper = NULL;
 	int err;
 
 	if (!d_inode(index))
 		return 0;
 
-	/*
-	 * 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.  We don't know the verification rules
-	 * for directory 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))
-		goto fail;
-
 	err = -EINVAL;
 	if (index->d_name.len < sizeof(struct ovl_fh)*2)
 		goto fail;
@@ -446,23 +455,38 @@ int ovl_verify_index(struct dentry *index, struct path *lowerstack,
 	if (ovl_is_whiteout(index))
 		goto out;
 
-	err = ovl_verify_origin_fh(index, fh);
+	/*
+	 * Directory index entries should have origin xattr pointing to the
+	 * real upper dir. Non-dir index entries are hardlinks to the upper
+	 * real inode. For non-dir index, we can read the copy up origin xattr
+	 * directly from the index dentry, but for dir index we first need to
+	 * decode the upper directory.
+	 */
+	upper = ovl_index_upper(index, mnt);
+	if (IS_ERR(upper)) {
+		err = PTR_ERR(upper);
+		if (err)
+			goto fail;
+	}
+
+	err = ovl_verify_origin_fh(upper, fh);
 	if (err)
 		goto fail;
 
-	err = ovl_check_origin(index, lowerstack, numlower, &stack, &ctr);
+	err = ovl_check_origin(upper, lowerstack, numlower, &stack, &ctr);
 	if (!err && !ctr)
 		err = -ESTALE;
 	if (err)
 		goto fail;
 
 	/* Check if index is orphan and don't warn before cleaning it */
-	if (d_inode(index)->i_nlink == 1 &&
+	if (!d_is_dir(index) && d_inode(index)->i_nlink == 1 &&
 	    ovl_get_nlink(index, origin.dentry, 0) == 0)
 		err = -ENOENT;
 
-	dput(origin.dentry);
 out:
+	dput(origin.dentry);
+	dput(upper);
 	kfree(fh);
 	return err;
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 35f5452e61e0..e4caad7bae19 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -263,8 +263,8 @@ static inline bool ovl_is_impuredir(struct dentry *dentry)
 /* namei.c */
 int ovl_verify_origin(struct dentry *dentry, struct dentry *origin,
 		      bool is_upper, bool set);
-int ovl_verify_index(struct dentry *index, struct path *lowerstack,
-		     unsigned int numlower);
+int ovl_verify_index(struct dentry *index, struct vfsmount *mnt,
+		     struct path *lowerstack, unsigned int numlower);
 int ovl_get_index_name(struct dentry *origin, struct qstr *name);
 int ovl_path_next(int idx, struct dentry *dentry, struct path *path, int *idxp);
 struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags);
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index ba44546ad1ed..479ce47ba411 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -1055,7 +1055,7 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
 			index = NULL;
 			break;
 		}
-		err = ovl_verify_index(index, lowerstack, numlower);
+		err = ovl_verify_index(index, mnt, lowerstack, numlower);
 		if (err) {
 			if (err == -EROFS)
 				break;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 07/11] ovl: index directories on copy up
  2017-10-17 16:00 [PATCH 00/11] Implement overlayfs index=all mount option Amir Goldstein
                   ` (5 preceding siblings ...)
  2017-10-17 16:00 ` [PATCH 06/11] ovl: verify directory index entries on mount Amir Goldstein
@ 2017-10-17 16:00 ` Amir Goldstein
  2017-10-17 16:00 ` [PATCH 08/11] ovl: cleanup dir index when dir nlink drops to zero Amir Goldstein
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2017-10-17 16:00 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

With mount option index=all, all files are indexed on copy up.
Non-dir files are copied up directly to indexdir and then hardlinked
to upper dir.

Directories are copied up to workdir, then an index entry is created
in indexdir with origin xattr pointing to the copied up dir and then
the copied up dir is moved to upper dir.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/copy_up.c | 128 ++++++++++++++++++++++++++++++++++++++++++++-----
 fs/overlayfs/util.c    |   2 +-
 2 files changed, 118 insertions(+), 12 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 6729ea2a5a1c..df1ce2d5a20e 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -289,8 +289,8 @@ struct ovl_fh *ovl_encode_fh(struct dentry *lower, bool is_upper)
 	return fh;
 }
 
-static int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
-			  struct dentry *upper)
+static int ovl_set_origin(struct dentry *dentry, struct dentry *origin,
+			  struct dentry *upper, bool is_upper)
 {
 	const struct ovl_fh *fh = NULL;
 	int err;
@@ -300,8 +300,8 @@ static int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
 	 * so we can use the overlay.origin xattr to distignuish between a copy
 	 * up and a pure upper inode.
 	 */
-	if (ovl_can_decode_fh(lower->d_sb)) {
-		fh = ovl_encode_fh(lower, false);
+	if (ovl_can_decode_fh(origin->d_sb)) {
+		fh = ovl_encode_fh(origin, is_upper);
 		if (IS_ERR(fh))
 			return PTR_ERR(fh);
 	}
@@ -316,6 +316,95 @@ static int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
 	return err;
 }
 
+/*
+ * Create and install index entry.
+ *
+ * With mount option index=all, this function is called for every dir copy up.
+ * Otherwise, it should be called before exporting an overlay file handle.
+ *
+ * Caller must hold 'lock_rename' on workdir+upperdir.
+ */
+static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
+			    struct dentry *upper)
+{
+	struct dentry *workdir = ovl_workdir(dentry);
+	struct dentry *indexdir = ovl_indexdir(dentry->d_sb);
+	struct inode *wdir = d_inode(workdir);
+	struct inode *idir = d_inode(indexdir);
+	struct dentry *index = NULL;
+	struct dentry *temp = NULL;
+	struct qstr name = { };
+	int err;
+
+	/*
+	 * For now this is only used for creating index entry for directories,
+	 * because non-dir are copied up directly to index and then hardlinked
+	 * to upper dir.
+	 *
+	 * TODO: implement create index for non-dir, so we can call it when
+	 * encoding file handle for non-dir in case index does not exist.
+	 */
+	if (WARN_ON(!d_is_dir(dentry)))
+		return -EIO;
+
+	/* Directory not expected to be indexed before copy up */
+	if (WARN_ON(ovl_test_flag(OVL_INDEX, d_inode(dentry))))
+		return -EIO;
+
+	err = ovl_get_index_name(origin, &name);
+	if (err)
+		return err;
+
+	temp = ovl_lookup_temp(workdir);
+	if (IS_ERR(temp))
+		goto temp_err;
+
+	err = ovl_do_mkdir(wdir, temp, S_IFDIR, true);
+	if (err)
+		goto out;
+
+	err = ovl_set_origin(dentry, upper, temp, true);
+	if (err)
+		goto out_cleanup;
+
+	/*
+	 * vfs_rename_mutex is held and so are workdir and upperdir inode mutex.
+	 * Because workdir and upperdir are sibling directories,
+	 * lock_rename(workdir, upperdir) used lockedp classes I_MUTEX_PARENT
+	 * and I_MUTEX_PARENT2, so it is safe to 'promote' the rename lock
+	 * to workdir+upperdir+indexdir (yet another sibling) using lockdep
+	 * class I_MUTEX_CHILD without releasing workdir+upperdir rename lock.
+	 */
+	inode_lock_nested(idir, I_MUTEX_CHILD);
+
+	index = lookup_one_len(name.name, indexdir, name.len);
+	if (IS_ERR(index)) {
+		err = PTR_ERR(index);
+	} else {
+		err = ovl_do_rename(wdir, temp, idir, index, 0);
+		dput(index);
+	}
+
+	inode_unlock(idir);
+
+	if (err)
+		goto out_cleanup;
+
+out:
+	dput(temp);
+	kfree(name.name);
+	return err;
+
+temp_err:
+	err = PTR_ERR(temp);
+	temp = NULL;
+	goto out;
+
+out_cleanup:
+	ovl_cleanup(wdir, temp);
+	goto out;
+}
+
 struct ovl_copy_up_ctx {
 	struct dentry *parent;
 	struct dentry *dentry;
@@ -328,6 +417,7 @@ struct ovl_copy_up_ctx {
 	struct dentry *workdir;
 	bool tmpfile;
 	bool origin;
+	bool indexed;
 };
 
 static int ovl_link_up(struct ovl_copy_up_ctx *c)
@@ -475,7 +565,8 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
 	 * hard link.
 	 */
 	if (c->origin) {
-		err = ovl_set_origin(c->dentry, c->lowerpath.dentry, temp);
+		err = ovl_set_origin(c->dentry, c->lowerpath.dentry, temp,
+				     false);
 		if (err)
 			return err;
 	}
@@ -498,6 +589,12 @@ static int ovl_copy_up_locked(struct ovl_copy_up_ctx *c)
 	if (err)
 		goto out_cleanup;
 
+	if (S_ISDIR(c->stat.mode) && c->indexed) {
+		err = ovl_create_index(c->dentry, c->lowerpath.dentry, temp);
+		if (err)
+			goto out_cleanup;
+	}
+
 	if (c->tmpfile) {
 		inode_lock_nested(udir, I_MUTEX_PARENT);
 		err = ovl_install_temp(c, temp, &newdentry);
@@ -532,12 +629,20 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 {
 	int err;
 	struct ovl_fs *ofs = c->dentry->d_sb->s_fs_info;
-	bool indexed = ovl_need_index(c->dentry);
+	bool to_index;
+
+	/*
+	 * Indexed non-dir is copied up directly to indexdir and then
+	 * hardlinked to upper dir. Indexed dir is copied up to workdir,
+	 * then index created in indexdir and then copied up dir installed.
+	 */
+	c->indexed = ovl_need_index(c->dentry);
+	to_index = c->indexed && !S_ISDIR(c->stat.mode);
 
-	if (S_ISDIR(c->stat.mode) || c->stat.nlink == 1 || indexed)
+	if (S_ISDIR(c->stat.mode) || c->stat.nlink == 1 || to_index)
 		c->origin = true;
 
-	if (indexed) {
+	if (to_index) {
 		c->destdir = ovl_indexdir(c->dentry->d_sb);
 		err = ovl_get_index_name(c->lowerpath.dentry, &c->destname);
 		if (err)
@@ -564,9 +669,10 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 		}
 	}
 
-	if (indexed) {
-		if (!err)
-			ovl_set_flag(OVL_INDEX, d_inode(c->dentry));
+	if (!err && c->indexed)
+		ovl_set_flag(OVL_INDEX, d_inode(c->dentry));
+
+	if (to_index) {
 		kfree(c->destname.name);
 	} else if (!err) {
 		struct inode *udir = d_inode(c->destdir);
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index fb501eb5e75c..3cd16bc20fcd 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -516,7 +516,7 @@ bool ovl_need_index(struct dentry *dentry)
 	if (!lower || !ofs->indexdir)
 		return false;
 
-	if (!d_is_dir(lower) && ofs->config.index == OVL_INDEX_ALL)
+	if (ofs->config.index == OVL_INDEX_ALL)
 		return true;
 
 	/* Index only lower hardlinks on copy up */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 08/11] ovl: cleanup dir index when dir nlink drops to zero
  2017-10-17 16:00 [PATCH 00/11] Implement overlayfs index=all mount option Amir Goldstein
                   ` (6 preceding siblings ...)
  2017-10-17 16:00 ` [PATCH 07/11] ovl: index directories on copy up Amir Goldstein
@ 2017-10-17 16:00 ` Amir Goldstein
  2017-10-17 16:00 ` [PATCH 09/11] ovl: whiteout index when union " Amir Goldstein
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2017-10-17 16:00 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

When non-dir index union nlink drops to zero the non-dir index
is cleaned. Do the same for directory type index entries when
union directory is removed.

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

diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 3cd16bc20fcd..22cdf9ecd7ec 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -466,7 +466,7 @@ static void ovl_cleanup_index(struct dentry *dentry)
 		goto fail;
 
 	inode = d_inode(upperdentry);
-	if (inode->i_nlink != 1) {
+	if (!S_ISDIR(inode->i_mode) && inode->i_nlink != 1) {
 		pr_warn_ratelimited("overlayfs: cleanup linked index (%pd2, ino=%lu, nlink=%u)\n",
 				    upperdentry, inode->i_ino, inode->i_nlink);
 		/*
@@ -537,7 +537,7 @@ int ovl_nlink_start(struct dentry *dentry, bool *locked)
 	const struct cred *old_cred;
 	int err;
 
-	if (!d_inode(dentry) || d_is_dir(dentry))
+	if (!d_inode(dentry))
 		return 0;
 
 	/*
@@ -564,7 +564,7 @@ int ovl_nlink_start(struct dentry *dentry, bool *locked)
 	if (err)
 		return err;
 
-	if (!ovl_test_flag(OVL_INDEX, d_inode(dentry)))
+	if (d_is_dir(dentry) || !ovl_test_flag(OVL_INDEX, d_inode(dentry)))
 		goto out;
 
 	old_cred = ovl_override_creds(dentry->d_sb);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 09/11] ovl: whiteout index when union nlink drops to zero
  2017-10-17 16:00 [PATCH 00/11] Implement overlayfs index=all mount option Amir Goldstein
                   ` (7 preceding siblings ...)
  2017-10-17 16:00 ` [PATCH 08/11] ovl: cleanup dir index when dir nlink drops to zero Amir Goldstein
@ 2017-10-17 16:00 ` Amir Goldstein
  2017-10-17 16:00 ` [PATCH 10/11] ovl: whiteout orphan index entries on mount Amir Goldstein
  2017-10-17 16:00 ` [PATCH 11/11] ovl: cleanup stale whiteout " Amir Goldstein
  10 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2017-10-17 16:00 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

When overlay inode nlink drops to zero, instead of cleanup the index
entry, replace it with a whiteout index entry.
This is needed for NFS export in order to prevent future open by handle
to open the lower file directly.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/dir.c       | 58 +++++++++++++++++++++++++++++-------------------
 fs/overlayfs/overlayfs.h |  2 ++
 fs/overlayfs/util.c      | 20 +++++++++++------
 3 files changed, 50 insertions(+), 30 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index d916fc19e724..060a1bfa0531 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -63,8 +63,7 @@ struct dentry *ovl_lookup_temp(struct dentry *workdir)
 }
 
 /* caller holds i_mutex on workdir */
-static struct dentry *ovl_whiteout(struct dentry *workdir,
-				   struct dentry *dentry)
+static struct dentry *ovl_whiteout(struct dentry *workdir)
 {
 	int err;
 	struct dentry *whiteout;
@@ -83,6 +82,38 @@ static struct dentry *ovl_whiteout(struct dentry *workdir,
 	return whiteout;
 }
 
+/* Caller must hold 'lock_rename' on workdir+dir */
+int ovl_cleanup_and_whiteout(struct dentry *workdir, struct inode *dir,
+			     struct dentry *dentry)
+{
+	struct inode *wdir = workdir->d_inode;
+	struct dentry *whiteout;
+	int err;
+	int flags = 0;
+
+	whiteout = ovl_whiteout(workdir);
+	err = PTR_ERR(whiteout);
+	if (IS_ERR(whiteout))
+		return err;
+
+	if (d_is_dir(dentry))
+		flags = RENAME_EXCHANGE;
+
+	err = ovl_do_rename(wdir, whiteout, dir, dentry, flags);
+	if (err)
+		goto kill_whiteout;
+	if (flags)
+		ovl_cleanup(wdir, dentry);
+
+out:
+	dput(whiteout);
+	return err;
+
+kill_whiteout:
+	ovl_cleanup(wdir, whiteout);
+	goto out;
+}
+
 int ovl_create_real(struct inode *dir, struct dentry *newdentry,
 		    struct cattr *attr, struct dentry *hardlink, bool debug)
 {
@@ -629,14 +660,10 @@ static bool ovl_matches_upper(struct dentry *dentry, struct dentry *upper)
 static int ovl_remove_and_whiteout(struct dentry *dentry, bool is_dir)
 {
 	struct dentry *workdir = ovl_workdir(dentry);
-	struct inode *wdir = workdir->d_inode;
 	struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
-	struct inode *udir = upperdir->d_inode;
-	struct dentry *whiteout;
 	struct dentry *upper;
 	struct dentry *opaquedir = NULL;
 	int err;
-	int flags = 0;
 
 	if (WARN_ON(!workdir))
 		return -EROFS;
@@ -665,24 +692,13 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, bool is_dir)
 		goto out_dput_upper;
 	}
 
-	whiteout = ovl_whiteout(workdir, dentry);
-	err = PTR_ERR(whiteout);
-	if (IS_ERR(whiteout))
-		goto out_dput_upper;
-
-	if (d_is_dir(upper))
-		flags = RENAME_EXCHANGE;
-
-	err = ovl_do_rename(wdir, whiteout, udir, upper, flags);
+	err = ovl_cleanup_and_whiteout(workdir, d_inode(upperdir), upper);
 	if (err)
-		goto kill_whiteout;
-	if (flags)
-		ovl_cleanup(wdir, upper);
+		goto out_d_drop;
 
 	ovl_dentry_version_inc(dentry->d_parent, true);
 out_d_drop:
 	d_drop(dentry);
-	dput(whiteout);
 out_dput_upper:
 	dput(upper);
 out_unlock:
@@ -691,10 +707,6 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, bool is_dir)
 	dput(opaquedir);
 out:
 	return err;
-
-kill_whiteout:
-	ovl_cleanup(wdir, whiteout);
-	goto out_d_drop;
 }
 
 static int ovl_remove_upper(struct dentry *dentry, bool is_dir)
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index e4caad7bae19..17c039485dc9 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -327,6 +327,8 @@ int ovl_create_real(struct inode *dir, struct dentry *newdentry,
 		    struct cattr *attr,
 		    struct dentry *hardlink, bool debug);
 int ovl_cleanup(struct inode *dir, struct dentry *dentry);
+int ovl_cleanup_and_whiteout(struct dentry *workdir, struct inode *dir,
+			     struct dentry *dentry);
 
 /* copy_up.c */
 int ovl_copy_up(struct dentry *dentry);
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 22cdf9ecd7ec..515ae156939f 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -453,7 +453,8 @@ void ovl_inuse_unlock(struct dentry *dentry)
 /* 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;
+	struct dentry *workdir = ovl_workdir(dentry);
+	struct dentry *indexdir = ovl_indexdir(dentry->d_sb);
 	struct dentry *lowerdentry = ovl_dentry_lower(dentry);
 	struct dentry *upperdentry = ovl_dentry_upper(dentry);
 	struct dentry *index = NULL;
@@ -483,16 +484,21 @@ static void ovl_cleanup_index(struct dentry *dentry)
 		goto out;
 	}
 
-	inode_lock_nested(dir, I_MUTEX_PARENT);
-	/* TODO: whiteout instead of cleanup to block future open by handle */
+	err = -EIO;
+	if (lock_rename(workdir, indexdir) != NULL)
+		goto fail;
+
 	index = lookup_one_len(name.name, ovl_indexdir(dentry->d_sb), name.len);
 	err = PTR_ERR(index);
-	if (!IS_ERR(index))
-		err = ovl_cleanup(dir, index);
-	else
+	if (!IS_ERR(index)) {
+		/* Whiteout index to block future open by handle */
+		err = ovl_cleanup_and_whiteout(workdir, d_inode(indexdir),
+					       index);
+	} else {
 		index = NULL;
+	}
 
-	inode_unlock(dir);
+	unlock_rename(workdir, indexdir);
 	if (err)
 		goto fail;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 10/11] ovl: whiteout orphan index entries on mount
  2017-10-17 16:00 [PATCH 00/11] Implement overlayfs index=all mount option Amir Goldstein
                   ` (8 preceding siblings ...)
  2017-10-17 16:00 ` [PATCH 09/11] ovl: whiteout index when union " Amir Goldstein
@ 2017-10-17 16:00 ` Amir Goldstein
  2017-10-17 16:00 ` [PATCH 11/11] ovl: cleanup stale whiteout " Amir Goldstein
  10 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2017-10-17 16:00 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

Invalid index entries and stale non-dir index entries are
removed on mount.

Non-dir index entries that have no more linked aliases
need to be whited out on mount to block future open by handle.

When dir index has a stale origin fh to upper dir, we assume
that upper dir was removed and we treat the dir index as orphan
entry that needs to be whited out.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/namei.c     | 27 ++++++++++++++++++++++++---
 fs/overlayfs/overlayfs.h |  5 +++--
 fs/overlayfs/readdir.c   | 46 ++++++++++++++++++++++++++++++++++------------
 fs/overlayfs/super.c     |  5 ++---
 4 files changed, 63 insertions(+), 20 deletions(-)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index aba57d31d850..1ec65dfbf1bb 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -174,8 +174,14 @@ static struct dentry *ovl_get_origin(struct dentry *dentry,
 				    bytes >> 2, (int)fh->type,
 				    ovl_acceptable, mnt);
 	if (IS_ERR(origin)) {
-		/* Treat stale file handle as "origin unknown" */
-		if (origin == ERR_PTR(-ESTALE))
+		/*
+		 * Treat stale file handle to lower file as "origin unknown".
+		 * upper file handle could become stale when upper file is
+		 * unlinked and this information is needed to handle stale
+		 * index entries correctly.
+		 */
+		if (origin == ERR_PTR(-ESTALE) &&
+		    !(fh->flags & OVL_FH_FLAG_PATH_UPPER))
 			origin = NULL;
 		goto out;
 	}
@@ -465,6 +471,14 @@ int ovl_verify_index(struct dentry *index, struct vfsmount *mnt,
 	upper = ovl_index_upper(index, mnt);
 	if (IS_ERR(upper)) {
 		err = PTR_ERR(upper);
+		/*
+		 * Invalid index entries and stale non-dir index entries need
+		 * to be removed.  When dir index has a stale origin fh to upper
+		 * dir, we assume that upper dir was removed and we treat the
+		 * dir index as orphan entry that needs to be whited out.
+		 */
+		if (err == -ESTALE)
+			goto orphan;
 		if (err)
 			goto fail;
 	}
@@ -482,7 +496,7 @@ int ovl_verify_index(struct dentry *index, struct vfsmount *mnt,
 	/* Check if index is orphan and don't warn before cleaning it */
 	if (!d_is_dir(index) && d_inode(index)->i_nlink == 1 &&
 	    ovl_get_nlink(index, origin.dentry, 0) == 0)
-		err = -ENOENT;
+		goto orphan;
 
 out:
 	dput(origin.dentry);
@@ -494,6 +508,13 @@ int ovl_verify_index(struct dentry *index, struct vfsmount *mnt,
 	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;
+
+orphan:
+	pr_warn_ratelimited("overlayfs: orphan index entry (%pd2, ftype=%x, nlink=%u)\n",
+			    index, d_inode(index)->i_mode & S_IFMT,
+			    d_inode(index)->i_nlink);
+	err = -ENOENT;
+	goto out;
 }
 
 /*
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 17c039485dc9..18ea89deb040 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -279,8 +279,9 @@ void ovl_dir_cache_free(struct inode *inode);
 int ovl_check_d_type_supported(struct path *realpath);
 void ovl_workdir_cleanup(struct inode *dir, struct vfsmount *mnt,
 			 struct dentry *dentry, int level);
-int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
-			 struct path *lowerstack, unsigned int numlower);
+struct ovl_fs;
+int ovl_indexdir_cleanup(struct ovl_fs *ofs, struct path *lowerstack,
+			 unsigned int numlower);
 
 /* inode.c */
 int ovl_set_nlink_upper(struct dentry *dentry);
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 479ce47ba411..ce8cdbb4c7d9 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -17,6 +17,7 @@
 #include <linux/cred.h>
 #include <linux/ratelimit.h>
 #include "overlayfs.h"
+#include "ovl_entry.h"
 
 struct ovl_cache_entry {
 	unsigned int len;
@@ -1019,13 +1020,16 @@ void ovl_workdir_cleanup(struct inode *dir, struct vfsmount *mnt,
 	}
 }
 
-int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
-			 struct path *lowerstack, unsigned int numlower)
+int ovl_indexdir_cleanup(struct ovl_fs *ofs, struct path *lowerstack,
+			 unsigned int numlower)
 {
 	int err;
+	struct dentry *workdir = ofs->workdir;
+	struct dentry *indexdir = ofs->indexdir;
+	struct vfsmount *mnt = ofs->upper_mnt;
 	struct dentry *index = NULL;
-	struct inode *dir = dentry->d_inode;
-	struct path path = { .mnt = mnt, .dentry = dentry };
+	struct inode *dir = indexdir->d_inode;
+	struct path path = { .mnt = mnt, .dentry = indexdir };
 	LIST_HEAD(list);
 	struct rb_root root = RB_ROOT;
 	struct ovl_cache_entry *p;
@@ -1041,7 +1045,6 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
 	if (err)
 		goto out;
 
-	inode_lock_nested(dir, I_MUTEX_PARENT);
 	list_for_each_entry(p, &list, l_node) {
 		if (p->name[0] == '.') {
 			if (p->len == 1)
@@ -1049,25 +1052,44 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
 			if (p->len == 2 && p->name[1] == '.')
 				continue;
 		}
-		index = lookup_one_len(p->name, dentry, p->len);
+
+		err = ovl_lock_rename_workdir(workdir, indexdir);
+		if (err)
+			break;
+
+		index = lookup_one_len(p->name, indexdir, p->len);
 		if (IS_ERR(index)) {
 			err = PTR_ERR(index);
 			index = NULL;
 			break;
 		}
 		err = ovl_verify_index(index, mnt, lowerstack, numlower);
-		if (err) {
-			if (err == -EROFS)
-				break;
+		if (!err || err == -EROFS || err == -ENOMEM) {
+			/*
+			 * Abort mount to avoid corrupting the index if
+			 * an incompatible index entry was found or on out
+			 * of memory.
+			 */
+			goto unlock;
+		} else if (err == -ENOENT) {
+			/*
+			 * Whiteout orphan index to block future open by
+			 * handle after overlay nlink dropepd to zero.
+			 */
+			err = ovl_cleanup_and_whiteout(workdir, dir, index);
+		} else {
+			/* Cleanup stale and invalid index entries */
 			err = ovl_cleanup(dir, index);
-			if (err)
-				break;
 		}
+
+unlock:
+		unlock_rename(workdir, indexdir);
 		dput(index);
 		index = NULL;
+		if (err)
+			break;
 	}
 	dput(index);
-	inode_unlock(dir);
 out:
 	ovl_cache_free(&list);
 	if (err)
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 24fac7d987a3..0db59616c840 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1100,9 +1100,8 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 
 			/* Cleanup bad/stale/orphan index entries */
 			if (!err)
-				err = ovl_indexdir_cleanup(ufs->indexdir,
-							   ufs->upper_mnt,
-							   stack, numlower);
+				err = ovl_indexdir_cleanup(ufs, stack,
+							   numlower);
 		}
 		if (err || !ufs->indexdir)
 			pr_warn("overlayfs: try deleting index dir or mounting with '-o index=off' to disable inodes index.\n");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 11/11] ovl: cleanup stale whiteout index entries on mount
  2017-10-17 16:00 [PATCH 00/11] Implement overlayfs index=all mount option Amir Goldstein
                   ` (9 preceding siblings ...)
  2017-10-17 16:00 ` [PATCH 10/11] ovl: whiteout orphan index entries on mount Amir Goldstein
@ 2017-10-17 16:00 ` Amir Goldstein
  10 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2017-10-17 16:00 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

Check on mount that whiteout index entries have a name that
resolves to a non-stale file handle.

Do not verify that they have a valid origin fh xattr and do
not verify nlink xattr for orphan index check.

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

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 1ec65dfbf1bb..e2b47d6494fd 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -100,9 +100,37 @@ static int ovl_acceptable(void *mnt, struct dentry *dentry)
 	return is_subdir(dentry, ((struct vfsmount *)mnt)->mnt_root);
 }
 
+
+/*
+ * Check validity of an overlay file handle buffer.
+ *
+ * Return 0 for a valid file handle.
+ * Return -ENODATA for "origin unknown".
+ * Return <0 for an invalid file handle.
+ */
+static int ovl_check_fh_len(struct ovl_fh *fh, int fh_len)
+{
+	if (fh_len < sizeof(struct ovl_fh) || fh_len < fh->len)
+		return -EINVAL;
+
+	if (fh->magic != OVL_FH_MAGIC)
+		return -EINVAL;
+
+	/* Treat larger version and unknown flags as "origin unknown" */
+	if (fh->version > OVL_FH_VERSION || fh->flags & ~OVL_FH_FLAG_ALL)
+		return -ENODATA;
+
+	/* Treat endianness mismatch as "origin unknown" */
+	if (!(fh->flags & OVL_FH_FLAG_ANY_ENDIAN) &&
+	    (fh->flags & OVL_FH_FLAG_BIG_ENDIAN) != OVL_FH_FLAG_CPU_ENDIAN)
+		return -ENODATA;
+
+	return 0;
+}
+
 static struct ovl_fh *ovl_get_origin_fh(struct dentry *dentry)
 {
-	int res;
+	int res, err;
 	struct ovl_fh *fh = NULL;
 
 	res = vfs_getxattr(dentry, OVL_XATTR_ORIGIN, NULL, 0);
@@ -115,7 +143,7 @@ static struct ovl_fh *ovl_get_origin_fh(struct dentry *dentry)
 	if (res == 0)
 		return NULL;
 
-	fh  = kzalloc(res, GFP_KERNEL);
+	fh = kzalloc(res, GFP_KERNEL);
 	if (!fh)
 		return ERR_PTR(-ENOMEM);
 
@@ -123,20 +151,12 @@ static struct ovl_fh *ovl_get_origin_fh(struct dentry *dentry)
 	if (res < 0)
 		goto fail;
 
-	if (res < sizeof(struct ovl_fh) || res < fh->len)
-		goto invalid;
-
-	if (fh->magic != OVL_FH_MAGIC)
+	err = ovl_check_fh_len(fh, res);
+	if (err < 0) {
+		if (err == -ENODATA)
+			goto out;
 		goto invalid;
-
-	/* Treat larger version and unknown flags as "origin unknown" */
-	if (fh->version > OVL_FH_VERSION || fh->flags & ~OVL_FH_FLAG_ALL)
-		goto out;
-
-	/* Treat endianness mismatch as "origin unknown" */
-	if (!(fh->flags & OVL_FH_FLAG_ANY_ENDIAN) &&
-	    (fh->flags & OVL_FH_FLAG_BIG_ENDIAN) != OVL_FH_FLAG_CPU_ENDIAN)
-		goto out;
+	}
 
 	return fh;
 
@@ -152,22 +172,17 @@ static struct ovl_fh *ovl_get_origin_fh(struct dentry *dentry)
 	goto out;
 }
 
-static struct dentry *ovl_get_origin(struct dentry *dentry,
-				     struct vfsmount *mnt)
+static struct dentry *ovl_decode_fh(struct ovl_fh *fh, struct vfsmount *mnt)
 {
-	struct dentry *origin = NULL;
-	struct ovl_fh *fh = ovl_get_origin_fh(dentry);
+	struct dentry *origin;
 	int bytes;
 
-	if (IS_ERR_OR_NULL(fh))
-		return (struct dentry *)fh;
-
 	/*
 	 * Make sure that the stored uuid matches the uuid of the lower
 	 * layer where file handle will be decoded.
 	 */
 	if (!uuid_equal(&fh->uuid, &mnt->mnt_sb->s_uuid))
-		goto out;
+		return NULL;
 
 	bytes = (fh->len - offsetof(struct ovl_fh, fid));
 	origin = exportfs_decode_fh(mnt, (struct fid *)fh->fid,
@@ -183,22 +198,15 @@ static struct dentry *ovl_get_origin(struct dentry *dentry,
 		if (origin == ERR_PTR(-ESTALE) &&
 		    !(fh->flags & OVL_FH_FLAG_PATH_UPPER))
 			origin = NULL;
-		goto out;
+		return origin;
 	}
 
-	if (ovl_dentry_weird(origin) ||
-	    ((d_inode(origin)->i_mode ^ d_inode(dentry)->i_mode) & S_IFMT))
-		goto invalid;
+	if (ovl_dentry_weird(origin)) {
+		dput(origin);
+		return NULL;
+	}
 
-out:
-	kfree(fh);
 	return origin;
-
-invalid:
-	pr_warn_ratelimited("overlayfs: invalid origin (%pd2)\n", origin);
-	dput(origin);
-	origin = NULL;
-	goto out;
 }
 
 static bool ovl_is_opaquedir(struct dentry *dentry)
@@ -303,29 +311,30 @@ static int ovl_lookup_layer(struct dentry *base, struct ovl_lookup_data *d,
 }
 
 
-static int ovl_check_origin(struct dentry *upperdentry,
-			    struct path *lowerstack, unsigned int numlower,
-			    struct path **stackp, unsigned int *ctrp)
+static int ovl_check_origin_fh(struct ovl_fh *fh, struct dentry *upperdentry,
+			       struct path *lowerstack, unsigned int numlower,
+			       struct path **stackp)
 {
 	struct vfsmount *mnt;
 	struct dentry *origin = NULL;
 	int i;
 
-
 	for (i = 0; i < numlower; i++) {
 		mnt = lowerstack[i].mnt;
-		origin = ovl_get_origin(upperdentry, mnt);
-		if (IS_ERR(origin))
-			return PTR_ERR(origin);
-
+		origin = ovl_decode_fh(fh, mnt);
 		if (origin)
 			break;
 	}
 
 	if (!origin)
-		return 0;
+		return -ESTALE;
+	else if (IS_ERR(origin))
+		return PTR_ERR(origin);
+
+	if (!ovl_is_whiteout(upperdentry) &&
+	    ((d_inode(origin)->i_mode ^ d_inode(upperdentry)->i_mode) & S_IFMT))
+		goto invalid;
 
-	BUG_ON(*ctrp);
 	if (!*stackp)
 		*stackp = kmalloc(sizeof(struct path), GFP_KERNEL);
 	if (!*stackp) {
@@ -333,9 +342,42 @@ static int ovl_check_origin(struct dentry *upperdentry,
 		return -ENOMEM;
 	}
 	**stackp = (struct path) { .dentry = origin, .mnt = mnt };
-	*ctrp = 1;
 
 	return 0;
+
+invalid:
+	pr_warn_ratelimited("overlayfs: invalid origin (%pd2, ftype=%x, origin ftype=%x).\n",
+			    upperdentry, d_inode(upperdentry)->i_mode & S_IFMT,
+			    d_inode(origin)->i_mode & S_IFMT);
+	dput(origin);
+	return -ESTALE;
+}
+
+static int ovl_check_origin(struct dentry *upperdentry,
+			    struct path *lowerstack, unsigned int numlower,
+			    struct path **stackp, unsigned int *ctrp)
+{
+	struct ovl_fh *fh = ovl_get_origin_fh(upperdentry);
+	int err;
+
+	if (IS_ERR_OR_NULL(fh))
+		return PTR_ERR(fh);
+
+	err = ovl_check_origin_fh(fh, upperdentry, lowerstack, numlower,
+				  stackp);
+	kfree(fh);
+
+	if (err) {
+		if (err == -ESTALE)
+			return 0;
+		return err;
+	}
+
+	if (WARN_ON(*ctrp))
+		return -EIO;
+
+	*ctrp = 1;
+	return 0;
 }
 
 /*
@@ -429,7 +471,6 @@ int ovl_verify_index(struct dentry *index, struct vfsmount *mnt,
 	size_t len;
 	struct path origin = { };
 	struct path *stack = &origin;
-	unsigned int ctr = 0;
 	struct dentry *upper = NULL;
 	int err;
 
@@ -447,7 +488,15 @@ int ovl_verify_index(struct dentry *index, struct vfsmount *mnt,
 		goto fail;
 
 	err = -EINVAL;
-	if (hex2bin((u8 *)fh, index->d_name.name, len) || len != fh->len)
+	if (hex2bin((u8 *)fh, index->d_name.name, len))
+		goto fail;
+
+	err = ovl_check_fh_len(fh, len);
+	if (err)
+		goto fail;
+
+	err = ovl_check_origin_fh(fh, index, lowerstack, numlower, &stack);
+	if (err)
 		goto fail;
 
 	/*
@@ -455,9 +504,7 @@ int ovl_verify_index(struct dentry *index, struct vfsmount *mnt,
 	 * an exported overlay file handle should be treated as stale
 	 * (i.e. after unlink/rmdir of the overlay inode).
 	 * These entries contain no xattr.
-	 * TODO: cleanup stale whiteout index (try to decode index name).
 	 */
-	err = 0;
 	if (ovl_is_whiteout(index))
 		goto out;
 
@@ -487,12 +534,6 @@ int ovl_verify_index(struct dentry *index, struct vfsmount *mnt,
 	if (err)
 		goto fail;
 
-	err = ovl_check_origin(upper, lowerstack, numlower, &stack, &ctr);
-	if (!err && !ctr)
-		err = -ESTALE;
-	if (err)
-		goto fail;
-
 	/* Check if index is orphan and don't warn before cleaning it */
 	if (!d_is_dir(index) && d_inode(index)->i_nlink == 1 &&
 	    ovl_get_nlink(index, origin.dentry, 0) == 0)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* Re: [PATCH 06/11] ovl: verify directory index entries on mount
  2017-10-17 16:00 ` [PATCH 06/11] ovl: verify directory index entries on mount Amir Goldstein
@ 2017-10-19 10:35   ` Amir Goldstein
  0 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2017-10-19 10:35 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: overlayfs

On Tue, Oct 17, 2017 at 7:00 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> Directory index entries should have origin xattr pointing to the
> real upper dir. Non-dir index entries are hardlinks to the upper
> real inode. For non-dir index, we can read the copy up origin xattr
> directly from the index dentry, but for dir index we first need to
> decode the upper directory.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/overlayfs/namei.c     | 60 +++++++++++++++++++++++++++++++++---------------
>  fs/overlayfs/overlayfs.h |  4 ++--
>  fs/overlayfs/readdir.c   |  2 +-
>  3 files changed, 45 insertions(+), 21 deletions(-)
>
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index 1afc6d6e99b9..aba57d31d850 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -391,36 +391,45 @@ int ovl_verify_origin(struct dentry *dentry, struct dentry *origin,
>         goto out;
>  }
>
> +/* Get upper dentry from index */
> +struct dentry *ovl_index_upper(struct dentry *index, struct vfsmount *mnt)
> +{
> +       struct path upperpath = { .mnt = mnt };
> +       struct path *stack = &upperpath;
> +       unsigned int ctr = 0;
> +       int err;
> +
> +       if (!d_is_dir(index))
> +               return dget(index);
> +
> +       err = ovl_check_origin(index, stack, 1, &stack, &ctr);
> +       if (!err && !ctr)
> +               err = -ENODATA;
> +       if (err)
> +               return ERR_PTR(err);
> +
> +       return upperpath.dentry;
> +}
> +
>  /*
>   * Verify that an index entry name matches the origin file handle stored in
>   * OVL_XATTR_ORIGIN and that origin file handle can be decoded to lower path.
>   * Return 0 on match, -ESTALE on mismatch or stale origin, < 0 on error.
>   */
> -int ovl_verify_index(struct dentry *index, struct path *lowerstack,
> -                    unsigned int numlower)
> +int ovl_verify_index(struct dentry *index, struct vfsmount *mnt,
> +                    struct path *lowerstack, unsigned int numlower)
>  {
>         struct ovl_fh *fh = NULL;
>         size_t len;
>         struct path origin = { };
>         struct path *stack = &origin;
>         unsigned int ctr = 0;
> +       struct dentry *upper = NULL;
>         int err;
>
>         if (!d_inode(index))
>                 return 0;
>
> -       /*
> -        * 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.  We don't know the verification rules
> -        * for directory 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))
> -               goto fail;
> -
>         err = -EINVAL;
>         if (index->d_name.len < sizeof(struct ovl_fh)*2)
>                 goto fail;
> @@ -446,23 +455,38 @@ int ovl_verify_index(struct dentry *index, struct path *lowerstack,
>         if (ovl_is_whiteout(index))
>                 goto out;
>
> -       err = ovl_verify_origin_fh(index, fh);
> +       /*
> +        * Directory index entries should have origin xattr pointing to the
> +        * real upper dir. Non-dir index entries are hardlinks to the upper
> +        * real inode. For non-dir index, we can read the copy up origin xattr
> +        * directly from the index dentry, but for dir index we first need to
> +        * decode the upper directory.
> +        */
> +       upper = ovl_index_upper(index, mnt);
> +       if (IS_ERR(upper)) {
> +               err = PTR_ERR(upper);
> +               if (err)
> +                       goto fail;
> +       }
> +
> +       err = ovl_verify_origin_fh(upper, fh);

FYI, dput(upper) here instead of out:
pushed fixup patch to ovl-nfs-export-wip-v1

>         if (err)
>                 goto fail;
>
> -       err = ovl_check_origin(index, lowerstack, numlower, &stack, &ctr);
> +       err = ovl_check_origin(upper, lowerstack, numlower, &stack, &ctr);
>         if (!err && !ctr)
>                 err = -ESTALE;
>         if (err)
>                 goto fail;
>
>         /* Check if index is orphan and don't warn before cleaning it */
> -       if (d_inode(index)->i_nlink == 1 &&
> +       if (!d_is_dir(index) && d_inode(index)->i_nlink == 1 &&
>             ovl_get_nlink(index, origin.dentry, 0) == 0)
>                 err = -ENOENT;
>
> -       dput(origin.dentry);
>  out:
> +       dput(origin.dentry);
> +       dput(upper);
>         kfree(fh);
>         return err;
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 35f5452e61e0..e4caad7bae19 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -263,8 +263,8 @@ static inline bool ovl_is_impuredir(struct dentry *dentry)
>  /* namei.c */
>  int ovl_verify_origin(struct dentry *dentry, struct dentry *origin,
>                       bool is_upper, bool set);
> -int ovl_verify_index(struct dentry *index, struct path *lowerstack,
> -                    unsigned int numlower);
> +int ovl_verify_index(struct dentry *index, struct vfsmount *mnt,
> +                    struct path *lowerstack, unsigned int numlower);
>  int ovl_get_index_name(struct dentry *origin, struct qstr *name);
>  int ovl_path_next(int idx, struct dentry *dentry, struct path *path, int *idxp);
>  struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags);
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index ba44546ad1ed..479ce47ba411 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -1055,7 +1055,7 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
>                         index = NULL;
>                         break;
>                 }
> -               err = ovl_verify_index(index, lowerstack, numlower);
> +               err = ovl_verify_index(index, mnt, lowerstack, numlower);
>                 if (err) {
>                         if (err == -EROFS)
>                                 break;
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-10-19 10:35 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-17 16:00 [PATCH 00/11] Implement overlayfs index=all mount option Amir Goldstein
2017-10-17 16:00 ` [PATCH 01/11] ovl: fix EIO from lookup of non-indexed upper Amir Goldstein
2017-10-17 16:00 ` [PATCH 02/11] ovl: verify whiteout index entries on mount Amir Goldstein
2017-10-17 16:00 ` [PATCH 03/11] ovl: create ovl_need_index() helper Amir Goldstein
2017-10-17 16:00 ` [PATCH 04/11] ovl: add support for mount option index=all Amir Goldstein
2017-10-17 16:00 ` [PATCH 05/11] ovl: lookup index for directories Amir Goldstein
2017-10-17 16:00 ` [PATCH 06/11] ovl: verify directory index entries on mount Amir Goldstein
2017-10-19 10:35   ` Amir Goldstein
2017-10-17 16:00 ` [PATCH 07/11] ovl: index directories on copy up Amir Goldstein
2017-10-17 16:00 ` [PATCH 08/11] ovl: cleanup dir index when dir nlink drops to zero Amir Goldstein
2017-10-17 16:00 ` [PATCH 09/11] ovl: whiteout index when union " Amir Goldstein
2017-10-17 16:00 ` [PATCH 10/11] ovl: whiteout orphan index entries on mount Amir Goldstein
2017-10-17 16:00 ` [PATCH 11/11] ovl: cleanup stale whiteout " 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.