linux-unionfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] Implement overlayfs verify_dir mount option
@ 2017-10-17 15:23 Amir Goldstein
  2017-10-17 15:23 ` [PATCH 1/7] ovl: fix rmdir problem on origin dir Amir Goldstein
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Amir Goldstein @ 2017-10-17 15:23 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs, zhangyi

Miklos,

This is the first 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 'verify_dir' can be relaxed if we attach
generation numbers to index entries as you suggested, but
this was not implemented for first version of NFS export.

The first two patches in this series, one from zhangyi and
one from me are not directly related to 'verify_dir', but
they make sense in this context, because 'verify_dir' may
refuse to follow to lower dir and leave a pure upper dir
with ORIGIN and whiteouts.
These two patches have been around for a while and already
have a merged xfstest to verify them (overlay/031).

The last two patches add the actual verification of lower
dir. They have been on my development branch for a few cycles
already and the feature is being exercised with snapshots test.

[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

Amir Goldstein (7):
  ovl: fix rmdir problem on origin dir
  ovl: no direct iteration for dir with copy up origin
  ovl: remove unneeded arg from ovl_verify_origin()
  ovl: disable redirect_dir and index when no xattr support
  ovl: ignore redirect_dir and index when no upper layer
  ovl: add support for verify_dir mount option
  ovl: follow decoded origin file handle of merge dir

 Documentation/filesystems/overlayfs.txt | 20 ++++++++++
 fs/dcache.c                             |  1 +
 fs/overlayfs/dir.c                      | 21 +++++++---
 fs/overlayfs/namei.c                    | 60 +++++++++++++++++++++++++----
 fs/overlayfs/overlayfs.h                |  8 ++--
 fs/overlayfs/ovl_entry.h                |  1 +
 fs/overlayfs/readdir.c                  | 59 ++++++++++++++++++++++------
 fs/overlayfs/super.c                    | 68 +++++++++++++++++++++++----------
 fs/overlayfs/util.c                     | 20 ++++++++++
 9 files changed, 209 insertions(+), 49 deletions(-)

-- 
2.7.4

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

* [PATCH 1/7] ovl: fix rmdir problem on origin dir
  2017-10-17 15:23 [PATCH 0/7] Implement overlayfs verify_dir mount option Amir Goldstein
@ 2017-10-17 15:23 ` Amir Goldstein
  2017-10-31 12:54   ` Miklos Szeredi
  2017-10-17 15:23 ` [PATCH 2/7] ovl: no direct iteration for dir with copy up origin Amir Goldstein
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Amir Goldstein @ 2017-10-17 15:23 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs, zhangyi

If an "origin && not merged" upper dir have leftover whiteouts
(last mount created), ovl do not clear this dir when we
deleting it, which may lead to rmdir fail or temp file left
in workdir.

Simple reproducer:
  mkdir lower upper work merge
  mkdir -p lower/dir
  touch lower/dir/a
  mount -t overlay overlay -olowerdir=lower,upperdir=upper,\
    workdir=work merge
  rm merge/dir/a
  umount merge
  rm -rf lower/*
  touch lower/dir  (*)
  mount -t overlay overlay -olowerdir=lower,upperdir=upper,\
    workdir=work merge
  rm -rf merge/dir

Syslog dump:
  overlayfs: cleanup of 'work/#7' failed (-39)

(*): if we do not creat the regular file, the result is different:
  rm: cannot remove "dir/": Directory not empty

This patch add explicitly check of OVL_XATTR_ORIGIN, and filter
out whiteouts in upperdentry in ovl_check_empty_dir(). Finally,
check and clear the dir when deleting it.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/dir.c       | 21 +++++++++++++++------
 fs/overlayfs/namei.c     |  7 +++++--
 fs/overlayfs/overlayfs.h |  3 ++-
 fs/overlayfs/readdir.c   | 30 ++++++++++++++++++++++--------
 fs/overlayfs/util.c      | 13 +++++++++++++
 5 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index cc961a3bd3bd..d916fc19e724 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -181,6 +181,14 @@ static bool ovl_type_origin(struct dentry *dentry)
 	return OVL_TYPE_ORIGIN(ovl_path_type(dentry));
 }
 
+static bool ovl_is_origin(struct dentry *dentry)
+{
+	if (!OVL_TYPE_UPPER(ovl_path_type(dentry)))
+		return false;
+
+	return ovl_check_origin_xattr(ovl_dentry_upper(dentry));
+}
+
 static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
 			    struct cattr *attr, struct dentry *hardlink)
 {
@@ -300,7 +308,6 @@ static struct dentry *ovl_check_empty_and_clear(struct dentry *dentry)
 {
 	int err;
 	struct dentry *ret = NULL;
-	enum ovl_path_type type = ovl_path_type(dentry);
 	LIST_HEAD(list);
 
 	err = ovl_check_empty_dir(dentry, &list);
@@ -313,13 +320,13 @@ static struct dentry *ovl_check_empty_and_clear(struct dentry *dentry)
 	 * When removing an empty opaque directory, then it makes no sense to
 	 * replace it with an exact replica of itself.
 	 *
-	 * If no upperdentry then skip clearing whiteouts.
+	 * If upperdentry has whiteouts, clear them.
 	 *
 	 * Can race with copy-up, since we don't hold the upperdir mutex.
 	 * Doesn't matter, since copy-up can't create a non-empty directory
 	 * from an empty one.
 	 */
-	if (OVL_TYPE_UPPER(type) && OVL_TYPE_MERGE(type))
+	if (!list_empty(&list))
 		ret = ovl_clear_empty(dentry, &list);
 
 out_free:
@@ -698,8 +705,9 @@ static int ovl_remove_upper(struct dentry *dentry, bool is_dir)
 	struct dentry *opaquedir = NULL;
 	int err;
 
-	/* Redirect dir can be !ovl_lower_positive && OVL_TYPE_MERGE */
-	if (is_dir && ovl_dentry_get_redirect(dentry)) {
+	/* Redirect/origin dir can be !ovl_lower_positive && not clean */
+	if (is_dir && (ovl_dentry_get_redirect(dentry) ||
+		       ovl_is_origin(dentry))) {
 		opaquedir = ovl_check_empty_and_clear(dentry);
 		err = PTR_ERR(opaquedir);
 		if (IS_ERR(opaquedir))
@@ -946,7 +954,8 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
 
 	old_cred = ovl_override_creds(old->d_sb);
 
-	if (overwrite && new_is_dir && ovl_type_merge_or_lower(new)) {
+	if (overwrite && new_is_dir && (ovl_type_merge_or_lower(new) ||
+					ovl_is_origin(new))) {
 		opaquedir = ovl_check_empty_and_clear(new);
 		err = PTR_ERR(opaquedir);
 		if (IS_ERR(opaquedir)) {
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 654bea1a5ac9..1005c4fa640c 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -557,18 +557,21 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,
  * Returns next layer in stack starting from top.
  * Returns -1 if this is the last layer.
  */
-int ovl_path_next(int idx, struct dentry *dentry, struct path *path)
+int ovl_path_next(int idx, struct dentry *dentry, struct path *path, int *idxp)
 {
 	struct ovl_entry *oe = dentry->d_fsdata;
 
 	BUG_ON(idx < 0);
 	if (idx == 0) {
 		ovl_path_upper(dentry, path);
-		if (path->dentry)
+		if (path->dentry) {
+			*idxp = 0;
 			return oe->numlower ? 1 : -1;
+		}
 		idx++;
 	}
 	BUG_ON(idx > oe->numlower);
+	*idxp = idx;
 	*path = oe->lowerstack[idx - 1];
 
 	return (idx < oe->numlower) ? idx + 1 : -1;
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index c706a6f99928..2aef95047006 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -223,6 +223,7 @@ bool ovl_is_whiteout(struct dentry *dentry);
 struct file *ovl_path_open(struct path *path, int flags);
 int ovl_copy_up_start(struct dentry *dentry);
 void ovl_copy_up_end(struct dentry *dentry);
+bool ovl_check_origin_xattr(struct dentry *dentry);
 bool ovl_check_dir_xattr(struct dentry *dentry, const char *name);
 int ovl_check_setxattr(struct dentry *dentry, struct dentry *upperdentry,
 		       const char *name, const void *value, size_t size,
@@ -249,7 +250,7 @@ int ovl_verify_origin(struct dentry *dentry, struct vfsmount *mnt,
 int ovl_verify_index(struct dentry *index, 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 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);
 bool ovl_lower_positive(struct dentry *dentry);
 
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 0f85ee9c3268..55623b6635b6 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -26,6 +26,7 @@ struct ovl_cache_entry {
 	struct list_head l_node;
 	struct rb_node node;
 	struct ovl_cache_entry *next_maybe_whiteout;
+	int idx;
 	bool is_whiteout;
 	char name[];
 };
@@ -46,6 +47,7 @@ struct ovl_readdir_data {
 	struct list_head middle;
 	struct ovl_cache_entry *first_maybe_whiteout;
 	int count;
+	int idx;
 	int err;
 	bool is_upper;
 	bool d_type_supported;
@@ -159,6 +161,7 @@ static struct ovl_cache_entry *ovl_cache_entry_new(struct ovl_readdir_data *rdd,
 	if (ovl_calc_d_ino(rdd, p))
 		p->ino = 0;
 	p->is_whiteout = false;
+	p->idx = rdd->idx;
 
 	if (d_type == DT_CHR) {
 		p->next_maybe_whiteout = rdd->first_maybe_whiteout;
@@ -348,7 +351,7 @@ static int ovl_dir_read_merged(struct dentry *dentry, struct list_head *list,
 	int idx, next;
 
 	for (idx = 0; idx != -1; idx = next) {
-		next = ovl_path_next(idx, dentry, &realpath);
+		next = ovl_path_next(idx, dentry, &realpath, &rdd.idx);
 		rdd.is_upper = ovl_dentry_upper(dentry) == realpath.dentry;
 
 		if (next != -1) {
@@ -835,7 +838,7 @@ const struct file_operations ovl_dir_operations = {
 int ovl_check_empty_dir(struct dentry *dentry, struct list_head *list)
 {
 	int err;
-	struct ovl_cache_entry *p;
+	struct ovl_cache_entry *p, *n;
 	struct rb_root root = RB_ROOT;
 
 	err = ovl_dir_read_merged(dentry, list, &root);
@@ -844,18 +847,29 @@ int ovl_check_empty_dir(struct dentry *dentry, struct list_head *list)
 
 	err = 0;
 
-	list_for_each_entry(p, list, l_node) {
-		if (p->is_whiteout)
-			continue;
+	list_for_each_entry_safe(p, n, list, l_node) {
+		/*
+		 * Select whiteouts in upperdir, they should
+		 * be cleared when deleting this directory.
+		 */
+		if (p->is_whiteout) {
+			if (p->idx == 0)
+				continue;
+			goto del_entry;
+		}
 
 		if (p->name[0] == '.') {
 			if (p->len == 1)
-				continue;
+				goto del_entry;
 			if (p->len == 2 && p->name[1] == '.')
-				continue;
+				goto del_entry;
 		}
 		err = -ENOTEMPTY;
 		break;
+
+del_entry:
+		list_del(&p->l_node);
+		kfree(p);
 	}
 
 	return err;
@@ -869,7 +883,7 @@ void ovl_cleanup_whiteouts(struct dentry *upper, struct list_head *list)
 	list_for_each_entry(p, list, l_node) {
 		struct dentry *dentry;
 
-		if (!p->is_whiteout)
+		if (!p->is_whiteout || p->idx != 0)
 			continue;
 
 		dentry = lookup_one_len(p->name, upper, p->len);
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index b9b239fa5cfd..51ca8bd16009 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -329,6 +329,19 @@ void ovl_copy_up_end(struct dentry *dentry)
 	mutex_unlock(&OVL_I(d_inode(dentry))->lock);
 }
 
+bool ovl_check_origin_xattr(struct dentry *dentry)
+{
+	int res;
+
+	res = vfs_getxattr(dentry, OVL_XATTR_ORIGIN, NULL, 0);
+
+	/* Zero size value means "copied up but origin unknown" */
+	if (res >= 0)
+		return true;
+
+	return false;
+}
+
 bool ovl_check_dir_xattr(struct dentry *dentry, const char *name)
 {
 	int res;
-- 
2.7.4

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

* [PATCH 2/7] ovl: no direct iteration for dir with copy up origin
  2017-10-17 15:23 [PATCH 0/7] Implement overlayfs verify_dir mount option Amir Goldstein
  2017-10-17 15:23 ` [PATCH 1/7] ovl: fix rmdir problem on origin dir Amir Goldstein
@ 2017-10-17 15:23 ` Amir Goldstein
  2017-10-31 13:08   ` Miklos Szeredi
  2017-10-17 15:23 ` [PATCH 3/7] ovl: remove unneeded arg from ovl_verify_origin() Amir Goldstein
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Amir Goldstein @ 2017-10-17 15:23 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

If a non-merge dir in an overlay mount has an overlay.origin xattr, it
means it was once an upper merge dir, which may contain whiteouts and
then the lower dir was removed under it.

Do not iterate real dir directly in this case to avoid exposing whiteouts.

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

diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 55623b6635b6..ba44546ad1ed 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -319,21 +319,42 @@ static inline int ovl_dir_read(struct path *realpath,
 	return err;
 }
 
+/* Can we iterate real dir directly? */
+static bool ovl_dir_is_real(struct dentry *dir)
+{
+	struct path realpath;
+	enum ovl_path_type type = ovl_path_real(dir, &realpath);
+
+	if (OVL_TYPE_MERGE(type))
+		return false;
+
+	/*
+	 * Non-merge dir may contain whiteouts from a time it was a merge
+	 * upper, before lower dir was removed under it and possibly before
+	 * it was rotated from upper to lower layer.
+	 */
+	return !ovl_check_origin_xattr(realpath.dentry);
+}
+
 static void ovl_dir_reset(struct file *file)
 {
 	struct ovl_dir_file *od = file->private_data;
 	struct ovl_dir_cache *cache = od->cache;
 	struct dentry *dentry = file->f_path.dentry;
-	enum ovl_path_type type = ovl_path_type(dentry);
+	bool is_real;
 
 	if (cache && ovl_dentry_version_get(dentry) != cache->version) {
 		ovl_cache_put(od, dentry);
 		od->cache = NULL;
 		od->cursor = NULL;
 	}
-	WARN_ON(!od->is_real && !OVL_TYPE_MERGE(type));
-	if (od->is_real && OVL_TYPE_MERGE(type))
+	is_real = ovl_dir_is_real(dentry);
+	if (od->is_real != is_real) {
+		/* is_real can only become false when dir is copied up */
+		if (WARN_ON(is_real))
+			return;
 		od->is_real = false;
+	}
 }
 
 static int ovl_dir_read_merged(struct dentry *dentry, struct list_head *list,
@@ -819,7 +840,7 @@ static int ovl_dir_open(struct inode *inode, struct file *file)
 		return PTR_ERR(realfile);
 	}
 	od->realfile = realfile;
-	od->is_real = !OVL_TYPE_MERGE(type);
+	od->is_real = ovl_dir_is_real(file->f_path.dentry);
 	od->is_upper = OVL_TYPE_UPPER(type);
 	file->private_data = od;
 
-- 
2.7.4

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

* [PATCH 3/7] ovl: remove unneeded arg from ovl_verify_origin()
  2017-10-17 15:23 [PATCH 0/7] Implement overlayfs verify_dir mount option Amir Goldstein
  2017-10-17 15:23 ` [PATCH 1/7] ovl: fix rmdir problem on origin dir Amir Goldstein
  2017-10-17 15:23 ` [PATCH 2/7] ovl: no direct iteration for dir with copy up origin Amir Goldstein
@ 2017-10-17 15:23 ` Amir Goldstein
  2017-10-17 15:23 ` [PATCH 4/7] ovl: disable redirect_dir and index when no xattr support Amir Goldstein
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Amir Goldstein @ 2017-10-17 15:23 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

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

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 1005c4fa640c..e39d0df10099 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -350,8 +350,8 @@ static int ovl_verify_origin_fh(struct dentry *dentry, const struct ovl_fh *fh)
  *
  * Return 0 on match, -ESTALE on mismatch, < 0 on error.
  */
-int ovl_verify_origin(struct dentry *dentry, struct vfsmount *mnt,
-		      struct dentry *origin, bool is_upper, bool set)
+int ovl_verify_origin(struct dentry *dentry, struct dentry *origin,
+		      bool is_upper, bool set)
 {
 	struct inode *inode;
 	struct ovl_fh *fh;
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 2aef95047006..29f9a24eaa50 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -245,8 +245,8 @@ static inline bool ovl_is_impuredir(struct dentry *dentry)
 
 
 /* namei.c */
-int ovl_verify_origin(struct dentry *dentry, struct vfsmount *mnt,
-		      struct dentry *origin, bool is_upper, bool set);
+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_get_index_name(struct dentry *origin, struct qstr *name);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 092d150643c1..e2ff17d5022b 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1059,8 +1059,8 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 
 	if (!(ovl_force_readonly(ufs)) && ufs->config.index) {
 		/* Verify lower root is upper root origin */
-		err = ovl_verify_origin(upperpath.dentry, ufs->lower_mnt[0],
-					stack[0].dentry, false, true);
+		err = ovl_verify_origin(upperpath.dentry, stack[0].dentry,
+					false, true);
 		if (err) {
 			pr_err("overlayfs: failed to verify upper root origin\n");
 			goto out_put_lower_mnt;
@@ -1070,8 +1070,8 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 						   OVL_INDEXDIR_NAME, true);
 		if (ufs->indexdir) {
 			/* Verify upper root is index dir origin */
-			err = ovl_verify_origin(ufs->indexdir, ufs->upper_mnt,
-						upperpath.dentry, true, true);
+			err = ovl_verify_origin(ufs->indexdir, upperpath.dentry,
+						true, true);
 			if (err)
 				pr_err("overlayfs: failed to verify index dir origin\n");
 
-- 
2.7.4

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

* [PATCH 4/7] ovl: disable redirect_dir and index when no xattr support
  2017-10-17 15:23 [PATCH 0/7] Implement overlayfs verify_dir mount option Amir Goldstein
                   ` (2 preceding siblings ...)
  2017-10-17 15:23 ` [PATCH 3/7] ovl: remove unneeded arg from ovl_verify_origin() Amir Goldstein
@ 2017-10-17 15:23 ` Amir Goldstein
  2017-10-17 15:23 ` [PATCH 5/7] ovl: ignore redirect_dir and index when no upper layer Amir Goldstein
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Amir Goldstein @ 2017-10-17 15:23 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

Overlayfs falls back to index=off if lower/upper fs does not support
file handles. We should do the same if upper fs does not support xattr.

The redirect_dir feature is implicitly disabled when upper fs does not
support xattr via the check in ovl_redirect_dir(). Make the feature
explicitly disabled in this case by emitting a warning at mount time
and setting redirect_dir to off so its true state is visible in
/proc/mounts.

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

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index e2ff17d5022b..a908f4f2e7c3 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1009,7 +1009,9 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 					      "0", 1, 0);
 			if (err) {
 				ufs->noxattr = true;
-				pr_warn("overlayfs: upper fs does not support xattr.\n");
+				ufs->config.redirect_dir = false;
+				ufs->config.index = false;
+				pr_warn("overlayfs: upper fs does not support xattr, falling back to redirect_dir=off, index=off and no opaque dir.\n");
 			} else {
 				vfs_removexattr(ufs->workdir, OVL_XATTR_OPAQUE);
 			}
-- 
2.7.4

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

* [PATCH 5/7] ovl: ignore redirect_dir and index when no upper layer
  2017-10-17 15:23 [PATCH 0/7] Implement overlayfs verify_dir mount option Amir Goldstein
                   ` (3 preceding siblings ...)
  2017-10-17 15:23 ` [PATCH 4/7] ovl: disable redirect_dir and index when no xattr support Amir Goldstein
@ 2017-10-17 15:23 ` Amir Goldstein
  2017-10-17 15:23 ` [PATCH 6/7] ovl: add support for verify_dir mount option Amir Goldstein
  2017-10-17 15:23 ` [PATCH 7/7] ovl: follow decoded origin file handle of merge dir Amir Goldstein
  6 siblings, 0 replies; 10+ messages in thread
From: Amir Goldstein @ 2017-10-17 15:23 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

For non-upper overlay mount, the values of redirect_dir and index
config are moot. When mounter tries to change the default value
of these mount options, emit a warning and leave their values at
their defaults so they won't show up in /proc/mounts.

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

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index a908f4f2e7c3..f4c81767523f 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -434,12 +434,20 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 		}
 	}
 
-	/* Workdir is useless in non-upper mount */
-	if (!config->upperdir && config->workdir) {
-		pr_info("overlayfs: option \"workdir=%s\" is useless in a non-upper mount, ignore\n",
-			config->workdir);
-		kfree(config->workdir);
-		config->workdir = NULL;
+	if (!config->upperdir) {
+		/* Workdir is useless in non-upper mount */
+		if (config->workdir) {
+			pr_info("overlayfs: option \"workdir=%s\" is useless in a non-upper mount, ignore\n",
+				config->workdir);
+			kfree(config->workdir);
+			config->workdir = NULL;
+		}
+		if (config->redirect_dir != ovl_redirect_dir_def ||
+		    config->index != ovl_index_def) {
+			pr_info("overlayfs: options \"redirect_dir\" and \"index\" are useless in a non-upper mount, ignore\n");
+			config->redirect_dir = ovl_redirect_dir_def;
+			config->index = ovl_index_def;
+		}
 	}
 
 	return 0;
@@ -641,10 +649,12 @@ static int ovl_lower_dir(const char *name, struct path *path,
 		*remote = true;
 
 	/*
-	 * The inodes index feature needs to encode and decode file
-	 * handles, so it requires that all layers support them.
+	 * The inodes index feature needs to encode and decode lower file
+	 * handles stored in upper inodes, so it requires that all lower
+	 * layers support file handles.
 	 */
-	if (ofs->config.index && !ovl_can_decode_fh(path->dentry->d_sb)) {
+	if (ofs->config.upperdir && !ovl_can_decode_fh(path->dentry->d_sb) &&
+	    ofs->config.index) {
 		ofs->config.index = false;
 		pr_warn("overlayfs: fs on '%s' does not support file handles, falling back to index=off.\n", name);
 	}
-- 
2.7.4

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

* [PATCH 6/7] ovl: add support for verify_dir mount option
  2017-10-17 15:23 [PATCH 0/7] Implement overlayfs verify_dir mount option Amir Goldstein
                   ` (4 preceding siblings ...)
  2017-10-17 15:23 ` [PATCH 5/7] ovl: ignore redirect_dir and index when no upper layer Amir Goldstein
@ 2017-10-17 15:23 ` Amir Goldstein
  2017-10-17 15:23 ` [PATCH 7/7] ovl: follow decoded origin file handle of merge dir Amir Goldstein
  6 siblings, 0 replies; 10+ messages in thread
From: Amir Goldstein @ 2017-10-17 15:23 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

When overlayfs is mounted with option 'verify_dir', a directory inode
found in lower layer by name or by redirect_dir is verified against the
file handle of the copy up origin that is stored in the upper layer.
If origin xattr does not exist, update it with the lower found by name.

This option introduces a change of behavior for the case of lower layer
modification while overlay is offline. A lower directory created or
moved offline under an exisitng upper directory, will not be merged with
that upper directory when the overlay in mounted with 'verify_dir'.

The 'verify_dir' option should not be used after copying layers,
because the new lower directory inodes would fail verification.

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

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 Documentation/filesystems/overlayfs.txt | 16 +++++++++++++
 fs/overlayfs/namei.c                    | 12 ++++++++++
 fs/overlayfs/overlayfs.h                |  1 +
 fs/overlayfs/ovl_entry.h                |  1 +
 fs/overlayfs/super.c                    | 42 ++++++++++++++++++++++-----------
 fs/overlayfs/util.c                     |  7 ++++++
 6 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
index 8caa60734647..17a3c9196fd3 100644
--- a/Documentation/filesystems/overlayfs.txt
+++ b/Documentation/filesystems/overlayfs.txt
@@ -265,6 +265,22 @@ filesystem are not allowed.  If the underlying filesystem is changed,
 the behavior of the overlay is undefined, though it will not result in
 a crash or deadlock.
 
+When the underlying filesystems supports NFS export, overlay mount can be
+made more resilient to offline and online changes of the underlying lower
+layer by enabling the "verify_dir" feature.
+
+On every copy_up, an NFS file handle of the lower inode, along with the
+UUID of the lower filesystem, are encoded and stored in an extended
+attribute "trusted.overlay.origin" on the upper inode.
+
+With the "verify_dir" feature, a lookup of a merged directory, that
+found a lower directory at the lookup path or at the path pointed to by
+the "trusted.overlay.redirect" extended attribute, will verify that the
+found lower directory file handle and lower filesystem UUID match the
+origin file handle that was stored at copy_up time.  If a found lower
+directory does not match the stored origin, that directory will be not be
+merged with the upper directory.
+
 Testsuite
 ---------
 
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index e39d0df10099..988cb5496474 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -666,6 +666,18 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		if (!this)
 			continue;
 
+		/*
+		 * Verify that uppermost lower matches the copy up origin fh.
+		 * If no origin fh is stored in upper, store fh of lower dir.
+		 */
+		if (this && upperdentry && !ctr) {
+			err = ovl_verify_origin(upperdentry, this, false, true);
+			if (err && ovl_verify_dir(dentry->d_sb)) {
+				dput(this);
+				break;
+			}
+		}
+
 		stack[ctr].dentry = this;
 		stack[ctr].mnt = lowerpath.mnt;
 		ctr++;
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 29f9a24eaa50..ba455642152f 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -212,6 +212,7 @@ void ovl_dentry_set_opaque(struct dentry *dentry);
 bool ovl_dentry_has_upper_alias(struct dentry *dentry);
 void ovl_dentry_set_upper_alias(struct dentry *dentry);
 bool ovl_redirect_dir(struct super_block *sb);
+bool ovl_verify_dir(struct super_block *sb);
 const char *ovl_dentry_get_redirect(struct dentry *dentry);
 void ovl_dentry_set_redirect(struct dentry *dentry, const char *redirect);
 void ovl_inode_init(struct inode *inode, struct dentry *upperdentry,
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 25d9b5adcd42..4c3d5ff1a30c 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -14,6 +14,7 @@ struct ovl_config {
 	char *workdir;
 	bool default_permissions;
 	bool redirect_dir;
+	bool verify_dir;
 	bool index;
 };
 
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index f4c81767523f..aa230efc1afe 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -303,6 +303,8 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 	if (ufs->config.index != ovl_index_def)
 		seq_printf(m, ",index=%s",
 			   ufs->config.index ? "on" : "off");
+	if (ufs->config.verify_dir)
+		seq_puts(m, ",verify_dir");
 	return 0;
 }
 
@@ -336,6 +338,7 @@ enum {
 	OPT_REDIRECT_DIR_OFF,
 	OPT_INDEX_ON,
 	OPT_INDEX_OFF,
+	OPT_VERIFY_DIR,
 	OPT_ERR,
 };
 
@@ -348,6 +351,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_VERIFY_DIR,		"verify_dir"},
 	{OPT_ERR,			NULL}
 };
 
@@ -428,6 +432,10 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 			config->index = false;
 			break;
 
+		case OPT_VERIFY_DIR:
+			config->verify_dir = true;
+			break;
+
 		default:
 			pr_err("overlayfs: unrecognized mount option \"%s\" or missing value\n", p);
 			return -EINVAL;
@@ -443,10 +451,11 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 			config->workdir = NULL;
 		}
 		if (config->redirect_dir != ovl_redirect_dir_def ||
-		    config->index != ovl_index_def) {
-			pr_info("overlayfs: options \"redirect_dir\" and \"index\" are useless in a non-upper mount, ignore\n");
+		    config->index != ovl_index_def || config->verify_dir) {
+			pr_info("overlayfs: options \"redirect_dir\", \"verify_dir\" and \"index\" are useless in a non-upper mount, ignore\n");
 			config->redirect_dir = ovl_redirect_dir_def;
 			config->index = ovl_index_def;
+			config->verify_dir = false;
 		}
 	}
 
@@ -649,14 +658,16 @@ static int ovl_lower_dir(const char *name, struct path *path,
 		*remote = true;
 
 	/*
-	 * The inodes index feature needs to encode and decode lower file
+	 * The features index and verify_dir need to decode lower file
 	 * handles stored in upper inodes, so it requires that all lower
 	 * layers support file handles.
 	 */
 	if (ofs->config.upperdir && !ovl_can_decode_fh(path->dentry->d_sb) &&
-	    ofs->config.index) {
+	    (ofs->config.index || ofs->config.verify_dir)) {
 		ofs->config.index = false;
-		pr_warn("overlayfs: fs on '%s' does not support file handles, falling back to index=off.\n", name);
+		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);
 	}
 
 	return 0;
@@ -1021,7 +1032,8 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 				ufs->noxattr = true;
 				ufs->config.redirect_dir = false;
 				ufs->config.index = false;
-				pr_warn("overlayfs: upper fs does not support xattr, falling back to redirect_dir=off, index=off and no opaque dir.\n");
+				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 {
 				vfs_removexattr(ufs->workdir, OVL_XATTR_OPAQUE);
 			}
@@ -1070,14 +1082,6 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		ufs->same_sb = NULL;
 
 	if (!(ovl_force_readonly(ufs)) && ufs->config.index) {
-		/* Verify lower root is upper root origin */
-		err = ovl_verify_origin(upperpath.dentry, stack[0].dentry,
-					false, true);
-		if (err) {
-			pr_err("overlayfs: failed to verify upper root origin\n");
-			goto out_put_lower_mnt;
-		}
-
 		ufs->indexdir = ovl_workdir_create(sb, ufs, workpath.dentry,
 						   OVL_INDEXDIR_NAME, true);
 		if (ufs->indexdir) {
@@ -1103,6 +1107,16 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	if (!ufs->indexdir)
 		ufs->config.index = false;
 
+	if (ufs->config.verify_dir || ufs->indexdir) {
+		/* Verify lower root is upper root origin */
+		err = ovl_verify_origin(upperpath.dentry, stack[0].dentry,
+					false, true);
+		if (err) {
+			pr_err("overlayfs: failed to verify upper root origin\n");
+			goto out_put_indexdir;
+		}
+	}
+
 	if (remote)
 		sb->s_d_op = &ovl_reval_dentry_operations;
 	else
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 51ca8bd16009..7355c51ae05f 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -234,6 +234,13 @@ bool ovl_redirect_dir(struct super_block *sb)
 	return ofs->config.redirect_dir && !ofs->noxattr;
 }
 
+bool ovl_verify_dir(struct super_block *sb)
+{
+	struct ovl_fs *ofs = sb->s_fs_info;
+
+	return ofs->config.verify_dir && !ofs->noxattr;
+}
+
 const char *ovl_dentry_get_redirect(struct dentry *dentry)
 {
 	return OVL_I(d_inode(dentry))->redirect;
-- 
2.7.4

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

* [PATCH 7/7] ovl: follow decoded origin file handle of merge dir
  2017-10-17 15:23 [PATCH 0/7] Implement overlayfs verify_dir mount option Amir Goldstein
                   ` (5 preceding siblings ...)
  2017-10-17 15:23 ` [PATCH 6/7] ovl: add support for verify_dir mount option Amir Goldstein
@ 2017-10-17 15:23 ` Amir Goldstein
  6 siblings, 0 replies; 10+ messages in thread
From: Amir Goldstein @ 2017-10-17 15:23 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs

When 'verify_dir' feature is enabled, if lower dir does not match the
origin fh stored in upper dir, or if no lower dir was found by name,
try to decode the stored origin fh and use the result as the merge dir
lower dentry.

When there is more than a single lower layer, a lower merge directory
followed from upper dir by origin fh is not followed down again to lower
layers by name.

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

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 Documentation/filesystems/overlayfs.txt |  6 +++++-
 fs/dcache.c                             |  1 +
 fs/overlayfs/namei.c                    | 37 ++++++++++++++++++++++++++++++---
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
index 17a3c9196fd3..d89041baceb9 100644
--- a/Documentation/filesystems/overlayfs.txt
+++ b/Documentation/filesystems/overlayfs.txt
@@ -279,7 +279,11 @@ the "trusted.overlay.redirect" extended attribute, will verify that the
 found lower directory file handle and lower filesystem UUID match the
 origin file handle that was stored at copy_up time.  If a found lower
 directory does not match the stored origin, that directory will be not be
-merged with the upper directory.
+merged with the upper directory.  If the stored origin file handle can be
+decoded to an existing lower directory, the decoded directory will be
+merged with the upper directory instead.  The "verify_dir" feature,
+therefore, makes an overlay mount cope better with the situations of
+lower directory rename and delete.
 
 Testsuite
 ---------
diff --git a/fs/dcache.c b/fs/dcache.c
index f90141387f01..f61e03a38a30 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -3529,6 +3529,7 @@ bool is_subdir(struct dentry *new_dentry, struct dentry *old_dentry)
 
 	return result;
 }
+EXPORT_SYMBOL(is_subdir);
 
 static enum d_walk_ret d_genocide_kill(void *data, struct dentry *dentry)
 {
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 988cb5496474..5de7aa60a7a6 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -83,9 +83,21 @@ static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
 	goto err_free;
 }
 
-static int ovl_acceptable(void *ctx, struct dentry *dentry)
+static int ovl_acceptable(void *mnt, struct dentry *dentry)
 {
-	return 1;
+	/*
+	 * A non-dir origin may be disconnected, which is fine, because
+	 * we only need it for its unique inode number.
+	 */
+	if (!d_is_dir(dentry))
+		return 1;
+
+	/* Don't want to follow a deleted empty lower directory */
+	if (d_unhashed(dentry))
+		return 0;
+
+	/* Check if directory belongs to the layer mounted at mnt */
+	return is_subdir(dentry, ((struct vfsmount *)mnt)->mnt_root);
 }
 
 static struct ovl_fh *ovl_get_origin_fh(struct dentry *dentry)
@@ -160,7 +172,7 @@ static struct dentry *ovl_get_origin(struct dentry *dentry,
 	bytes = (fh->len - offsetof(struct ovl_fh, fid));
 	origin = exportfs_decode_fh(mnt, (struct fid *)fh->fid,
 				    bytes >> 2, (int)fh->type,
-				    ovl_acceptable, NULL);
+				    ovl_acceptable, mnt);
 	if (IS_ERR(origin)) {
 		/* Treat stale file handle as "origin unknown" */
 		if (origin == ERR_PTR(-ESTALE))
@@ -697,6 +709,25 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		}
 	}
 
+	/*
+	 * If lower not found by name or couldn't verify origin fh, try to
+	 * follow the decoded origin fh in upper to the first lower dir.
+	 */
+	if (!d.stop && poe->numlower && upperdentry && !ctr &&
+	    ovl_verify_dir(dentry->d_sb)) {
+		err = ovl_check_origin(upperdentry, roe->lowerstack,
+				       roe->numlower, &stack, &ctr);
+		if (err)
+			goto out_put;
+		/*
+		 * XXX: We do not continue layers lookup from decoded origin for
+		 * more than a single lower layer. This would require setting
+		 * d.redirect to decoded origin path and jump back to the
+		 * lowerstack layers lookup loop with 'i' set to the root layer
+		 * number where we found the decoded origin.
+		 */
+	}
+
 	/* 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;
-- 
2.7.4

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

* Re: [PATCH 1/7] ovl: fix rmdir problem on origin dir
  2017-10-17 15:23 ` [PATCH 1/7] ovl: fix rmdir problem on origin dir Amir Goldstein
@ 2017-10-31 12:54   ` Miklos Szeredi
  0 siblings, 0 replies; 10+ messages in thread
From: Miklos Szeredi @ 2017-10-31 12:54 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: linux-unionfs@vger.kernel.org, zhangyi

On Tue, Oct 17, 2017 at 5:23 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> If an "origin && not merged" upper dir have leftover whiteouts
> (last mount created), ovl do not clear this dir when we
> deleting it, which may lead to rmdir fail or temp file left
> in workdir.
>
> Simple reproducer:
>   mkdir lower upper work merge
>   mkdir -p lower/dir
>   touch lower/dir/a
>   mount -t overlay overlay -olowerdir=lower,upperdir=upper,\
>     workdir=work merge
>   rm merge/dir/a
>   umount merge
>   rm -rf lower/*
>   touch lower/dir  (*)
>   mount -t overlay overlay -olowerdir=lower,upperdir=upper,\
>     workdir=work merge
>   rm -rf merge/dir
>
> Syslog dump:
>   overlayfs: cleanup of 'work/#7' failed (-39)
>
> (*): if we do not creat the regular file, the result is different:
>   rm: cannot remove "dir/": Directory not empty
>
> This patch add explicitly check of OVL_XATTR_ORIGIN, and filter
> out whiteouts in upperdentry in ovl_check_empty_dir(). Finally,
> check and clear the dir when deleting it.

I suggest to split this into two:

1) filter out whiteouts on upper

2) add checks for OVL_XATTR_ORIGIN on directories

>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/overlayfs/dir.c       | 21 +++++++++++++++------
>  fs/overlayfs/namei.c     |  7 +++++--
>  fs/overlayfs/overlayfs.h |  3 ++-
>  fs/overlayfs/readdir.c   | 30 ++++++++++++++++++++++--------
>  fs/overlayfs/util.c      | 13 +++++++++++++
>  5 files changed, 57 insertions(+), 17 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index cc961a3bd3bd..d916fc19e724 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -181,6 +181,14 @@ static bool ovl_type_origin(struct dentry *dentry)
>         return OVL_TYPE_ORIGIN(ovl_path_type(dentry));
>  }
>
> +static bool ovl_is_origin(struct dentry *dentry)
> +{
> +       if (!OVL_TYPE_UPPER(ovl_path_type(dentry)))
> +               return false;
> +
> +       return ovl_check_origin_xattr(ovl_dentry_upper(dentry));
> +}
> +

^ this goes into #2.

>  static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
>                             struct cattr *attr, struct dentry *hardlink)
>  {
> @@ -300,7 +308,6 @@ static struct dentry *ovl_check_empty_and_clear(struct dentry *dentry)
>  {
>         int err;
>         struct dentry *ret = NULL;
> -       enum ovl_path_type type = ovl_path_type(dentry);
>         LIST_HEAD(list);
>
>         err = ovl_check_empty_dir(dentry, &list);
> @@ -313,13 +320,13 @@ static struct dentry *ovl_check_empty_and_clear(struct dentry *dentry)
>          * When removing an empty opaque directory, then it makes no sense to
>          * replace it with an exact replica of itself.
>          *
> -        * If no upperdentry then skip clearing whiteouts.
> +        * If upperdentry has whiteouts, clear them.
>          *
>          * Can race with copy-up, since we don't hold the upperdir mutex.
>          * Doesn't matter, since copy-up can't create a non-empty directory
>          * from an empty one.
>          */
> -       if (OVL_TYPE_UPPER(type) && OVL_TYPE_MERGE(type))
> +       if (!list_empty(&list))
>                 ret = ovl_clear_empty(dentry, &list);
>

^ into #1.

>  out_free:
> @@ -698,8 +705,9 @@ static int ovl_remove_upper(struct dentry *dentry, bool is_dir)
>         struct dentry *opaquedir = NULL;
>         int err;
>
> -       /* Redirect dir can be !ovl_lower_positive && OVL_TYPE_MERGE */
> -       if (is_dir && ovl_dentry_get_redirect(dentry)) {
> +       /* Redirect/origin dir can be !ovl_lower_positive && not clean */
> +       if (is_dir && (ovl_dentry_get_redirect(dentry) ||
> +                      ovl_is_origin(dentry))) {
>                 opaquedir = ovl_check_empty_and_clear(dentry);
>                 err = PTR_ERR(opaquedir);
>                 if (IS_ERR(opaquedir))
> @@ -946,7 +954,8 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
>
>         old_cred = ovl_override_creds(old->d_sb);
>
> -       if (overwrite && new_is_dir && ovl_type_merge_or_lower(new)) {
> +       if (overwrite && new_is_dir && (ovl_type_merge_or_lower(new) ||
> +                                       ovl_is_origin(new))) {
>                 opaquedir = ovl_check_empty_and_clear(new);
>                 err = PTR_ERR(opaquedir);
>                 if (IS_ERR(opaquedir)) {

^ into #2.

> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index 654bea1a5ac9..1005c4fa640c 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -557,18 +557,21 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,
>   * Returns next layer in stack starting from top.
>   * Returns -1 if this is the last layer.
>   */
> -int ovl_path_next(int idx, struct dentry *dentry, struct path *path)
> +int ovl_path_next(int idx, struct dentry *dentry, struct path *path, int *idxp)
>  {
>         struct ovl_entry *oe = dentry->d_fsdata;
>
>         BUG_ON(idx < 0);
>         if (idx == 0) {
>                 ovl_path_upper(dentry, path);
> -               if (path->dentry)
> +               if (path->dentry) {
> +                       *idxp = 0;
>                         return oe->numlower ? 1 : -1;
> +               }
>                 idx++;
>         }
>         BUG_ON(idx > oe->numlower);
> +       *idxp = idx;
>         *path = oe->lowerstack[idx - 1];
>
>         return (idx < oe->numlower) ? idx + 1 : -1;

Not necessary.  See below.

> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index c706a6f99928..2aef95047006 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -223,6 +223,7 @@ bool ovl_is_whiteout(struct dentry *dentry);
>  struct file *ovl_path_open(struct path *path, int flags);
>  int ovl_copy_up_start(struct dentry *dentry);
>  void ovl_copy_up_end(struct dentry *dentry);
> +bool ovl_check_origin_xattr(struct dentry *dentry);
>  bool ovl_check_dir_xattr(struct dentry *dentry, const char *name);
>  int ovl_check_setxattr(struct dentry *dentry, struct dentry *upperdentry,
>                        const char *name, const void *value, size_t size,
> @@ -249,7 +250,7 @@ int ovl_verify_origin(struct dentry *dentry, struct vfsmount *mnt,
>  int ovl_verify_index(struct dentry *index, 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 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);
>  bool ovl_lower_positive(struct dentry *dentry);
>
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index 0f85ee9c3268..55623b6635b6 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -26,6 +26,7 @@ struct ovl_cache_entry {
>         struct list_head l_node;
>         struct rb_node node;
>         struct ovl_cache_entry *next_maybe_whiteout;
> +       int idx;

Change this to "bool is_upper".

>         bool is_whiteout;
>         char name[];
>  };
> @@ -46,6 +47,7 @@ struct ovl_readdir_data {
>         struct list_head middle;
>         struct ovl_cache_entry *first_maybe_whiteout;
>         int count;
> +       int idx;

Not necessary: we already have "is_upper" that carries the info we are
interested in.

>         int err;
>         bool is_upper;
>         bool d_type_supported;
> @@ -159,6 +161,7 @@ static struct ovl_cache_entry *ovl_cache_entry_new(struct ovl_readdir_data *rdd,
>         if (ovl_calc_d_ino(rdd, p))
>                 p->ino = 0;
>         p->is_whiteout = false;
> +       p->idx = rdd->idx;
>
>         if (d_type == DT_CHR) {
>                 p->next_maybe_whiteout = rdd->first_maybe_whiteout;
> @@ -348,7 +351,7 @@ static int ovl_dir_read_merged(struct dentry *dentry, struct list_head *list,
>         int idx, next;
>
>         for (idx = 0; idx != -1; idx = next) {
> -               next = ovl_path_next(idx, dentry, &realpath);
> +               next = ovl_path_next(idx, dentry, &realpath, &rdd.idx);
>                 rdd.is_upper = ovl_dentry_upper(dentry) == realpath.dentry;
>
>                 if (next != -1) {
> @@ -835,7 +838,7 @@ const struct file_operations ovl_dir_operations = {
>  int ovl_check_empty_dir(struct dentry *dentry, struct list_head *list)
>  {
>         int err;
> -       struct ovl_cache_entry *p;
> +       struct ovl_cache_entry *p, *n;
>         struct rb_root root = RB_ROOT;
>
>         err = ovl_dir_read_merged(dentry, list, &root);
> @@ -844,18 +847,29 @@ int ovl_check_empty_dir(struct dentry *dentry, struct list_head *list)
>
>         err = 0;
>
> -       list_for_each_entry(p, list, l_node) {
> -               if (p->is_whiteout)
> -                       continue;
> +       list_for_each_entry_safe(p, n, list, l_node) {
> +               /*
> +                * Select whiteouts in upperdir, they should
> +                * be cleared when deleting this directory.
> +                */
> +               if (p->is_whiteout) {
> +                       if (p->idx == 0)
> +                               continue;
> +                       goto del_entry;
> +               }
>
>                 if (p->name[0] == '.') {
>                         if (p->len == 1)
> -                               continue;
> +                               goto del_entry;
>                         if (p->len == 2 && p->name[1] == '.')
> -                               continue;
> +                               goto del_entry;
>                 }
>                 err = -ENOTEMPTY;
>                 break;
> +
> +del_entry:
> +               list_del(&p->l_node);
> +               kfree(p);
>         }
>

Into #1.

>         return err;
> @@ -869,7 +883,7 @@ void ovl_cleanup_whiteouts(struct dentry *upper, struct list_head *list)
>         list_for_each_entry(p, list, l_node) {
>                 struct dentry *dentry;
>
> -               if (!p->is_whiteout)
> +               if (!p->is_whiteout || p->idx != 0)
>                         continue;

Haven't we already thrown out all non-upper whiteouts?

>
>                 dentry = lookup_one_len(p->name, upper, p->len);
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index b9b239fa5cfd..51ca8bd16009 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -329,6 +329,19 @@ void ovl_copy_up_end(struct dentry *dentry)
>         mutex_unlock(&OVL_I(d_inode(dentry))->lock);
>  }
>
> +bool ovl_check_origin_xattr(struct dentry *dentry)
> +{
> +       int res;
> +
> +       res = vfs_getxattr(dentry, OVL_XATTR_ORIGIN, NULL, 0);
> +
> +       /* Zero size value means "copied up but origin unknown" */
> +       if (res >= 0)
> +               return true;
> +
> +       return false;
> +}
> +
>  bool ovl_check_dir_xattr(struct dentry *dentry, const char *name)
>  {
>         int res;

Into #2.

> --
> 2.7.4
>

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

* Re: [PATCH 2/7] ovl: no direct iteration for dir with copy up origin
  2017-10-17 15:23 ` [PATCH 2/7] ovl: no direct iteration for dir with copy up origin Amir Goldstein
@ 2017-10-31 13:08   ` Miklos Szeredi
  0 siblings, 0 replies; 10+ messages in thread
From: Miklos Szeredi @ 2017-10-31 13:08 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: linux-unionfs@vger.kernel.org

On Tue, Oct 17, 2017 at 5:23 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> If a non-merge dir in an overlay mount has an overlay.origin xattr, it
> means it was once an upper merge dir, which may contain whiteouts and
> then the lower dir was removed under it.
>
> Do not iterate real dir directly in this case to avoid exposing whiteouts.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/overlayfs/readdir.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index 55623b6635b6..ba44546ad1ed 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -319,21 +319,42 @@ static inline int ovl_dir_read(struct path *realpath,
>         return err;
>  }
>
> +/* Can we iterate real dir directly? */
> +static bool ovl_dir_is_real(struct dentry *dir)
> +{
> +       struct path realpath;
> +       enum ovl_path_type type = ovl_path_real(dir, &realpath);
> +
> +       if (OVL_TYPE_MERGE(type))
> +               return false;
> +
> +       /*
> +        * Non-merge dir may contain whiteouts from a time it was a merge
> +        * upper, before lower dir was removed under it and possibly before
> +        * it was rotated from upper to lower layer.
> +        */
> +       return !ovl_check_origin_xattr(realpath.dentry);

This patch is okay, but I started wondering if
ovl_check_origin_xattr() should cache the "has origin" bit for
directories.   Can be a followup patch, or we can leave it later.
Just thought I'd mention it as something to think about.

Thanks,
Miklos

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

end of thread, other threads:[~2017-10-31 13:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-17 15:23 [PATCH 0/7] Implement overlayfs verify_dir mount option Amir Goldstein
2017-10-17 15:23 ` [PATCH 1/7] ovl: fix rmdir problem on origin dir Amir Goldstein
2017-10-31 12:54   ` Miklos Szeredi
2017-10-17 15:23 ` [PATCH 2/7] ovl: no direct iteration for dir with copy up origin Amir Goldstein
2017-10-31 13:08   ` Miklos Szeredi
2017-10-17 15:23 ` [PATCH 3/7] ovl: remove unneeded arg from ovl_verify_origin() Amir Goldstein
2017-10-17 15:23 ` [PATCH 4/7] ovl: disable redirect_dir and index when no xattr support Amir Goldstein
2017-10-17 15:23 ` [PATCH 5/7] ovl: ignore redirect_dir and index when no upper layer Amir Goldstein
2017-10-17 15:23 ` [PATCH 6/7] ovl: add support for verify_dir mount option Amir Goldstein
2017-10-17 15:23 ` [PATCH 7/7] ovl: follow decoded origin file handle of merge dir Amir Goldstein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).