linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miklos Szeredi <mszeredi@redhat.com>
To: linux-unionfs@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 24/28] ovl: Check redirect on index as well
Date: Tue, 29 May 2018 16:46:08 +0200	[thread overview]
Message-ID: <20180529144612.16675-25-mszeredi@redhat.com> (raw)
In-Reply-To: <20180529144612.16675-1-mszeredi@redhat.com>

From: Vivek Goyal <vgoyal@redhat.com>

Right now we seem to check redirect only if upperdentry is found.  But it
is possible that there is no upperdentry but later we found an index.

We need to check redirect on index as well and set it in
ovl_inode->redirect.  Otherwise link code can assume that dentry does not
have redirect and place a new one which breaks things.  In my testing
overlay/033 test started failing in xfstests.  Following are the details.

For example do following.

$ mkdir lower upper work merged

 - Make lower dir with 4 links.
  $ echo "foo" > lower/l0.txt
  $ ln  lower/l0.txt lower/l1.txt
  $ ln  lower/l0.txt lower/l2.txt
  $ ln  lower/l0.txt lower/l3.txt

 - Mount with index on and metacopy on.

  $ mount -t overlay -o lowerdir=lower,upperdir=upper,workdir=work,\
                        index=on,metacopy=on none merged

 - Link lower

  $ ln merged/l0.txt merged/l4.txt
    (This will metadata copy up of l0.txt and put an absolute redirect
     /l0.txt)

  $ echo 2 > /proc/sys/vm/drop/caches

  $ ls merged/l1.txt
  (Now l1.txt will be looked up.  There is no upper dentry but there is
   lower dentry and index will be found.  We don't check for redirect on
   index, hence ovl_inode->redirect will be NULL.)

 - Link Upper

  $ ln merged/l4.txt merged/l5.txt
  (Lookup of l4.txt will use inode from l1.txt lookup which is still in
   cache.  It has ovl_inode->redirect NULL, hence link will put a new
   redirect and replace /l0.txt with /l4.txt

 - Drop caches.
  echo 2 > /proc/sys/vm/drop_caches

 - List l1.txt and it returns -ESTALE

  $ ls merged/l0.txt

  (It returns stale because, we found a metacopy of l0.txt in upper and it
   has redirect l4.txt but there is no file named l4.txt in lower layer.
   So lower data copy is not found and -ESTALE is returned.)

So problem here is that we did not process redirect on index.  Check
redirect on index as well and then problem is fixed.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/overlayfs/namei.c     | 50 +++++++++++++-----------------------------------
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/util.c      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 37 deletions(-)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index e38fa61e08df..2acd494ea9f0 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -31,32 +31,13 @@ static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
 			      size_t prelen, const char *post)
 {
 	int res;
-	char *s, *next, *buf = NULL;
+	char *buf;
 
-	res = vfs_getxattr(dentry, OVL_XATTR_REDIRECT, NULL, 0);
-	if (res < 0) {
-		if (res == -ENODATA || res == -EOPNOTSUPP)
-			return 0;
-		goto fail;
-	}
-	buf = kzalloc(prelen + res + strlen(post) + 1, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
+	buf = ovl_get_redirect_xattr(dentry, prelen + strlen(post));
+	if (IS_ERR_OR_NULL(buf))
+		return PTR_ERR(buf);
 
-	if (res == 0)
-		goto invalid;
-
-	res = vfs_getxattr(dentry, OVL_XATTR_REDIRECT, buf, res);
-	if (res < 0)
-		goto fail;
-	if (res == 0)
-		goto invalid;
 	if (buf[0] == '/') {
-		for (s = buf; *s++ == '/'; s = next) {
-			next = strchrnul(s, '/');
-			if (s == next)
-				goto invalid;
-		}
 		/*
 		 * One of the ancestor path elements in an absolute path
 		 * lookup in ovl_lookup_layer() could have been opaque and
@@ -67,9 +48,7 @@ static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
 		 */
 		d->stop = false;
 	} else {
-		if (strchr(buf, '/') != NULL)
-			goto invalid;
-
+		res = strlen(buf) + 1;
 		memmove(buf + prelen, buf, res);
 		memcpy(buf, d->name.name, prelen);
 	}
@@ -81,16 +60,6 @@ static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
 	d->name.len = strlen(d->redirect);
 
 	return 0;
-
-err_free:
-	kfree(buf);
-	return 0;
-fail:
-	pr_warn_ratelimited("overlayfs: failed to get redirect (%i)\n", res);
-	goto err_free;
-invalid:
-	pr_warn_ratelimited("overlayfs: invalid redirect (%s)\n", buf);
-	goto err_free;
 }
 
 static int ovl_acceptable(void *ctx, struct dentry *dentry)
@@ -1071,8 +1040,15 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 
 	if (upperdentry)
 		ovl_dentry_set_upper_alias(dentry);
-	else if (index)
+	else if (index) {
 		upperdentry = dget(index);
+		upperredirect = ovl_get_redirect_xattr(upperdentry, 0);
+		if (IS_ERR(upperredirect)) {
+			err = PTR_ERR(upperredirect);
+			upperredirect = NULL;
+			goto out_free_oe;
+		}
+	}
 
 	if (upperdentry || ctr) {
 		struct ovl_inode_params oip = {
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index bde352e414e7..24f1d0e8a178 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -276,6 +276,7 @@ void ovl_nlink_end(struct dentry *dentry, bool locked);
 int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir);
 int ovl_check_metacopy_xattr(struct dentry *dentry);
 bool ovl_is_metacopy_dentry(struct dentry *dentry);
+char *ovl_get_redirect_xattr(struct dentry *dentry, int padding);
 
 static inline bool ovl_is_impuredir(struct dentry *dentry)
 {
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 1aa9e0c5a327..8cfb62cc8672 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -865,3 +865,53 @@ bool ovl_is_metacopy_dentry(struct dentry *dentry)
 
 	return (oe->numlower > 1);
 }
+
+char *ovl_get_redirect_xattr(struct dentry *dentry, int padding)
+{
+	int res;
+	char *s, *next, *buf = NULL;
+
+	res = vfs_getxattr(dentry, OVL_XATTR_REDIRECT, NULL, 0);
+	if (res < 0) {
+		if (res == -ENODATA || res == -EOPNOTSUPP)
+			return NULL;
+		goto fail;
+	}
+
+	buf = kzalloc(res + padding + 1, GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
+	if (res == 0)
+		goto invalid;
+
+	res = vfs_getxattr(dentry, OVL_XATTR_REDIRECT, buf, res);
+	if (res < 0)
+		goto fail;
+	if (res == 0)
+		goto invalid;
+
+	if (buf[0] == '/') {
+		for (s = buf; *s++ == '/'; s = next) {
+			next = strchrnul(s, '/');
+			if (s == next)
+				goto invalid;
+		}
+	} else {
+		if (strchr(buf, '/') != NULL)
+			goto invalid;
+	}
+
+	return buf;
+
+err_free:
+	kfree(buf);
+	return ERR_PTR(res);
+fail:
+	pr_warn_ratelimited("overlayfs: failed to get redirect (%i)\n", res);
+	goto err_free;
+invalid:
+	pr_warn_ratelimited("overlayfs: invalid redirect (%s)\n", buf);
+	res = -EINVAL;
+	goto err_free;
+}
-- 
2.14.3

  parent reply	other threads:[~2018-05-29 14:46 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-29 14:45 [PATCH 00/28] overlayfs: Delayed copy up of data Miklos Szeredi
2018-05-29 14:45 ` [PATCH 01/28] ovl: Initialize ovl_inode->redirect in ovl_get_inode() Miklos Szeredi
2018-05-29 14:45 ` [PATCH 02/28] ovl: Move the copy up helpers to copy_up.c Miklos Szeredi
2018-05-29 14:45 ` [PATCH 03/28] ovl: Provide a mount option metacopy=on/off for metadata copyup Miklos Szeredi
2018-05-29 20:44   ` Randy Dunlap
2018-05-30  8:27     ` Miklos Szeredi
2018-05-29 14:45 ` [PATCH 04/28] ovl: During copy up, first copy up metadata and then data Miklos Szeredi
2018-05-29 14:45 ` [PATCH 05/28] ovl: Copy up only metadata during copy up where it makes sense Miklos Szeredi
2018-05-29 14:45 ` [PATCH 06/28] ovl: Add helper ovl_already_copied_up() Miklos Szeredi
2018-05-29 14:45 ` [PATCH 07/28] ovl: A new xattr OVL_XATTR_METACOPY for file on upper Miklos Szeredi
2018-05-29 14:45 ` [PATCH 08/28] ovl: Use out_err instead of out_nomem Miklos Szeredi
2018-05-29 14:45 ` [PATCH 09/28] ovl: Modify ovl_lookup() and friends to lookup metacopy dentry Miklos Szeredi
2018-05-29 14:45 ` [PATCH 10/28] ovl: Copy up meta inode data from lowest data inode Miklos Szeredi
2018-05-29 14:45 ` [PATCH 11/28] ovl: Add helper ovl_dentry_lowerdata() to get lower data dentry Miklos Szeredi
2018-05-29 14:45 ` [PATCH 12/28] ovl: Fix ovl_getattr() to get number of blocks from lower Miklos Szeredi
2018-05-29 14:45 ` [PATCH 13/28] ovl: Store lower data inode in ovl_inode Miklos Szeredi
2018-05-29 14:45 ` [PATCH 14/28] ovl: Add helper ovl_inode_realdata() Miklos Szeredi
2018-05-29 14:45 ` [PATCH 15/28] ovl: Open file with data except for the case of fsync Miklos Szeredi
2018-05-30 14:30   ` Vivek Goyal
2018-05-30 15:12     ` Miklos Szeredi
2018-05-30 15:48       ` Vivek Goyal
2018-05-29 14:46 ` [PATCH 16/28] ovl: Do not expose metacopy only dentry from d_real() Miklos Szeredi
2018-05-30 21:05   ` Vivek Goyal
2018-05-31  4:30     ` Amir Goldstein
2018-05-29 14:46 ` [PATCH 17/28] ovl: Move some dir related ovl_lookup_single() code in else block Miklos Szeredi
2018-05-29 14:46 ` [PATCH 18/28] ovl: Check redirects for metacopy files Miklos Szeredi
2018-05-29 14:46 ` [PATCH 19/28] ovl: Treat metacopy dentries as type OVL_PATH_MERGE Miklos Szeredi
2018-05-29 14:46 ` [PATCH 20/28] ovl: Add an inode flag OVL_CONST_INO Miklos Szeredi
2018-05-29 14:46 ` [PATCH 21/28] ovl: Do not set dentry type ORIGIN for broken hardlinks Miklos Szeredi
2018-05-29 14:46 ` [PATCH 22/28] ovl: Set redirect on metacopy files upon rename Miklos Szeredi
2018-05-29 14:46 ` [PATCH 23/28] ovl: Set redirect on upper inode when it is linked Miklos Szeredi
2018-05-29 14:46 ` Miklos Szeredi [this message]
2018-05-29 14:46 ` [PATCH 25/28] ovl: Disbale metacopy for MAP_SHARED mmap() Miklos Szeredi
2018-05-29 14:46 ` [PATCH 26/28] ovl: Do not do metadata only copy-up for truncate operation Miklos Szeredi
2018-05-29 14:46 ` [PATCH 27/28] ovl: Do not do metacopy only for ioctl modifying file attr Miklos Szeredi
2018-05-29 14:46 ` [PATCH 28/28] ovl: Enable metadata only feature Miklos Szeredi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180529144612.16675-25-mszeredi@redhat.com \
    --to=mszeredi@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --subject='Re: [PATCH 24/28] ovl: Check redirect on index as well' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).