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@xxxxxxxxxxxxxxx>
Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html