linux-unionfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Vivek Goyal <vgoyal@redhat.com>, Josh England <jjengla@gmail.com>,
	linux-unionfs@vger.kernel.org
Subject: [PATCH RFC 2/2] ovl: invalidate dentry if lower was renamed
Date: Mon, 13 Jul 2020 13:57:32 +0300	[thread overview]
Message-ID: <20200713105732.2886-3-amir73il@gmail.com> (raw)
In-Reply-To: <20200713105732.2886-1-amir73il@gmail.com>

Changes to lower layer while overlay in mounted result in undefined
behavior.  Therefore, we can change the behavior to invalidate the
overlay dentry on dcache lookup if one of the dentries in the lowerstack
was renamed since the lowerstack was composed.

To be absolute certain that lower dentry was not renamed we would need to
know the redirect path that lead to it, but that is not necessary.
Instead, we just store the hash of the parent/name from when we composed
the stack, which gives a good enough probablity to detect a lower rename
and is much less complexity.

We do not provide this protection for upper dentries, because that would
require updating the hash on overlay initiated renames and that is harder
to implement with lockless lookup.

This doesn't make live changes to underlying layers valid, because
invalid dentry stacks may still be referenced by open files, but it
reduces the window for possible bugs caused by lower rename, because
lookup cannot return those invalid dentry stacks.

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

diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
index 0e696f72cf65..7221b6226e26 100644
--- a/fs/overlayfs/export.c
+++ b/fs/overlayfs/export.c
@@ -319,6 +319,7 @@ static struct dentry *ovl_obtain_alias(struct super_block *sb,
 	if (lower) {
 		oe->lowerstack->dentry = dget(lower);
 		oe->lowerstack->layer = lowerpath->layer;
+		oe->lowerstack->hash = lower->d_name.hash_len;
 	}
 	dentry->d_fsdata = oe;
 	if (upper_alias)
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 3566282a9199..ae1c1216a038 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -375,7 +375,8 @@ int ovl_check_origin_fh(struct ovl_fs *ofs, struct ovl_fh *fh, bool connected,
 	}
 	**stackp = (struct ovl_path){
 		.dentry = origin,
-		.layer = &ofs->layers[i]
+		.layer = &ofs->layers[i],
+		.hash = origin->d_name.hash_len,
 	};
 
 	return 0;
@@ -968,6 +969,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		} else {
 			stack[ctr].dentry = this;
 			stack[ctr].layer = lower.layer;
+			stack[ctr].hash = this->d_name.hash_len;
 			ctr++;
 		}
 
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index b429c80879ee..557f1782f53b 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -42,6 +42,8 @@ struct ovl_layer {
 struct ovl_path {
 	const struct ovl_layer *layer;
 	struct dentry *dentry;
+	/* Hash of the lower parent/name when we found it */
+	u64 hash;
 };
 
 /* private information held for overlayfs's superblock */
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index f2c74387e05b..4b7cb2d98203 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -119,13 +119,13 @@ static bool ovl_dentry_is_dead(struct dentry *d)
 }
 
 static int ovl_revalidate_real(struct dentry *d, unsigned int flags, bool weak,
-			       bool is_upper)
+			       bool is_upper, u64 hash)
 {
 	bool strict = !weak;
 	int ret = 1;
 
-	/* Invalidate dentry if real was deleted since we found it */
-	if (ovl_dentry_is_dead(d)) {
+	/* Invalidate dentry if real was deleted/renamed since we found it */
+	if (ovl_dentry_is_dead(d) || (hash && hash != d->d_name.hash_len)) {
 		ret = 0;
 		/* Raced with overlay unlink/rmdir? */
 		if (is_upper)
@@ -156,17 +156,18 @@ static int ovl_dentry_revalidate_common(struct dentry *dentry,
 					unsigned int flags, bool weak)
 {
 	struct ovl_entry *oe = dentry->d_fsdata;
+	struct ovl_path *lower = oe->lowerstack;
 	struct dentry *upper;
 	unsigned int i;
 	int ret = 1;
 
 	upper = ovl_dentry_upper(dentry);
 	if (upper)
-		ret = ovl_revalidate_real(upper, flags, weak, true);
+		ret = ovl_revalidate_real(upper, flags, weak, true, 0);
 
-	for (i = 0; ret > 0 && i < oe->numlower; i++) {
-		ret = ovl_revalidate_real(oe->lowerstack[i].dentry, flags,
-					  weak, false);
+	for (i = 0; ret > 0 && i < oe->numlower; i++, lower++) {
+		ret = ovl_revalidate_real(lower->dentry, flags, weak, false,
+					  lower->hash);
 	}
 	return ret;
 }
@@ -1652,6 +1653,8 @@ static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb,
 	for (i = 0; i < numlower; i++) {
 		oe->lowerstack[i].dentry = dget(stack[i].dentry);
 		oe->lowerstack[i].layer = &ofs->layers[i+1];
+		/* layer root should not be invalidated by rename */
+		oe->lowerstack->hash = 0;
 	}
 
 out:
-- 
2.17.1


  parent reply	other threads:[~2020-07-13 10:57 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-13 10:57 [PATCH RFC 0/2] Invalidate overlayfs dentries on underlying changes Amir Goldstein
2020-07-13 10:57 ` [PATCH RFC 1/2] ovl: invalidate dentry with deleted real dir Amir Goldstein
2020-07-13 19:25   ` Vivek Goyal
2020-07-14  3:28     ` Amir Goldstein
2020-07-14 13:41       ` Vivek Goyal
2020-07-14 14:05         ` Amir Goldstein
2020-07-15  8:57           ` Miklos Szeredi
2020-07-15  9:12             ` Amir Goldstein
2020-07-13 10:57 ` Amir Goldstein [this message]
2020-07-13 20:05   ` [PATCH RFC 2/2] ovl: invalidate dentry if lower was renamed Vivek Goyal
2020-07-14  2:55     ` Amir Goldstein
2020-07-14  9:29 ` [PATCH RFC 0/2] Invalidate overlayfs dentries on underlying changes Amir Goldstein
2020-07-14 16:22   ` Vivek Goyal
2020-07-14 16:57     ` Amir Goldstein

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=20200713105732.2886-3-amir73il@gmail.com \
    --to=amir73il@gmail.com \
    --cc=jjengla@gmail.com \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=vgoyal@redhat.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).