Fix possible filp_cachep memory corruption

In commit 31e6b01f4183 ("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>

diff --git a/fs/namei.c b/fs/namei.c
index 7d77f24d32a98a115e320605369006eeef30ae0d..ec4b2d0190a8584db09b2252260bd6a8fb8771b2 100644 (file)
--- a/fs/namei.c
+++ b/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)
 {
-       if (nd->intent.open.file->f_path.dentry == NULL)
-               put_filp(nd->intent.open.file);
-       else
-               fput(nd->intent.open.file);
+       struct file *file = nd->intent.open.file;
+
+       if (file && !IS_ERR(file)) {
+               if (file->f_path.dentry == NULL)
+                       put_filp(file);
+               else
+                       fput(file);
+       }
 }
 
 /*
@@ -2265,8 +2269,6 @@ static struct file *finish_open(struct nameidata *nd,
        return filp;
 
 exit:
-       if (!IS_ERR(nd->intent.open.file))
-               release_open_intent(nd);
        path_put(&nd->path);
        return ERR_PTR(error);
 }
@@ -2389,8 +2391,6 @@ exit_mutex_unlock:
 exit_dput:
        path_put_conditional(path, nd);
 exit:
-       if (!IS_ERR(nd->intent.open.file))
-               release_open_intent(nd);
        path_put(&nd->path);
        return ERR_PTR(error);
 }
@@ -2477,6 +2477,7 @@ struct file *do_filp_open(int dfd, const char *pathname,
        }
        audit_inode(pathname, nd.path.dentry);
        filp = finish_open(&nd, open_flag, acc_mode);
+       release_open_intent(&nd);
        return filp;
 
 creat:
@@ -2553,6 +2554,7 @@ out:
                path_put(&nd.root);
        if (filp == ERR_PTR(-ESTALE) && !(flags & LOOKUP_REVAL))
                goto reval;
+       release_open_intent(&nd);
        return filp;
 
 exit_dput:
@@ -2560,8 +2562,6 @@ exit_dput:
 out_path:
        path_put(&nd.path);
 out_filp:
-       if (!IS_ERR(nd.intent.open.file))
-               release_open_intent(&nd);
        filp = ERR_PTR(error);
        goto out;
 }

diff --git a/fs/open.c b/fs/open.c
index e52389e1f05b4c010b49ef9a6e4d8cb11bfac504..5a2c6ebc22b5d9a1e355050cb14d3218f7a9d0d3 100644 (file)
--- a/fs/open.c
+++ b/fs/open.c
@@ -790,6 +790,8 @@ struct file *nameidata_to_filp(struct nameidata *nd)
 
        /* Pick up the filp from the open intent */
        filp = nd->intent.open.file;
+       nd->intent.open.file = NULL;
+
        /* Has the filesystem initialised the file for us? */
        if (filp->f_path.dentry == NULL) {
                path_get(&nd->path);