[IB] uverbs: Close some exploitable races
Al Viro pointed out that the current IB userspace verbs interface allows userspace to cause mischief by closing file descriptors before we're ready, or issuing the same command twice at the same time. This patch closes those races, and fixes other obvious problems such as a module reference leak. Some other interface bogosities will require an ABI change to fix properly, so I'm deferring those fixes until 2.6.15. Signed-off-by: Roland Dreier <rolandd@cisco.com>
This commit is contained in:
Roland Dreier
parent
44dd823b00
commit
63c47c286d
4 changed files with 86 additions and 63 deletions
|
|
@ -69,6 +69,7 @@ struct ib_uverbs_event_file {
|
|||
|
||||
struct ib_uverbs_file {
|
||||
struct kref ref;
|
||||
struct semaphore mutex;
|
||||
struct ib_uverbs_device *device;
|
||||
struct ib_ucontext *ucontext;
|
||||
struct ib_event_handler event_handler;
|
||||
|
|
|
|||
|
|
@ -76,8 +76,9 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
|
|||
struct ib_uverbs_get_context_resp resp;
|
||||
struct ib_udata udata;
|
||||
struct ib_device *ibdev = file->device->ib_dev;
|
||||
struct ib_ucontext *ucontext;
|
||||
int i;
|
||||
int ret = in_len;
|
||||
int ret;
|
||||
|
||||
if (out_len < sizeof resp)
|
||||
return -ENOSPC;
|
||||
|
|
@ -85,45 +86,56 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
|
|||
if (copy_from_user(&cmd, buf, sizeof cmd))
|
||||
return -EFAULT;
|
||||
|
||||
down(&file->mutex);
|
||||
|
||||
if (file->ucontext) {
|
||||
ret = -EINVAL;
|
||||
goto err;
|
||||
}
|
||||
|
||||
INIT_UDATA(&udata, buf + sizeof cmd,
|
||||
(unsigned long) cmd.response + sizeof resp,
|
||||
in_len - sizeof cmd, out_len - sizeof resp);
|
||||
|
||||
file->ucontext = ibdev->alloc_ucontext(ibdev, &udata);
|
||||
if (IS_ERR(file->ucontext)) {
|
||||
ret = PTR_ERR(file->ucontext);
|
||||
file->ucontext = NULL;
|
||||
return ret;
|
||||
}
|
||||
ucontext = ibdev->alloc_ucontext(ibdev, &udata);
|
||||
if (IS_ERR(ucontext))
|
||||
return PTR_ERR(file->ucontext);
|
||||
|
||||
file->ucontext->device = ibdev;
|
||||
INIT_LIST_HEAD(&file->ucontext->pd_list);
|
||||
INIT_LIST_HEAD(&file->ucontext->mr_list);
|
||||
INIT_LIST_HEAD(&file->ucontext->mw_list);
|
||||
INIT_LIST_HEAD(&file->ucontext->cq_list);
|
||||
INIT_LIST_HEAD(&file->ucontext->qp_list);
|
||||
INIT_LIST_HEAD(&file->ucontext->srq_list);
|
||||
INIT_LIST_HEAD(&file->ucontext->ah_list);
|
||||
spin_lock_init(&file->ucontext->lock);
|
||||
ucontext->device = ibdev;
|
||||
INIT_LIST_HEAD(&ucontext->pd_list);
|
||||
INIT_LIST_HEAD(&ucontext->mr_list);
|
||||
INIT_LIST_HEAD(&ucontext->mw_list);
|
||||
INIT_LIST_HEAD(&ucontext->cq_list);
|
||||
INIT_LIST_HEAD(&ucontext->qp_list);
|
||||
INIT_LIST_HEAD(&ucontext->srq_list);
|
||||
INIT_LIST_HEAD(&ucontext->ah_list);
|
||||
|
||||
resp.async_fd = file->async_file.fd;
|
||||
for (i = 0; i < file->device->num_comp; ++i)
|
||||
if (copy_to_user((void __user *) (unsigned long) cmd.cq_fd_tab +
|
||||
i * sizeof (__u32),
|
||||
&file->comp_file[i].fd, sizeof (__u32)))
|
||||
goto err;
|
||||
&file->comp_file[i].fd, sizeof (__u32))) {
|
||||
ret = -EFAULT;
|
||||
goto err_free;
|
||||
}
|
||||
|
||||
if (copy_to_user((void __user *) (unsigned long) cmd.response,
|
||||
&resp, sizeof resp))
|
||||
goto err;
|
||||
&resp, sizeof resp)) {
|
||||
ret = -EFAULT;
|
||||
goto err_free;
|
||||
}
|
||||
|
||||
file->ucontext = ucontext;
|
||||
up(&file->mutex);
|
||||
|
||||
return in_len;
|
||||
|
||||
err:
|
||||
ibdev->dealloc_ucontext(file->ucontext);
|
||||
file->ucontext = NULL;
|
||||
err_free:
|
||||
ibdev->dealloc_ucontext(ucontext);
|
||||
|
||||
return -EFAULT;
|
||||
err:
|
||||
up(&file->mutex);
|
||||
return ret;
|
||||
}
|
||||
|
||||
ssize_t ib_uverbs_query_device(struct ib_uverbs_file *file,
|
||||
|
|
@ -352,9 +364,9 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
|
|||
if (ret)
|
||||
goto err_pd;
|
||||
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_add_tail(&uobj->list, &file->ucontext->pd_list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
memset(&resp, 0, sizeof resp);
|
||||
resp.pd_handle = uobj->id;
|
||||
|
|
@ -368,9 +380,9 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
|
|||
return in_len;
|
||||
|
||||
err_list:
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_del(&uobj->list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
down(&ib_uverbs_idr_mutex);
|
||||
idr_remove(&ib_uverbs_pd_idr, uobj->id);
|
||||
|
|
@ -410,9 +422,9 @@ ssize_t ib_uverbs_dealloc_pd(struct ib_uverbs_file *file,
|
|||
|
||||
idr_remove(&ib_uverbs_pd_idr, cmd.pd_handle);
|
||||
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_del(&uobj->list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
kfree(uobj);
|
||||
|
||||
|
|
@ -512,9 +524,9 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
|
|||
|
||||
resp.mr_handle = obj->uobject.id;
|
||||
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_add_tail(&obj->uobject.list, &file->ucontext->mr_list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
if (copy_to_user((void __user *) (unsigned long) cmd.response,
|
||||
&resp, sizeof resp)) {
|
||||
|
|
@ -527,9 +539,9 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
|
|||
return in_len;
|
||||
|
||||
err_list:
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_del(&obj->uobject.list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
err_unreg:
|
||||
ib_dereg_mr(mr);
|
||||
|
|
@ -570,9 +582,9 @@ ssize_t ib_uverbs_dereg_mr(struct ib_uverbs_file *file,
|
|||
|
||||
idr_remove(&ib_uverbs_mr_idr, cmd.mr_handle);
|
||||
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_del(&memobj->uobject.list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
ib_umem_release(file->device->ib_dev, &memobj->umem);
|
||||
kfree(memobj);
|
||||
|
|
@ -647,9 +659,9 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
|
|||
if (ret)
|
||||
goto err_cq;
|
||||
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_add_tail(&uobj->uobject.list, &file->ucontext->cq_list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
memset(&resp, 0, sizeof resp);
|
||||
resp.cq_handle = uobj->uobject.id;
|
||||
|
|
@ -664,9 +676,9 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
|
|||
return in_len;
|
||||
|
||||
err_list:
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_del(&uobj->uobject.list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
down(&ib_uverbs_idr_mutex);
|
||||
idr_remove(&ib_uverbs_cq_idr, uobj->uobject.id);
|
||||
|
|
@ -712,9 +724,9 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
|
|||
|
||||
idr_remove(&ib_uverbs_cq_idr, cmd.cq_handle);
|
||||
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_del(&uobj->uobject.list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
spin_lock_irq(&file->comp_file[0].lock);
|
||||
list_for_each_entry_safe(evt, tmp, &uobj->comp_list, obj_list) {
|
||||
|
|
@ -847,9 +859,9 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
|
|||
|
||||
resp.qp_handle = uobj->uobject.id;
|
||||
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_add_tail(&uobj->uobject.list, &file->ucontext->qp_list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
if (copy_to_user((void __user *) (unsigned long) cmd.response,
|
||||
&resp, sizeof resp)) {
|
||||
|
|
@ -862,9 +874,9 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
|
|||
return in_len;
|
||||
|
||||
err_list:
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_del(&uobj->uobject.list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
err_destroy:
|
||||
ib_destroy_qp(qp);
|
||||
|
|
@ -989,9 +1001,9 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
|
|||
|
||||
idr_remove(&ib_uverbs_qp_idr, cmd.qp_handle);
|
||||
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_del(&uobj->uobject.list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
spin_lock_irq(&file->async_file.lock);
|
||||
list_for_each_entry_safe(evt, tmp, &uobj->event_list, obj_list) {
|
||||
|
|
@ -1136,9 +1148,9 @@ ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file,
|
|||
|
||||
resp.srq_handle = uobj->uobject.id;
|
||||
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_add_tail(&uobj->uobject.list, &file->ucontext->srq_list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
if (copy_to_user((void __user *) (unsigned long) cmd.response,
|
||||
&resp, sizeof resp)) {
|
||||
|
|
@ -1151,9 +1163,9 @@ ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file,
|
|||
return in_len;
|
||||
|
||||
err_list:
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_del(&uobj->uobject.list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
err_destroy:
|
||||
ib_destroy_srq(srq);
|
||||
|
|
@ -1227,9 +1239,9 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
|
|||
|
||||
idr_remove(&ib_uverbs_srq_idr, cmd.srq_handle);
|
||||
|
||||
spin_lock_irq(&file->ucontext->lock);
|
||||
down(&file->mutex);
|
||||
list_del(&uobj->uobject.list);
|
||||
spin_unlock_irq(&file->ucontext->lock);
|
||||
up(&file->mutex);
|
||||
|
||||
spin_lock_irq(&file->async_file.lock);
|
||||
list_for_each_entry_safe(evt, tmp, &uobj->event_list, obj_list) {
|
||||
|
|
|
|||
|
|
@ -448,7 +448,9 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
|
|||
if (hdr.in_words * 4 != count)
|
||||
return -EINVAL;
|
||||
|
||||
if (hdr.command < 0 || hdr.command >= ARRAY_SIZE(uverbs_cmd_table))
|
||||
if (hdr.command < 0 ||
|
||||
hdr.command >= ARRAY_SIZE(uverbs_cmd_table) ||
|
||||
!uverbs_cmd_table[hdr.command])
|
||||
return -EINVAL;
|
||||
|
||||
if (!file->ucontext &&
|
||||
|
|
@ -484,27 +486,29 @@ static int ib_uverbs_open(struct inode *inode, struct file *filp)
|
|||
file = kmalloc(sizeof *file +
|
||||
(dev->num_comp - 1) * sizeof (struct ib_uverbs_event_file),
|
||||
GFP_KERNEL);
|
||||
if (!file)
|
||||
return -ENOMEM;
|
||||
if (!file) {
|
||||
ret = -ENOMEM;
|
||||
goto err;
|
||||
}
|
||||
|
||||
file->device = dev;
|
||||
kref_init(&file->ref);
|
||||
init_MUTEX(&file->mutex);
|
||||
|
||||
file->ucontext = NULL;
|
||||
|
||||
kref_get(&file->ref);
|
||||
ret = ib_uverbs_event_init(&file->async_file, file);
|
||||
if (ret)
|
||||
goto err;
|
||||
goto err_kref;
|
||||
|
||||
file->async_file.is_async = 1;
|
||||
|
||||
kref_get(&file->ref);
|
||||
|
||||
for (i = 0; i < dev->num_comp; ++i) {
|
||||
kref_get(&file->ref);
|
||||
ret = ib_uverbs_event_init(&file->comp_file[i], file);
|
||||
if (ret)
|
||||
goto err_async;
|
||||
kref_get(&file->ref);
|
||||
file->comp_file[i].is_async = 0;
|
||||
}
|
||||
|
||||
|
|
@ -524,9 +528,16 @@ static int ib_uverbs_open(struct inode *inode, struct file *filp)
|
|||
|
||||
ib_uverbs_event_release(&file->async_file);
|
||||
|
||||
err:
|
||||
err_kref:
|
||||
/*
|
||||
* One extra kref_put() because we took a reference before the
|
||||
* event file creation that failed and got us here.
|
||||
*/
|
||||
kref_put(&file->ref, ib_uverbs_release_file);
|
||||
kref_put(&file->ref, ib_uverbs_release_file);
|
||||
|
||||
err:
|
||||
module_put(dev->ib_dev->owner);
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -665,7 +665,6 @@ struct ib_ucontext {
|
|||
struct list_head qp_list;
|
||||
struct list_head srq_list;
|
||||
struct list_head ah_list;
|
||||
spinlock_t lock;
|
||||
};
|
||||
|
||||
struct ib_uobject {
|
||||
|
|
|
|||
Loading…
Reference in a new issue