[PATCH] Missed error checking for intent's filp in open_namei().
authorOleg Drokin <green@linuxhacker.ru>
Sat, 25 Mar 2006 11:06:54 +0000 (03:06 -0800)
committerLinus Torvalds <torvalds@g5.osdl.org>
Sat, 25 Mar 2006 16:22:51 +0000 (08:22 -0800)
It seems there is error check missing in open_namei for errors returned
through intent.open.file (from lookup_instantiate_filp).

If there is plain open performed, then such a check done inside
__path_lookup_intent_open called from path_lookup_open(), but when the open
is performed with O_CREAT flag set, then __path_lookup_intent_open is only
called with LOOKUP_PARENT set where no file opening can occur yet.

Later on lookup_hash is called where exact opening might take place and
intent.open.file may be filled.  If it is filled with error value of some
sort, then we get kernel attempting to dereference this error value as
address (and corresponding oops) in nameidata_to_filp() called from
filp_open().

While this is relatively simple to workaround in ->lookup() method by just
checking lookup_instantiate_filp() return value and returning error as
needed, this is not so easy in ->d_revalidate(), where we can only return
"yes, dentry is valid" or "no, dentry is invalid, perform full lookup
again", and just returning 0 on error would cause extra lookup (with
potential extra costly RPCs).

So in short, I believe that there should be no difference in error handling
for opening a file and creating a file in open_namei() and propose this
simple patch as a solution.

Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
fs/namei.c

index c72b940797fc0321fcb6043610bc50138a2325ed..1baf1b06fe4793435e48f80c978469f8d39d08b2 100644 (file)
@@ -1628,6 +1628,12 @@ do_last:
                goto exit;
        }
 
+       if (IS_ERR(nd->intent.open.file)) {
+               mutex_unlock(&dir->d_inode->i_mutex);
+               error = PTR_ERR(nd->intent.open.file);
+               goto exit_dput;
+       }
+
        /* Negative dentry, just create the file */
        if (!path.dentry->d_inode) {
                if (!IS_POSIXACL(dir->d_inode))