diff mbox

[3/4] IB/uverbs: check userspace output buffer size in ib_uverbs_poll_cq()

Message ID 5e4defa2410d0a5b3f3a28508e850dd5d6c287b1.1405884453.git.ydroneaud@opteya.com (mailing list archive)
State Rejected
Headers show

Commit Message

Yann Droneaud July 20, 2014, 7:51 p.m. UTC
Makes ib_uverbs_poll_cq() check the length of the output buffer
against number of wc: this will prevent the function from writing
past userspace provided buffer.

Note that it might affect existing userspace programs that were
setting 'struct ibv_poll_cq' field 'ne' to a value too large
compared to the size of the buffer allocated to hold the work
completion items.

Perhaps such userspace programs were hoping to have allocated
enough memory to hold the maximum number of work completions
that can be returned on this particular completion queue or
it would have been broken with random memory corruption due
to the kernel writting past the response buffer.

Example:

    int vendor_poll_cq_broken(struct ibv_cq *cq, int ne, struct ibv_wc *wc)
    {
        struct ibv_poll_cq       cmd;
        struct ibv_poll_cq_resp *resp;
        int                      resp_size;
        ssize_t                  sret;
        int                      ret = -1;

        cmd.cq_handle = cq->handle;
        cmd.ne        = ne;

         /* forget about ne * sizeof(struct ibv_kern_wc) */
        resp_size = sizeof(*resp);

        resp = malloc(resp_size);
        assert(resp != NULL);

        IBV_INIT_CMD_RESP(&cmd, sizeof(cmd),
                          POLL_CQ,
                          resp, resp_size);

        sret = write(cq->context->cmd_fd,
                     &cmd, sizeof(cmd));

        if (sret != sizeof(cmd))
            goto leave;

        ret = resp->count;

        /* ignore the response */

    leave:

        free(resp);

        return ret;
    }

Since libibverbs is the most common (if not the only) userspace
component that access the uverbs API directly, it should be safe
to have the kernel ensure coherency between the request number
of entries and the size of the response buffer that will be used
to hold the entries.

Link: http://marc.info/?i=cover.1405884453.git.ydroneaud@opteya.com
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---
 drivers/infiniband/core/uverbs_cmd.c | 3 +++
 1 file changed, 3 insertions(+)
diff mbox

Patch

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 9b971514db88..3626143db6ef 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1427,6 +1427,9 @@  ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	if ((out_len - sizeof resp)/(sizeof(struct ib_uverbs_wc)) < cmd.ne)
+		return -ENOSPC;
+
 	cq = idr_read_cq(cmd.cq_handle, file->ucontext, 0);
 	if (!cq)
 		return -EINVAL;