[PATCH] ovl: fix possible use after free on redirect dir lookup

[Amir Goldstein <amir73il@gmail.com>]

ovl_lookup_layer() iterates on path elements of d->name.name
but also frees and allocates a new pointer for d->name.name.

For the case of lookup in upper layer, the initial d->name.name
pointer is stable (dentry->d_name), but for lower layers, the
initial d->name.name can be d->redirect, which can be freed during
iteration.

Make a copy of initial d->name.name before iterating it and reset
the iteration pointer to point inside the newly allocated d->redirect
path after lookup of every element in the path.

cc: stable [v4.9] <stable@vger.kernel.org>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Miklos,

I have not written a test case to prove this bug yet, just had
a WTF moment while working on redirect_fh.

Am I missing something??

Will try to add a test case to reproduce.

Amir.

 fs/overlayfs/namei.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 9ad48d9..9cb589c 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -154,6 +154,7 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
 static int ovl_lookup_layer(struct dentry *base, struct ovl_lookup_data *d,
 			    struct dentry **ret)
 {
+	struct qstr name = d->name;
 	const char *s = d->name.name;
 	struct dentry *dentry = NULL;
 	int err;
@@ -162,24 +163,36 @@ static int ovl_lookup_layer(struct dentry *base, struct ovl_lookup_data *d,
 		return ovl_lookup_single(base, d, d->name.name, d->name.len,
 					 0, "", ret);
 
+	/* ovl_lookup_single() -> ovl_check_redirect() may free d->name.name */
+	s = name.name = kstrdup(d->name.name, GFP_KERNEL);
+	if (!s)
+		return -ENOMEM;
+
 	while (*s++ == '/' && !IS_ERR_OR_NULL(base) && d_can_lookup(base)) {
-		const char *next = strchrnul(s, '/');
+		const char *post = strchrnul(s, '/');
 		size_t slen = strlen(s);
+		size_t postlen = slen - (post - s);
 
-		if (WARN_ON(slen > d->name.len) ||
-		    WARN_ON(strcmp(d->name.name + d->name.len - slen, s)))
-			return -EIO;
-
-		err = ovl_lookup_single(base, d, s, next - s,
-					d->name.len - slen, next, &base);
+		err = ovl_lookup_single(base, d, s, post - s,
+					d->name.len - slen, post, &base);
 		dput(dentry);
 		if (err)
-			return err;
+			goto out_err;
 		dentry = base;
-		s = next;
+
+		/* d->name.name may be a new string with modified prefix */
+		s = d->name.name + d->name.len - postlen;
+		err = -EIO;
+		if (WARN_ON(postlen > name.len) ||
+		    WARN_ON(strcmp(name.name + name.len - postlen, s)))
+			goto out_err;
 	}
+
 	*ret = dentry;
-	return 0;
+	err = 0;
+out_err:
+	kfree(name.name);
+	return err;
 }
 
 /*
-- 
2.7.4