public inbox for linux-unionfs@vger.kernel.org
 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 1/2] ovl: invalidate dentry with deleted real dir
Date: Mon, 13 Jul 2020 13:57:31 +0300	[thread overview]
Message-ID: <20200713105732.2886-2-amir73il@gmail.com> (raw)
In-Reply-To: <20200713105732.2886-1-amir73il@gmail.com>

Changes to underlying layers 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
underlying dentries was deleted since the dentry was composed.

Negative underlying dentries are not expected in overlay upper and
lower dentries.  If they are found it is probably dcache lookup racing
with an overlay unlink, before d_drop() was called on the overlay dentry.
IS_DEADDIR directories may be caused by underlying rmdir, so invalidate
overlay dentry on dcache lookup if we find those.

We preserve the legacy behaior of returning -ESTALE on invalid cache
for lower dentries, but we relax this behavior for upper dentries
that may be invalidated by a race with overlay unlink/rmdir.

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 underlying delete,
because lookup cannot return those invalid dentry stacks.

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

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 06ec3cb977e6..f2c74387e05b 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -113,21 +113,42 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
 	return dentry;
 }
 
-static int ovl_revalidate_real(struct dentry *d, unsigned int flags, bool weak)
+static bool ovl_dentry_is_dead(struct dentry *d)
 {
+	return unlikely(!d->d_inode || IS_DEADDIR(d->d_inode));
+}
+
+static int ovl_revalidate_real(struct dentry *d, unsigned int flags, bool weak,
+			       bool is_upper)
+{
+	bool strict = !weak;
 	int ret = 1;
 
-	if (weak) {
+	/* Invalidate dentry if real was deleted since we found it */
+	if (ovl_dentry_is_dead(d)) {
+		ret = 0;
+		/* Raced with overlay unlink/rmdir? */
+		if (is_upper)
+			strict = false;
+	} else if (weak) {
 		if (d->d_flags & DCACHE_OP_WEAK_REVALIDATE)
-			ret =  d->d_op->d_weak_revalidate(d, flags);
+			ret = d->d_op->d_weak_revalidate(d, flags);
 	} else if (d->d_flags & DCACHE_OP_REVALIDATE) {
 		ret = d->d_op->d_revalidate(d, flags);
-		if (!ret) {
-			if (!(flags & LOOKUP_RCU))
-				d_invalidate(d);
-			ret = -ESTALE;
-		}
 	}
+
+	/*
+	 * Legacy overlayfs strict behavior is to return an error to user on
+	 * non-weak revalidate rather than retry the lookup, because underlying
+	 * layer changes are not expected. We may want to relax this in the
+	 * future either for upper only or also for lower.
+	 */
+	if (strict && !ret) {
+		if (!(flags & LOOKUP_RCU))
+			d_invalidate(d);
+		ret = -ESTALE;
+	}
+
 	return ret;
 }
 
@@ -141,11 +162,11 @@ static int ovl_dentry_revalidate_common(struct dentry *dentry,
 
 	upper = ovl_dentry_upper(dentry);
 	if (upper)
-		ret = ovl_revalidate_real(upper, flags, weak);
+		ret = ovl_revalidate_real(upper, flags, weak, true);
 
 	for (i = 0; ret > 0 && i < oe->numlower; i++) {
 		ret = ovl_revalidate_real(oe->lowerstack[i].dentry, flags,
-					  weak);
+					  weak, false);
 	}
 	return ret;
 }
-- 
2.17.1


  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 ` Amir Goldstein [this message]
2020-07-13 19:25   ` [PATCH RFC 1/2] ovl: invalidate dentry with deleted real dir 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 ` [PATCH RFC 2/2] ovl: invalidate dentry if lower was renamed Amir Goldstein
2020-07-13 20:05   ` 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-2-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