public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Oleg Drokin <green@linuxhacker.ru>
To: linux-kernel@vger.kernel.org, trond.myklebust@fys.uio.no
Subject: Missed error checking for intent's filp in open_namei in 2.6.15
Date: Sun, 19 Feb 2006 01:11:53 +0200	[thread overview]
Message-ID: <20060218231153.GA32003@linuxhacker.ru> (raw)

Hello!

   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.
   What do you think?

--- fs/namei.c.orig	2006-02-19 00:33:24.000000000 +0200
+++ fs/namei.c	2006-02-19 00:46:28.000000000 +0200
@@ -1575,6 +1575,12 @@ do_last:
 		goto exit;
 	}
 
+        if (IS_ERR(nd->intent.open.file)) {
+		up(&dir->d_inode->i_sem);
+		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))

Bye,
    Oleg

             reply	other threads:[~2006-02-18 23:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-02-18 23:11 Oleg Drokin [this message]
2006-02-18 23:42 ` Missed error checking for intent's filp in open_namei in 2.6.15 Trond Myklebust

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060218231153.GA32003@linuxhacker.ru \
    --to=green@linuxhacker.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=trond.myklebust@fys.uio.no \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox