From: "Américo Wang" <xiyou.wangcong@gmail.com>
To: LKML <linux-kernel@vger.kernel.org>
Cc: "Américo Wang" <xiyou.wangcong@gmail.com>,
"André Goddard Rosa" <andre.goddard@gmail.com>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Serge E . Hallyn" <serue@us.ibm.com>,
"Cedric Le Goater" <clg@fr.ibm.com>,
"Al Viro" <viro@zeniv.linux.org.uk>,
"Xiaotian Feng" <xtfeng@gmail.com>
Subject: [Patch] mqueue: fix the bad code in sys_mq_open()
Date: Thu, 25 Feb 2010 21:40:23 +0800 [thread overview]
Message-ID: <20100225134023.GB3842@hack> (raw)
In-Reply-To: <7b6bb4a51002250249t7e4f03c9r6b2b9a8f348a29aa@mail.gmail.com>
Fix bad code in ipc/mqueue.c.
Inspired by André Goddard Rosa's patch.
a) do_create() and do_open() should not release the resources which
are not acquired within themselves, it's their caller's work;
b) Fix an fd leak;
c) The goto label 'out_upsem' doesn't make any sense, rename to
'out_unlock';
d) mntget() should be called before looking up dentry within
->mnt->mnt_root;
e) When dealing with failure case, we should release the resources
in a reversed order of acquiring, so reorder the goto labels;
f) Remove some unused labels due to reorder.
Also, this shrinks the binary by 60 bytes:
text data bss dec hex filename
7674 1684 8 9366 2496 ipc/mqueue.o.BEFORE
7614 1684 8 9306 245a ipc/mqueue.o.AFTER
Signed-off-by: WANG Cong <xiyou.wangcong@gmail.com>
Cc: André Goddard Rosa <andre.goddard@gmail.com>
---
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index c79bd57..fdd09da 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -650,8 +650,6 @@ static struct file *do_create(struct ipc_namespace *ipc_ns, struct dentry *dir,
out_drop_write:
mnt_drop_write(ipc_ns->mq_mnt);
out:
- dput(dentry);
- mntput(ipc_ns->mq_mnt);
return ERR_PTR(ret);
}
@@ -664,17 +662,11 @@ static struct file *do_open(struct ipc_namespace *ipc_ns,
static const int oflag2acc[O_ACCMODE] = { MAY_READ, MAY_WRITE,
MAY_READ | MAY_WRITE };
- if ((oflag & O_ACCMODE) == (O_RDWR | O_WRONLY)) {
- dput(dentry);
- mntput(ipc_ns->mq_mnt);
+ if ((oflag & O_ACCMODE) == (O_RDWR | O_WRONLY))
return ERR_PTR(-EINVAL);
- }
- if (inode_permission(dentry->d_inode, oflag2acc[oflag & O_ACCMODE])) {
- dput(dentry);
- mntput(ipc_ns->mq_mnt);
+ if (inode_permission(dentry->d_inode, oflag2acc[oflag & O_ACCMODE]))
return ERR_PTR(-EACCES);
- }
return dentry_open(dentry, ipc_ns->mq_mnt, oflag, cred);
}
@@ -686,7 +678,7 @@ SYSCALL_DEFINE4(mq_open, const char __user *, u_name, int, oflag, mode_t, mode,
struct file *filp;
char *name;
struct mq_attr attr;
- int fd, error;
+ int fd, error = 0;
struct ipc_namespace *ipc_ns = current->nsproxy->ipc_ns;
if (u_attr && copy_from_user(&attr, u_attr, sizeof(struct mq_attr)))
@@ -701,13 +693,13 @@ SYSCALL_DEFINE4(mq_open, const char __user *, u_name, int, oflag, mode_t, mode,
if (fd < 0)
goto out_putname;
+ mntget(ipc_ns->mq_mnt);
mutex_lock(&ipc_ns->mq_mnt->mnt_root->d_inode->i_mutex);
dentry = lookup_one_len(name, ipc_ns->mq_mnt->mnt_root, strlen(name));
if (IS_ERR(dentry)) {
error = PTR_ERR(dentry);
- goto out_err;
+ goto out_unlock;
}
- mntget(ipc_ns->mq_mnt);
if (oflag & O_CREAT) {
if (dentry->d_inode) { /* entry already exists */
@@ -731,24 +723,23 @@ SYSCALL_DEFINE4(mq_open, const char __user *, u_name, int, oflag, mode_t, mode,
if (IS_ERR(filp)) {
error = PTR_ERR(filp);
- goto out_putfd;
+ goto out;
}
fd_install(fd, filp);
- goto out_upsem;
+ goto out_unlock;
out:
dput(dentry);
- mntput(ipc_ns->mq_mnt);
-out_putfd:
- put_unused_fd(fd);
-out_err:
- fd = error;
-out_upsem:
+out_unlock:
mutex_unlock(&ipc_ns->mq_mnt->mnt_root->d_inode->i_mutex);
+ if (error) {
+ mntput(ipc_ns->mq_mnt);
+ put_unused_fd(fd);
+ }
out_putname:
putname(name);
- return fd;
+ return error;
}
SYSCALL_DEFINE1(mq_unlink, const char __user *, u_name)
next prev parent reply other threads:[~2010-02-25 13:37 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-23 7:04 [PATCH 0/6] Fix file descriptor leak on user-space processes and cleanup André Goddard Rosa
2010-02-23 7:04 ` [PATCH 1/6] mqueue: remove unneeded info->messages initialization André Goddard Rosa
2010-02-23 7:04 ` [PATCH 2/6] mqueue: apply mathematics distributivity on mq_bytes calculation André Goddard Rosa
2010-02-23 7:04 ` [PATCH 3/6] mqueue: simplify do_open() error handling André Goddard Rosa
2010-02-23 7:04 ` [PATCH 4/6] mqueue: only set error codes if they are really necessary André Goddard Rosa
2010-02-23 7:04 ` [PATCH 5/6] mqueue: fix typo "failues" -> "failures" André Goddard Rosa
2010-02-23 7:04 ` [PATCH 6/6] mqueue: fix mq_open() file descriptor leak on user-space processes André Goddard Rosa
2010-02-25 3:35 ` Américo Wang
2010-02-25 4:00 ` Xiaotian Feng
2010-02-25 4:25 ` Américo Wang
2010-02-25 6:59 ` Américo Wang
2010-02-25 10:49 ` Xiaotian Feng
2010-02-25 13:17 ` Américo Wang
2010-02-25 13:40 ` Américo Wang [this message]
2010-02-25 15:41 ` [Patch] mqueue: fix the bad code in sys_mq_open() André Goddard Rosa
2010-02-25 16:15 ` Américo Wang
2010-03-03 19:54 ` Al Viro
2010-02-25 10:56 ` [PATCH 6/6] mqueue: fix mq_open() file descriptor leak on user-space processes Xiaotian Feng
2010-02-24 22:01 ` [PATCH 0/6] Fix file descriptor leak on user-space processes and cleanup Andrew Morton
2010-02-25 15:52 ` André Goddard Rosa
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=20100225134023.GB3842@hack \
--to=xiyou.wangcong@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=andre.goddard@gmail.com \
--cc=clg@fr.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=serue@us.ibm.com \
--cc=viro@zeniv.linux.org.uk \
--cc=xtfeng@gmail.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.