lkml.org 
[lkml]   [2010]   [Feb]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[Patch] mqueue: fix the bad code in sys_mq_open()

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)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2010-02-25 14:39    [W:0.020 / U:0.248 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site