From 4294a8eedb17bbc45e1e7447c2a4d05332943248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Goddard=20Rosa?= Date: Tue, 23 Feb 2010 04:04:28 -0300 Subject: mqueue: fix mq_open() file descriptor leak on user-space processes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We leak fd on lookup_one_len() failure Signed-off-by: André Goddard Rosa Signed-off-by: Al Viro --- ipc/mqueue.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'ipc/mqueue.c') diff --git a/ipc/mqueue.c b/ipc/mqueue.c index c79bd57353e7..04985a7256c2 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -705,7 +705,7 @@ SYSCALL_DEFINE4(mq_open, const char __user *, u_name, int, oflag, mode_t, mode, 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_putfd; } mntget(ipc_ns->mq_mnt); @@ -742,7 +742,6 @@ out: mntput(ipc_ns->mq_mnt); out_putfd: put_unused_fd(fd); -out_err: fd = error; out_upsem: mutex_unlock(&ipc_ns->mq_mnt->mnt_root->d_inode->i_mutex); -- cgit v1.2.1 From c8308b1c91056b09e96d40dbde4880ea685c377e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Goddard=20Rosa?= Date: Tue, 23 Feb 2010 04:04:23 -0300 Subject: mqueue: remove unneeded info->messages initialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... and abort earlier if we couldn't allocate the message pointers array, avoiding the u->mq_bytes accounting logic. It reduces code size: text data bss dec hex filename 9949 72 16 10037 2735 ipc/mqueue-BEFORE.o 9941 72 16 10029 272d ipc/mqueue-AFTER.o Signed-off-by: André Goddard Rosa Signed-off-by: Al Viro --- ipc/mqueue.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'ipc/mqueue.c') diff --git a/ipc/mqueue.c b/ipc/mqueue.c index 04985a7256c2..3660c334ee6b 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -134,7 +134,6 @@ static struct inode *mqueue_get_inode(struct super_block *sb, init_waitqueue_head(&info->wait_q); INIT_LIST_HEAD(&info->e_wait_q[0].list); INIT_LIST_HEAD(&info->e_wait_q[1].list); - info->messages = NULL; info->notify_owner = NULL; info->qsize = 0; info->user = NULL; /* set when all is ok */ @@ -146,6 +145,10 @@ static struct inode *mqueue_get_inode(struct super_block *sb, info->attr.mq_msgsize = attr->mq_msgsize; } mq_msg_tblsz = info->attr.mq_maxmsg * sizeof(struct msg_msg *); + info->messages = kmalloc(mq_msg_tblsz, GFP_KERNEL); + if (!info->messages) + goto out_inode; + mq_bytes = (mq_msg_tblsz + (info->attr.mq_maxmsg * info->attr.mq_msgsize)); @@ -154,18 +157,12 @@ static struct inode *mqueue_get_inode(struct super_block *sb, u->mq_bytes + mq_bytes > p->signal->rlim[RLIMIT_MSGQUEUE].rlim_cur) { spin_unlock(&mq_lock); + kfree(info->messages); goto out_inode; } u->mq_bytes += mq_bytes; spin_unlock(&mq_lock); - info->messages = kmalloc(mq_msg_tblsz, GFP_KERNEL); - if (!info->messages) { - spin_lock(&mq_lock); - u->mq_bytes -= mq_bytes; - spin_unlock(&mq_lock); - goto out_inode; - } /* all is ok */ info->user = get_uid(u); } else if (S_ISDIR(mode)) { -- cgit v1.2.1 From 8834cf796a4320be2d3a70b1e4f9aba732a0f4ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Goddard=20Rosa?= Date: Tue, 23 Feb 2010 04:04:24 -0300 Subject: mqueue: apply mathematics distributivity on mq_bytes calculation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code size reduction: text data bss dec hex filename 9941 72 16 10029 272d ipc/mqueue-BEFORE.o 9925 72 16 10013 271d ipc/mqueue-AFTER.o Signed-off-by: André Goddard Rosa Signed-off-by: Al Viro --- ipc/mqueue.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'ipc/mqueue.c') diff --git a/ipc/mqueue.c b/ipc/mqueue.c index 3660c334ee6b..15eabf9d51fd 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -261,8 +261,9 @@ static void mqueue_delete_inode(struct inode *inode) clear_inode(inode); - mq_bytes = (info->attr.mq_maxmsg * sizeof(struct msg_msg *) + - (info->attr.mq_maxmsg * info->attr.mq_msgsize)); + /* Total amount of bytes accounted for the mqueue */ + mq_bytes = info->attr.mq_maxmsg * (sizeof(struct msg_msg *) + + info->attr.mq_msgsize); user = info->user; if (user) { spin_lock(&mq_lock); @@ -601,8 +602,8 @@ static int mq_attr_ok(struct ipc_namespace *ipc_ns, struct mq_attr *attr) /* check for overflow */ if (attr->mq_msgsize > ULONG_MAX/attr->mq_maxmsg) return 0; - if ((unsigned long)(attr->mq_maxmsg * attr->mq_msgsize) + - (attr->mq_maxmsg * sizeof (struct msg_msg *)) < + if ((unsigned long)(attr->mq_maxmsg * (attr->mq_msgsize + + sizeof (struct msg_msg *))) < (unsigned long)(attr->mq_maxmsg * attr->mq_msgsize)) return 0; return 1; -- cgit v1.2.1 From 04db0dde0ee1c29110642dff57fba9e438eb805c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Goddard=20Rosa?= Date: Tue, 23 Feb 2010 04:04:25 -0300 Subject: mqueue: simplify do_open() error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It reduces code size: text data bss dec hex filename 9925 72 16 10013 271d ipc/mqueue-BEFORE.o 9885 72 16 9973 26f5 ipc/mqueue-AFTER.o Signed-off-by: André Goddard Rosa Signed-off-by: Al Viro --- ipc/mqueue.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'ipc/mqueue.c') diff --git a/ipc/mqueue.c b/ipc/mqueue.c index 15eabf9d51fd..3853116a2ef8 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -657,24 +657,28 @@ out: static struct file *do_open(struct ipc_namespace *ipc_ns, struct dentry *dentry, int oflag) { + int ret; const struct cred *cred = current_cred(); 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); - return ERR_PTR(-EINVAL); + ret = -EINVAL; + goto err; } if (inode_permission(dentry->d_inode, oflag2acc[oflag & O_ACCMODE])) { - dput(dentry); - mntput(ipc_ns->mq_mnt); - return ERR_PTR(-EACCES); + ret = -EACCES; + goto err; } return dentry_open(dentry, ipc_ns->mq_mnt, oflag, cred); + +err: + dput(dentry); + mntput(ipc_ns->mq_mnt); + return ERR_PTR(ret); } SYSCALL_DEFINE4(mq_open, const char __user *, u_name, int, oflag, mode_t, mode, -- cgit v1.2.1 From 8d8ffefaaf63f0468f17fbd1270165e739cf335e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Goddard=20Rosa?= Date: Tue, 23 Feb 2010 04:04:26 -0300 Subject: mqueue: only set error codes if they are really necessary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... postponing assignments until they're needed. Doesn't change code size. Signed-off-by: André Goddard Rosa Signed-off-by: Al Viro --- ipc/mqueue.c | 77 +++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 27 deletions(-) (limited to 'ipc/mqueue.c') diff --git a/ipc/mqueue.c b/ipc/mqueue.c index 3853116a2ef8..547d9c8631f5 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -184,7 +184,7 @@ static int mqueue_fill_super(struct super_block *sb, void *data, int silent) { struct inode *inode; struct ipc_namespace *ns = data; - int error = 0; + int error; sb->s_blocksize = PAGE_CACHE_SIZE; sb->s_blocksize_bits = PAGE_CACHE_SHIFT; @@ -202,7 +202,9 @@ static int mqueue_fill_super(struct super_block *sb, void *data, int silent) if (!sb->s_root) { iput(inode); error = -ENOMEM; + goto out; } + error = 0; out: return error; @@ -621,9 +623,10 @@ static struct file *do_create(struct ipc_namespace *ipc_ns, struct dentry *dir, int ret; if (attr) { - ret = -EINVAL; - if (!mq_attr_ok(ipc_ns, attr)) + if (!mq_attr_ok(ipc_ns, attr)) { + ret = -EINVAL; goto out; + } /* store for use during create */ dentry->d_fsdata = attr; } @@ -714,9 +717,10 @@ SYSCALL_DEFINE4(mq_open, const char __user *, u_name, int, oflag, mode_t, mode, if (oflag & O_CREAT) { if (dentry->d_inode) { /* entry already exists */ audit_inode(name, dentry); - error = -EEXIST; - if (oflag & O_EXCL) + if (oflag & O_EXCL) { + error = -EEXIST; goto out; + } filp = do_open(ipc_ns, dentry, oflag); } else { filp = do_create(ipc_ns, ipc_ns->mq_mnt->mnt_root, @@ -724,9 +728,10 @@ SYSCALL_DEFINE4(mq_open, const char __user *, u_name, int, oflag, mode_t, mode, u_attr ? &attr : NULL); } } else { - error = -ENOENT; - if (!dentry->d_inode) + if (!dentry->d_inode) { + error = -ENOENT; goto out; + } audit_inode(name, dentry); filp = do_open(ipc_ns, dentry, oflag); } @@ -873,19 +878,24 @@ SYSCALL_DEFINE5(mq_timedsend, mqd_t, mqdes, const char __user *, u_msg_ptr, audit_mq_sendrecv(mqdes, msg_len, msg_prio, p); timeout = prepare_timeout(p); - ret = -EBADF; filp = fget(mqdes); - if (unlikely(!filp)) + if (unlikely(!filp)) { + ret = -EBADF; goto out; + } inode = filp->f_path.dentry->d_inode; - if (unlikely(filp->f_op != &mqueue_file_operations)) + if (unlikely(filp->f_op != &mqueue_file_operations)) { + ret = -EBADF; goto out_fput; + } info = MQUEUE_I(inode); audit_inode(NULL, filp->f_path.dentry); - if (unlikely(!(filp->f_mode & FMODE_WRITE))) + if (unlikely(!(filp->f_mode & FMODE_WRITE))) { + ret = -EBADF; goto out_fput; + } if (unlikely(msg_len > info->attr.mq_msgsize)) { ret = -EMSGSIZE; @@ -962,19 +972,24 @@ SYSCALL_DEFINE5(mq_timedreceive, mqd_t, mqdes, char __user *, u_msg_ptr, audit_mq_sendrecv(mqdes, msg_len, 0, p); timeout = prepare_timeout(p); - ret = -EBADF; filp = fget(mqdes); - if (unlikely(!filp)) + if (unlikely(!filp)) { + ret = -EBADF; goto out; + } inode = filp->f_path.dentry->d_inode; - if (unlikely(filp->f_op != &mqueue_file_operations)) + if (unlikely(filp->f_op != &mqueue_file_operations)) { + ret = -EBADF; goto out_fput; + } info = MQUEUE_I(inode); audit_inode(NULL, filp->f_path.dentry); - if (unlikely(!(filp->f_mode & FMODE_READ))) + if (unlikely(!(filp->f_mode & FMODE_READ))) { + ret = -EBADF; goto out_fput; + } /* checks if buffer is big enough */ if (unlikely(msg_len < info->attr.mq_msgsize)) { @@ -1064,13 +1079,14 @@ SYSCALL_DEFINE2(mq_notify, mqd_t, mqdes, /* create the notify skb */ nc = alloc_skb(NOTIFY_COOKIE_LEN, GFP_KERNEL); - ret = -ENOMEM; - if (!nc) + if (!nc) { + ret = -ENOMEM; goto out; - ret = -EFAULT; + } if (copy_from_user(nc->data, notification.sigev_value.sival_ptr, NOTIFY_COOKIE_LEN)) { + ret = -EFAULT; goto out; } @@ -1079,9 +1095,10 @@ SYSCALL_DEFINE2(mq_notify, mqd_t, mqdes, /* and attach it to the socket */ retry: filp = fget(notification.sigev_signo); - ret = -EBADF; - if (!filp) + if (!filp) { + ret = -EBADF; goto out; + } sock = netlink_getsockbyfilp(filp); fput(filp); if (IS_ERR(sock)) { @@ -1093,7 +1110,7 @@ retry: timeo = MAX_SCHEDULE_TIMEOUT; ret = netlink_attachskb(sock, nc, &timeo, NULL); if (ret == 1) - goto retry; + goto retry; if (ret) { sock = NULL; nc = NULL; @@ -1102,14 +1119,17 @@ retry: } } - ret = -EBADF; filp = fget(mqdes); - if (!filp) + if (!filp) { + ret = -EBADF; goto out; + } inode = filp->f_path.dentry->d_inode; - if (unlikely(filp->f_op != &mqueue_file_operations)) + if (unlikely(filp->f_op != &mqueue_file_operations)) { + ret = -EBADF; goto out_fput; + } info = MQUEUE_I(inode); ret = 0; @@ -1172,14 +1192,17 @@ SYSCALL_DEFINE3(mq_getsetattr, mqd_t, mqdes, return -EINVAL; } - ret = -EBADF; filp = fget(mqdes); - if (!filp) + if (!filp) { + ret = -EBADF; goto out; + } inode = filp->f_path.dentry->d_inode; - if (unlikely(filp->f_op != &mqueue_file_operations)) + if (unlikely(filp->f_op != &mqueue_file_operations)) { + ret = -EBADF; goto out_fput; + } info = MQUEUE_I(inode); spin_lock(&info->lock); -- cgit v1.2.1 From 2329e392accdb1b277927e8d9cbf568ba3f3856d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Goddard=20Rosa?= Date: Tue, 23 Feb 2010 04:04:27 -0300 Subject: mqueue: fix typo "failues" -> "failures" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: André Goddard Rosa Signed-off-by: Al Viro --- ipc/mqueue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'ipc/mqueue.c') diff --git a/ipc/mqueue.c b/ipc/mqueue.c index 547d9c8631f5..b6cb06451f4b 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -1296,7 +1296,7 @@ static int __init init_mqueue_fs(void) if (mqueue_inode_cachep == NULL) return -ENOMEM; - /* ignore failues - they are not fatal */ + /* ignore failures - they are not fatal */ mq_sysctl_table = mq_register_sysctl_table(); error = register_filesystem(&mqueue_fs_type); -- cgit v1.2.1