[PATCH RFC 1/2] ovl: invalidate dentry with deleted real dir

[Amir Goldstein <amir73il@xxxxxxxxx>]

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@xxxxxxxxx>
---
 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