Fix possible filp_cachep memory corruption
In commit 31e6b01f41
("fs: rcu-walk for path lookup") we started doing
path lookup using RCU, which then falls back to a careful non-RCU lookup
in case of problems (LOOKUP_REVAL). So do_filp_open() has this "re-do
the lookup carefully" looping case.
However, that means that we must not release the open-intent file data
if we are going to loop around and use it once more!
Fix this by moving the release of the open-intent data to the function
that allocates it (do_filp_open() itself) rather than the helper
functions that can get called multiple times (finish_open() and
do_last()). This makes the logic for the lifetime of that field much
more obvious, and avoids the possible double free.
Reported-by: J. R. Okajima <hooanon05@yahoo.co.jp>
Acked-by: Al Viro <viro@zeniv.linux.org.uk>
Cc: Nick Piggin <npiggin@kernel.dk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
18
fs/namei.c
18
fs/namei.c
@@ -561,10 +561,14 @@ static inline int nameidata_drop_rcu_last_maybe(struct nameidata *nd)
|
|||||||
*/
|
*/
|
||||||
void release_open_intent(struct nameidata *nd)
|
void release_open_intent(struct nameidata *nd)
|
||||||
{
|
{
|
||||||
if (nd->intent.open.file->f_path.dentry == NULL)
|
struct file *file = nd->intent.open.file;
|
||||||
put_filp(nd->intent.open.file);
|
|
||||||
|
if (file && !IS_ERR(file)) {
|
||||||
|
if (file->f_path.dentry == NULL)
|
||||||
|
put_filp(file);
|
||||||
else
|
else
|
||||||
fput(nd->intent.open.file);
|
fput(file);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@@ -2265,8 +2269,6 @@ static struct file *finish_open(struct nameidata *nd,
|
|||||||
return filp;
|
return filp;
|
||||||
|
|
||||||
exit:
|
exit:
|
||||||
if (!IS_ERR(nd->intent.open.file))
|
|
||||||
release_open_intent(nd);
|
|
||||||
path_put(&nd->path);
|
path_put(&nd->path);
|
||||||
return ERR_PTR(error);
|
return ERR_PTR(error);
|
||||||
}
|
}
|
||||||
@@ -2389,8 +2391,6 @@ exit_mutex_unlock:
|
|||||||
exit_dput:
|
exit_dput:
|
||||||
path_put_conditional(path, nd);
|
path_put_conditional(path, nd);
|
||||||
exit:
|
exit:
|
||||||
if (!IS_ERR(nd->intent.open.file))
|
|
||||||
release_open_intent(nd);
|
|
||||||
path_put(&nd->path);
|
path_put(&nd->path);
|
||||||
return ERR_PTR(error);
|
return ERR_PTR(error);
|
||||||
}
|
}
|
||||||
@@ -2477,6 +2477,7 @@ struct file *do_filp_open(int dfd, const char *pathname,
|
|||||||
}
|
}
|
||||||
audit_inode(pathname, nd.path.dentry);
|
audit_inode(pathname, nd.path.dentry);
|
||||||
filp = finish_open(&nd, open_flag, acc_mode);
|
filp = finish_open(&nd, open_flag, acc_mode);
|
||||||
|
release_open_intent(&nd);
|
||||||
return filp;
|
return filp;
|
||||||
|
|
||||||
creat:
|
creat:
|
||||||
@@ -2553,6 +2554,7 @@ out:
|
|||||||
path_put(&nd.root);
|
path_put(&nd.root);
|
||||||
if (filp == ERR_PTR(-ESTALE) && !(flags & LOOKUP_REVAL))
|
if (filp == ERR_PTR(-ESTALE) && !(flags & LOOKUP_REVAL))
|
||||||
goto reval;
|
goto reval;
|
||||||
|
release_open_intent(&nd);
|
||||||
return filp;
|
return filp;
|
||||||
|
|
||||||
exit_dput:
|
exit_dput:
|
||||||
@@ -2560,8 +2562,6 @@ exit_dput:
|
|||||||
out_path:
|
out_path:
|
||||||
path_put(&nd.path);
|
path_put(&nd.path);
|
||||||
out_filp:
|
out_filp:
|
||||||
if (!IS_ERR(nd.intent.open.file))
|
|
||||||
release_open_intent(&nd);
|
|
||||||
filp = ERR_PTR(error);
|
filp = ERR_PTR(error);
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
@@ -790,6 +790,8 @@ struct file *nameidata_to_filp(struct nameidata *nd)
|
|||||||
|
|
||||||
/* Pick up the filp from the open intent */
|
/* Pick up the filp from the open intent */
|
||||||
filp = nd->intent.open.file;
|
filp = nd->intent.open.file;
|
||||||
|
nd->intent.open.file = NULL;
|
||||||
|
|
||||||
/* Has the filesystem initialised the file for us? */
|
/* Has the filesystem initialised the file for us? */
|
||||||
if (filp->f_path.dentry == NULL) {
|
if (filp->f_path.dentry == NULL) {
|
||||||
path_get(&nd->path);
|
path_get(&nd->path);
|
||||||
|
Reference in New Issue
Block a user