[PATCHv1 0/6] IB/uverbs: check request parameters

[PATCHv1 0/6] IB/uverbs: check request parameters

[Yann Droneaud]

Hi,

Please find a patchset against uverbs to improve the checks done
on uverbs request parameters. This patchset in an extract of a
previous patchset sent some times ago[1].

I've provided some explanation of the issues partialy addressed
by this patchset in a previous message[2].

As we're now addressing overflows, I think it's time to apply
this patchset.

Changes since v0 [3]
- updated against v4.1-rc2
- incorporated patches to add check on response buffer using
  access_ok()

[1] "[PATCH 00/22] infiniband: improve userspace input check"

http://marc.info/?i=cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

[2] "Re: [PATCHv4 for-3.13 00/10] create_flow/destroy_flow fixes for v3.13"

http://marc.info/?i=1387493822.11925.217.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org
http://mid.gmane.org/1387493822.11925.217.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org

[3] "[PATCH 0/4] IB/uverbs: check request parameters"

http://marc.info/?i=cover.1405884453.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
http://mid.gmane.org/cover.1405884453.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

Yann Droneaud (6):
  IB/uverbs: check userspace input buffer size
  IB/uverbs: check userspace output buffer size
  IB/uverbs: check userspace output buffer size in ib_uverbs_poll_cq()
  IB/uverbs: subtract command header from input size
  IB/uverbs: move cast from u64 to void __user pointer to its own
    variable
  IB/uverbs: check access to userspace response buffer

 drivers/infiniband/core/uverbs_cmd.c         | 449 +++++++++++++++++++++------
 drivers/infiniband/core/uverbs_main.c        |  29 +-
 drivers/infiniband/hw/mlx5/cq.c              |   6 +-
 drivers/infiniband/hw/mlx5/main.c            |   2 +-
 drivers/infiniband/hw/mlx5/srq.c             |   6 +-
 drivers/infiniband/hw/mthca/mthca_provider.c |   2 +-
 6 files changed, 382 insertions(+), 112 deletions(-)

-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv1 1/6] IB/uverbs: check userspace input buffer size

[Yann Droneaud]

This patch makes uverbs functions check the length of the input
buffer before reading the command content.

This will detect truncated command and will prevent uverbs from
reading past userspace provided buffer.

Additionally, it will also prevent underflow while computing
remaining input space. Such underflow would set inlen field in
struct ib_udata in call to INIT_UDATA() to a meaningless value.

For example:

    INIT_UDATA(&udata, buf + sizeof cmd,
               (unsigned long) cmd.response + sizeof resp,
               in_len - sizeof cmd, out_len - sizeof resp);

If in_len is less than sizeof(cmd), the result of subtraction
is a negative value since in_len is an int per function prototype.
But inlen field in struct ib_udata is unsigned, so the result is
an overly large value in struct ib_udata. Such value will make
further size checking useless, allowing more out of bound read
in providers (eg. hw/) code.

Note that checking the input size and preventing the underflow
and/or overflow might break existing userspace program that
incorrectly set the buffer length in a uverbs request.

It's theoretically possible for a userspace driver to send
a request with a wrong size that can still be processed
correctly with the kernel:

Let's build a request through fake 'libibverbs' and
'libvendor-rdma' userspace driver.

It's a request which might be valid with current kernel
but will set inlen to (size_t) -1 while it will be possible
for vendor driver (eg. provider, under hw/) to read vendor
fields:

    #include <infiniband/kern-abi.h>

    /* aka. struct ib_uverbs_cmd_hdr */
    struct ibv_cmd_hdr {
        __u32 command;
        __u16 in_words;
        __u16 out_words;
    };

    struct vendor_create_cq {
        struct ibv_create_cq ibv_cmd;
        /* vendor defined fields */
    };

    struct vendor_create_cq_resp resp {
        struct ibv_create_cq_resp ibv_resp;
        /* vendor defined fields */
    };

    struct ibv_cq *vendor_create_cq_broken_inlen(...)
    {
        struct vendor_create_cq      cmd;
        struct vendor_create_cq_resp resp;
        size_t cmd_size;

        /* the command header size must be subtracted here
           to ensure (inlen - sizeof(struct ib_uverbs_create_cq)
           is equal to 0 in ib_uverbs_create_cq(), as the header
           size is not subtracted in ib_uverbs_write() before
           invoking ib_uverbs_create_cq() */

        assert(sizeof(struct ibv_create_cq) >= sizeof(struct ibv_cmd_hdr));
        cmd_size = sizeof(struct ibv_create_cq) -
                   sizeof(struct ibv_cmd_hdr);

        /* instead of 0, make (inlen - sizeof(cmd)) underflow */
        cmd_size--;

        /* For the command to be valid in ib_uverbs_write(),
           it size must be at least equal to sizeof(struct
           ib_uverbs_cmd_hdr). */

        assert(cmd_size >= sizeof(struct ibv_cmd_hdr));
        ...

        IBV_INIT_CMD_RESP(&cmd,
                          cmd_size,
                          CREATE_CQ,
                          &resp,
                          sizeof(resp));

        ...

        write(context->cmd_fd,
              &cmd,
              cmd_size);

        ...

    }

This example shows how a request can be created to trigger
un-handled underflow while allowing provider (eg. hw/) to
process the request.

The provided patch will make it impossible to do it on
patched kernels.

Applications using libibverbs are not going to be affected
by this patch, but unknown broken applications using directly
kernel uverbs API may fail.

Link: http://marc.info/?i=1387493822.11925.217.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org
Link: http://marc.info/?i=cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/core/uverbs_cmd.c | 105 +++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index a9f048990dfc..6d26bfab1bc6 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -296,6 +296,9 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
 	struct file			 *filp;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -455,6 +458,9 @@ ssize_t ib_uverbs_query_device(struct ib_uverbs_file *file,
 	struct ib_device_attr              attr;
 	int                                ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -484,6 +490,9 @@ ssize_t ib_uverbs_query_port(struct ib_uverbs_file *file,
 	struct ib_port_attr              attr;
 	int                              ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -536,6 +545,9 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
 	struct ib_pd                  *pd;
 	int                            ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -607,6 +619,9 @@ ssize_t ib_uverbs_dealloc_pd(struct ib_uverbs_file *file,
 	struct ib_uobject          *uobj;
 	int                         ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -733,6 +748,9 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file,
 	int				ret = 0;
 	int				new_xrcd = 0;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -868,6 +886,9 @@ ssize_t ib_uverbs_close_xrcd(struct ib_uverbs_file *file,
 	int                         live;
 	int                         ret = 0;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -945,6 +966,9 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
 	struct ib_mr                *mr;
 	int                          ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -1055,6 +1079,9 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
 	int                          ret;
 	struct ib_uobject	    *uobj;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(resp))
 		return -ENOSPC;
 
@@ -1144,6 +1171,9 @@ ssize_t ib_uverbs_dereg_mr(struct ib_uverbs_file *file,
 	struct ib_uobject	 *uobj;
 	int                       ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -1184,6 +1214,9 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file,
 	struct ib_mw                  *mw;
 	int                            ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(resp))
 		return -ENOSPC;
 
@@ -1264,6 +1297,9 @@ ssize_t ib_uverbs_dealloc_mw(struct ib_uverbs_file *file,
 	struct ib_uobject	   *uobj;
 	int                         ret = -EINVAL;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof(cmd)))
 		return -EFAULT;
 
@@ -1302,6 +1338,9 @@ ssize_t ib_uverbs_create_comp_channel(struct ib_uverbs_file *file,
 	struct file				  *filp;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -1342,6 +1381,9 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
 	struct ib_cq                   *cq;
 	int                             ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -1441,6 +1483,9 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
 	struct ib_cq			*cq;
 	int				ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -1506,6 +1551,9 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 	struct ib_wc                   wc;
 	int                            ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -1552,6 +1600,9 @@ ssize_t ib_uverbs_req_notify_cq(struct ib_uverbs_file *file,
 	struct ib_uverbs_req_notify_cq cmd;
 	struct ib_cq                  *cq;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -1579,6 +1630,9 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
 	struct ib_uverbs_event_file	*ev_file;
 	int                        	 ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -1637,6 +1691,9 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
 	struct ib_qp_init_attr          attr;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -1827,6 +1884,9 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
 	struct ib_qp_open_attr          attr;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -1919,6 +1979,9 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 	struct ib_qp_init_attr         *init_attr;
 	int                            ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -2032,6 +2095,9 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
 	struct ib_qp_attr         *attr;
 	int                        ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -2129,6 +2195,9 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
 	struct ib_uqp_object        	*obj;
 	int                        	 ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -2189,6 +2258,9 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file,
 	int				is_ud;
 	ssize_t                         ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -2430,6 +2502,9 @@ ssize_t ib_uverbs_post_recv(struct ib_uverbs_file *file,
 	struct ib_qp                   *qp;
 	ssize_t                         ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -2479,6 +2554,9 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file,
 	struct ib_srq                      *srq;
 	ssize_t                             ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -2530,6 +2608,9 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
 	struct ib_ah_attr		attr;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -2618,6 +2699,9 @@ ssize_t ib_uverbs_destroy_ah(struct ib_uverbs_file *file,
 	struct ib_uobject	   *uobj;
 	int			    ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -2656,6 +2740,9 @@ ssize_t ib_uverbs_attach_mcast(struct ib_uverbs_file *file,
 	struct ib_uverbs_mcast_entry *mcast;
 	int                           ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -2703,6 +2790,9 @@ ssize_t ib_uverbs_detach_mcast(struct ib_uverbs_file *file,
 	struct ib_uverbs_mcast_entry *mcast;
 	int                           ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -3118,6 +3208,9 @@ ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file,
 	struct ib_udata                  udata;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -3151,6 +3244,9 @@ ssize_t ib_uverbs_create_xsrq(struct ib_uverbs_file *file,
 	struct ib_udata                  udata;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -3178,6 +3274,9 @@ ssize_t ib_uverbs_modify_srq(struct ib_uverbs_file *file,
 	struct ib_srq_attr          attr;
 	int                         ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -3208,6 +3307,9 @@ ssize_t ib_uverbs_query_srq(struct ib_uverbs_file *file,
 	struct ib_srq                   *srq;
 	int                             ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;
 
@@ -3251,6 +3353,9 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
 	struct ib_usrq_object		 *us;
 	enum ib_srq_type		  srq_type;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv1 2/6] IB/uverbs: check userspace output buffer size

[Yann Droneaud]

This patch makes uverbs functions check the length of the
output buffer.

This will prevent uverbs from writing past userspace provided
buffer.

Additionally, it will prevent an underflow while computing
remaining output space. Such underflow would set outlen field in
struct ib_udata in call to INIT_UDATA() to a meaningless value.

For example:

    INIT_UDATA(&udata, buf + sizeof cmd,
               (unsigned long) cmd.response + sizeof resp,
               in_len - sizeof cmd, out_len - sizeof resp);

If out_len is less than sizeof(resp), the result of subtraction
is a negative value since out_len is an int per function prototype.
But outlen field in struct ib_udata is unsigned, so the result is
an overly large value in struct ib_udata. Such value will make
further size checking useless, allowing more out of bound write
in providers (eg. hw/) code.

Note that checking the output size and preventing the underflow
and/or overflow might break existing userspace program that
incorrectly set the buffer length in a uverbs request.

It's theoretically possible for a userspace driver to send
a request with a wrong size that can still be processed
correctly with the kernel:

Let's build a request through fake 'libibverbs' and
'libvendor-rdma' userspace driver.

It's a request which might be valid with current kernel
but will set outlen to (size_t) -1 while it will be possible
for vendor driver (eg. provider, under hw/) to write vendor
fields:

    #include <infiniband/kern-abi.h>

    /* aka. struct ib_uverbs_cmd_hdr */
    struct ibv_cmd_hdr {
        __u32 command;
        __u16 in_words;
        __u16 out_words;
    };

    struct vendor_resize_cq {
        struct ibv_resize_cq ibv_cmd;
        /* vendor defined fields */
    };

    struct vendor_resize_cq_resp resp {
        struct ibv_resize_cq_resp ibv_resp;
        /* vendor defined fields */
    };

    struct ibv_cq *vendor_resize_cq_broken_outlen(...)
    {
        struct vendor_resize_cq      cmd;
        struct vendor_resize_cq_resp resp;
        size_t                       resp_size;

        ...

        /* slightly less than struct ib_uverbs_resize_cq_resp */
        resp_size = sizeof(struct ibv_create_cq_resp);
        resp_size--;

        ...

        IBV_INIT_CMD_RESP(&cmd,
                          sizeof(cmd),
                          RESIZE_CQ,
                          &resp,
                          resp_size);
        ...
        write(context->cmd_fd,
              &cmd,
              sizeof(cmd));
        ...
    }

This example shows how a request can be created to trigger
un-handled underflow while allowing provider (eg. hw/) to
process the request.

The provided patch will make it impossible to do it on
patched kernels.

Applications using libibverbs are not going to be affected
by this patch, but unknown broken applications using directly
kernel uverbs API may fail.

Link: http://marc.info/?i=1387493822.11925.217.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org
Link: http://marc.info/?i=cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/core/uverbs_cmd.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 6d26bfab1bc6..a3a3b6eafd1d 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1486,6 +1486,9 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -1554,6 +1557,9 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -1633,6 +1639,9 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -1982,6 +1991,9 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -2198,6 +2210,9 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -2261,6 +2276,9 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -2505,6 +2523,9 @@ ssize_t ib_uverbs_post_recv(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -2557,6 +2578,9 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
@@ -3356,6 +3380,9 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Yann Droneaud]

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 writing 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;
    }

The provided patch will make it impossible to do it on
patched kernels.

Applications using libibverbs are not going to be affected
by this patch, but unknown broken applications using directly
kernel uverbs API may fail.

Link: http://marc.info/?i=cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/core/uverbs_cmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index a3a3b6eafd1d..147f61c8692b 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1563,6 +1563,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;
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv1 4/6] IB/uverbs: subtract command header from input size

[Yann Droneaud]

The uverbs handler functions are given a pointer
to the command buffer, which point right after the
command header.

But the size given to the function is the total size,
including the command header size.

The size of the command header must be subtracted
from the input length before calling the uverbs handler
function so that the parameters are in par.

This patch makes ib_uverbs_write() subtract the header
size from the total size when calling uverbs function.

As the return value from the uverbs function will
be the size without the header (or a negative value),
it must not be used as-is. Instead the function use
the total size as return value when no error occurred.

Additionally, drivers (mthca, mlx5) taking the size
of command header in account in their size checks are
modified accordingly.

This patch is a pre-requisite to make bound checking
effective.

Link: http://marc.info/?i=cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/core/uverbs_main.c        | 18 +++++++++++++-----
 drivers/infiniband/hw/mlx5/cq.c              |  6 ++----
 drivers/infiniband/hw/mlx5/main.c            |  2 +-
 drivers/infiniband/hw/mlx5/srq.c             |  6 ++----
 drivers/infiniband/hw/mthca/mthca_provider.c |  2 +-
 5 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 88cce9bb72fe..6ab287c6693c 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -601,6 +601,7 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
 {
 	struct ib_uverbs_file *file = filp->private_data;
 	struct ib_uverbs_cmd_hdr hdr;
+	const size_t written_count = count;
 	__u32 flags;
 
 	if (count < sizeof hdr)
@@ -614,6 +615,7 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
 
 	if (!flags) {
 		__u32 command;
+		ssize_t err;
 
 		if (hdr.command & ~(__u32)(IB_USER_VERBS_CMD_FLAGS_MASK |
 					   IB_USER_VERBS_CMD_COMMAND_MASK))
@@ -635,10 +637,17 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
 		if (hdr.in_words * 4 != count)
 			return -EINVAL;
 
-		return uverbs_cmd_table[command](file,
-						 buf + sizeof(hdr),
-						 hdr.in_words * 4,
-						 hdr.out_words * 4);
+		count -= sizeof(hdr);
+		buf += sizeof(hdr);
+
+		err = uverbs_cmd_table[command](file,
+						buf,
+						count,
+						hdr.out_words * 4);
+		if (err < 0)
+			return err;
+
+		return written_count;
 
 	} else if (flags == IB_USER_VERBS_CMD_FLAG_EXTENDED) {
 		__u32 command;
@@ -647,7 +656,6 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
 		struct ib_udata ucore;
 		struct ib_udata uhw;
 		int err;
-		size_t written_count = count;
 
 		if (hdr.command & ~(__u32)(IB_USER_VERBS_CMD_FLAGS_MASK |
 					   IB_USER_VERBS_CMD_COMMAND_MASK))
diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
index 2ee6b1051975..3b38febf685e 100644
--- a/drivers/infiniband/hw/mlx5/cq.c
+++ b/drivers/infiniband/hw/mlx5/cq.c
@@ -613,10 +613,8 @@ static int create_cq_user(struct mlx5_ib_dev *dev, struct ib_udata *udata,
 	int ncont;
 	int err;
 
-	ucmdlen =
-		(udata->inlen - sizeof(struct ib_uverbs_cmd_hdr) <
-		 sizeof(ucmd)) ? (sizeof(ucmd) -
-				  sizeof(ucmd.reserved)) : sizeof(ucmd);
+	ucmdlen = (udata->inlen < sizeof(ucmd)) ?
+	    (sizeof(ucmd) - sizeof(ucmd.reserved)) : sizeof(ucmd);
 
 	if (ib_copy_from_udata(&ucmd, udata, ucmdlen))
 		return -EFAULT;
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 57c9809e8b87..d951bb9a89a3 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -408,7 +408,7 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev,
 		return ERR_PTR(-EAGAIN);
 
 	memset(&req, 0, sizeof(req));
-	reqlen = udata->inlen - sizeof(struct ib_uverbs_cmd_hdr);
+	reqlen = udata->inlen;
 	if (reqlen == sizeof(struct mlx5_ib_alloc_ucontext_req))
 		ver = 0;
 	else if (reqlen == sizeof(struct mlx5_ib_alloc_ucontext_req_v2))
diff --git a/drivers/infiniband/hw/mlx5/srq.c b/drivers/infiniband/hw/mlx5/srq.c
index 02d77a29764d..c342bd47e168 100644
--- a/drivers/infiniband/hw/mlx5/srq.c
+++ b/drivers/infiniband/hw/mlx5/srq.c
@@ -86,10 +86,8 @@ static int create_srq_user(struct ib_pd *pd, struct mlx5_ib_srq *srq,
 	int ncont;
 	u32 offset;
 
-	ucmdlen =
-		(udata->inlen - sizeof(struct ib_uverbs_cmd_hdr) <
-		 sizeof(ucmd)) ? (sizeof(ucmd) -
-				  sizeof(ucmd.reserved)) : sizeof(ucmd);
+	ucmdlen = (udata->inlen < sizeof(ucmd)) ?
+	    (sizeof(ucmd) - sizeof(ucmd.reserved)) : sizeof(ucmd);
 
 	if (ib_copy_from_udata(&ucmd, udata, ucmdlen)) {
 		mlx5_ib_dbg(dev, "failed copy udata\n");
diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c b/drivers/infiniband/hw/mthca/mthca_provider.c
index 415f8e1a54db..e5933b032fc3 100644
--- a/drivers/infiniband/hw/mthca/mthca_provider.c
+++ b/drivers/infiniband/hw/mthca/mthca_provider.c
@@ -986,7 +986,7 @@ static struct ib_mr *mthca_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
 	int err = 0;
 	int write_mtt_size;
 
-	if (udata->inlen - sizeof (struct ib_uverbs_cmd_hdr) < sizeof ucmd) {
+	if (udata->inlen < sizeof ucmd) {
 		if (!to_mucontext(pd->uobject->context)->reg_mr_warned) {
 			mthca_warn(dev, "Process '%s' did not pass in MR attrs.\n",
 				   current->comm);
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv1 5/6] IB/uverbs: move cast from u64 to void __user pointer to its own variable

[Yann Droneaud]

Use a dedicated variable to hold address of the response buffer after
conversion (aka. cast) from u64 to void __user *, so that this value
could be used for INIT_UDATA(), copy_to_user() or access_ok(), reducing
the visual clutter introduced by the multiple casts.

This variable will be used for multiple purposes:
- check pointer / size validity with access_ok()
- setup ib_udata through INIT_UDATA()
- in copy_to_user()

When implicit cast will be removed from INIT_UDATA() macro,
this variable will be used so that the address will be stored
in pointer and will not trigger warnings from compiler.

Link: http://marc.info/?i=cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/core/uverbs_cmd.c  | 175 ++++++++++++++++++++++------------
 drivers/infiniband/core/uverbs_main.c |  11 ++-
 2 files changed, 121 insertions(+), 65 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 147f61c8692b..046ca87480e9 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -287,6 +287,7 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_get_context      cmd;
 	struct ib_uverbs_get_context_resp resp;
+	char __user			 *response;
 	struct ib_udata                   udata;
 	struct ib_device                 *ibdev = file->device->ib_dev;
 #ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
@@ -305,6 +306,8 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	mutex_lock(&file->mutex);
 
 	if (file->ucontext) {
@@ -313,7 +316,7 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
 	}
 
 	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
+		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
 
 	ucontext = ibdev->alloc_ucontext(ibdev, &udata);
@@ -364,8 +367,7 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
 		goto err_fd;
 	}
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp)) {
+	if (copy_to_user(response, &resp, sizeof(resp))) {
 		ret = -EFAULT;
 		goto err_file;
 	}
@@ -455,6 +457,7 @@ ssize_t ib_uverbs_query_device(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_query_device      cmd;
 	struct ib_uverbs_query_device_resp resp;
+	char __user			  *response;
 	struct ib_device_attr              attr;
 	int                                ret;
 
@@ -467,6 +470,8 @@ ssize_t ib_uverbs_query_device(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	ret = ib_query_device(file->device->ib_dev, &attr);
 	if (ret)
 		return ret;
@@ -474,8 +479,7 @@ ssize_t ib_uverbs_query_device(struct ib_uverbs_file *file,
 	memset(&resp, 0, sizeof resp);
 	copy_query_dev_fields(file, &resp, &attr);
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp))
+	if (copy_to_user(response, &resp, sizeof(resp)))
 		return -EFAULT;
 
 	return in_len;
@@ -487,6 +491,7 @@ ssize_t ib_uverbs_query_port(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_query_port      cmd;
 	struct ib_uverbs_query_port_resp resp;
+	char __user			*response;
 	struct ib_port_attr              attr;
 	int                              ret;
 
@@ -499,6 +504,8 @@ ssize_t ib_uverbs_query_port(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	ret = ib_query_port(file->device->ib_dev, cmd.port_num, &attr);
 	if (ret)
 		return ret;
@@ -527,8 +534,7 @@ ssize_t ib_uverbs_query_port(struct ib_uverbs_file *file,
 	resp.link_layer      = rdma_port_get_link_layer(file->device->ib_dev,
 							cmd.port_num);
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp))
+	if (copy_to_user(response, &resp, sizeof(resp)))
 		return -EFAULT;
 
 	return in_len;
@@ -540,6 +546,7 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_alloc_pd      cmd;
 	struct ib_uverbs_alloc_pd_resp resp;
+	char __user		      *response;
 	struct ib_udata                udata;
 	struct ib_uobject             *uobj;
 	struct ib_pd                  *pd;
@@ -554,8 +561,10 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
+		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
 
 	uobj = kmalloc(sizeof *uobj, GFP_KERNEL);
@@ -584,8 +593,7 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
 	memset(&resp, 0, sizeof resp);
 	resp.pd_handle = uobj->id;
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp)) {
+	if (copy_to_user(response, &resp, sizeof(resp))) {
 		ret = -EFAULT;
 		goto err_copy;
 	}
@@ -740,6 +748,7 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_open_xrcd	cmd;
 	struct ib_uverbs_open_xrcd_resp	resp;
+	char __user		       *response;
 	struct ib_udata			udata;
 	struct ib_uxrcd_object         *obj;
 	struct ib_xrcd                 *xrcd = NULL;
@@ -757,8 +766,10 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
+		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof  resp);
 
 	mutex_lock(&file->device->xrcd_tree_mutex);
@@ -830,8 +841,7 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file,
 		atomic_inc(&xrcd->usecnt);
 	}
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp)) {
+	if (copy_to_user(response, &resp, sizeof(resp))) {
 		ret = -EFAULT;
 		goto err_copy;
 	}
@@ -960,6 +970,7 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_reg_mr      cmd;
 	struct ib_uverbs_reg_mr_resp resp;
+	char __user		    *response;
 	struct ib_udata              udata;
 	struct ib_uobject           *uobj;
 	struct ib_pd                *pd;
@@ -975,8 +986,10 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
+		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
 
 	if ((cmd.start & ~PAGE_MASK) != (cmd.hca_va & ~PAGE_MASK))
@@ -1034,8 +1047,7 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
 	resp.rkey      = mr->rkey;
 	resp.mr_handle = uobj->id;
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp)) {
+	if (copy_to_user(response, &resp, sizeof(resp))) {
 		ret = -EFAULT;
 		goto err_copy;
 	}
@@ -1072,6 +1084,7 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_rereg_mr      cmd;
 	struct ib_uverbs_rereg_mr_resp resp;
+	char __user                   *response;
 	struct ib_udata              udata;
 	struct ib_pd                *pd = NULL;
 	struct ib_mr                *mr;
@@ -1088,8 +1101,10 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof(cmd)))
 		return -EFAULT;
 
+	response = (char __user *)(unsigned long)cmd.response;
+
 	INIT_UDATA(&udata, buf + sizeof(cmd),
-		   (unsigned long) cmd.response + sizeof(resp),
+		   response + sizeof(resp),
 		   in_len - sizeof(cmd), out_len - sizeof(resp));
 
 	if (cmd.flags & ~IB_MR_REREG_SUPPORTED || !cmd.flags)
@@ -1145,8 +1160,7 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
 	resp.lkey      = mr->lkey;
 	resp.rkey      = mr->rkey;
 
-	if (copy_to_user((void __user *)(unsigned long)cmd.response,
-			 &resp, sizeof(resp)))
+	if (copy_to_user(response, &resp, sizeof(resp)))
 		ret = -EFAULT;
 	else
 		ret = in_len;
@@ -1209,6 +1223,7 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_alloc_mw      cmd;
 	struct ib_uverbs_alloc_mw_resp resp;
+	char __user		      *response;
 	struct ib_uobject             *uobj;
 	struct ib_pd                  *pd;
 	struct ib_mw                  *mw;
@@ -1223,6 +1238,8 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof(cmd)))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	uobj = kmalloc(sizeof(*uobj), GFP_KERNEL);
 	if (!uobj)
 		return -ENOMEM;
@@ -1256,8 +1273,7 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file,
 	resp.rkey      = mw->rkey;
 	resp.mw_handle = uobj->id;
 
-	if (copy_to_user((void __user *)(unsigned long)cmd.response,
-			 &resp, sizeof(resp))) {
+	if (copy_to_user(response, &resp, sizeof(resp))) {
 		ret = -EFAULT;
 		goto err_copy;
 	}
@@ -1335,6 +1351,7 @@ ssize_t ib_uverbs_create_comp_channel(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_create_comp_channel	   cmd;
 	struct ib_uverbs_create_comp_channel_resp  resp;
+	char __user				  *response;
 	struct file				  *filp;
 	int ret;
 
@@ -1347,6 +1364,8 @@ ssize_t ib_uverbs_create_comp_channel(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	ret = get_unused_fd_flags(O_CLOEXEC);
 	if (ret < 0)
 		return ret;
@@ -1358,8 +1377,7 @@ ssize_t ib_uverbs_create_comp_channel(struct ib_uverbs_file *file,
 		return PTR_ERR(filp);
 	}
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp)) {
+	if (copy_to_user(response, &resp, sizeof(resp))) {
 		put_unused_fd(resp.fd);
 		fput(filp);
 		return -EFAULT;
@@ -1375,6 +1393,7 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_create_cq      cmd;
 	struct ib_uverbs_create_cq_resp resp;
+	char __user		       *response;
 	struct ib_udata                 udata;
 	struct ib_ucq_object           *obj;
 	struct ib_uverbs_event_file    *ev_file = NULL;
@@ -1390,8 +1409,10 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
+		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
 
 	if (cmd.comp_vector >= file->device->num_comp_vectors)
@@ -1442,8 +1463,7 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
 	resp.cq_handle = obj->uobject.id;
 	resp.cqe       = cq->cqe;
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp)) {
+	if (copy_to_user(response, &resp, sizeof(resp))) {
 		ret = -EFAULT;
 		goto err_copy;
 	}
@@ -1479,6 +1499,7 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_resize_cq	cmd;
 	struct ib_uverbs_resize_cq_resp	resp;
+	char __user		       *response;
 	struct ib_udata                 udata;
 	struct ib_cq			*cq;
 	int				ret = -EINVAL;
@@ -1492,8 +1513,10 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
+		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
 
 	cq = idr_read_cq(cmd.cq_handle, file->ucontext, 0);
@@ -1506,8 +1529,7 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
 
 	resp.cqe = cq->cqe;
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp.cqe))
+	if (copy_to_user(response, &resp, sizeof(resp.cqe)))
 		ret = -EFAULT;
 
 out:
@@ -1548,7 +1570,7 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_poll_cq       cmd;
 	struct ib_uverbs_poll_cq_resp  resp;
-	u8 __user                     *header_ptr;
+	char __user		      *response;
 	u8 __user                     *data_ptr;
 	struct ib_cq                  *cq;
 	struct ib_wc                   wc;
@@ -1563,6 +1585,8 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	if ((out_len - sizeof resp)/(sizeof(struct ib_uverbs_wc)) < cmd.ne)
 		return -ENOSPC;
 
@@ -1571,8 +1595,7 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 		return -EINVAL;
 
 	/* we copy a struct ib_uverbs_poll_cq_resp to user space */
-	header_ptr = (void __user *)(unsigned long) cmd.response;
-	data_ptr = header_ptr + sizeof resp;
+	data_ptr = response + sizeof resp;
 
 	memset(&resp, 0, sizeof resp);
 	while (resp.count < cmd.ne) {
@@ -1590,7 +1613,7 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 		++resp.count;
 	}
 
-	if (copy_to_user(header_ptr, &resp, sizeof resp)) {
+	if (copy_to_user(response, &resp, sizeof resp)) {
 		ret = -EFAULT;
 		goto out_put;
 	}
@@ -1633,6 +1656,7 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_destroy_cq      cmd;
 	struct ib_uverbs_destroy_cq_resp resp;
+	char __user			*response;
 	struct ib_uobject		*uobj;
 	struct ib_cq               	*cq;
 	struct ib_ucq_object        	*obj;
@@ -1648,6 +1672,8 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	uobj = idr_write_uobj(&ib_uverbs_cq_idr, cmd.cq_handle, file->ucontext);
 	if (!uobj)
 		return -EINVAL;
@@ -1678,8 +1704,7 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
 
 	put_uobj(uobj);
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp))
+	if (copy_to_user(response, &resp, sizeof(resp)))
 		return -EFAULT;
 
 	return in_len;
@@ -1691,6 +1716,7 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_create_qp      cmd;
 	struct ib_uverbs_create_qp_resp resp;
+	char __user		       *response;
 	struct ib_udata                 udata;
 	struct ib_uqp_object           *obj;
 	struct ib_device	       *device;
@@ -1712,11 +1738,13 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	if (cmd.qp_type == IB_QPT_RAW_PACKET && !capable(CAP_NET_RAW))
 		return -EPERM;
 
 	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
+		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
 
 	obj = kzalloc(sizeof *obj, GFP_KERNEL);
@@ -1829,8 +1857,7 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
 	resp.max_send_wr     = attr.cap.max_send_wr;
 	resp.max_inline_data = attr.cap.max_inline_data;
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp)) {
+	if (copy_to_user(response, &resp, sizeof(resp))) {
 		ret = -EFAULT;
 		goto err_copy;
 	}
@@ -1888,6 +1915,7 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_open_qp        cmd;
 	struct ib_uverbs_create_qp_resp resp;
+	char __user		       *response;
 	struct ib_udata                 udata;
 	struct ib_uqp_object           *obj;
 	struct ib_xrcd		       *xrcd;
@@ -1905,8 +1933,10 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
+		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
 
 	obj = kmalloc(sizeof *obj, GFP_KERNEL);
@@ -1948,8 +1978,7 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
 	resp.qpn       = qp->qp_num;
 	resp.qp_handle = obj->uevent.uobject.id;
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp)) {
+	if (copy_to_user(response, &resp, sizeof(resp))) {
 		ret = -EFAULT;
 		goto err_remove;
 	}
@@ -1986,6 +2015,7 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_query_qp      cmd;
 	struct ib_uverbs_query_qp_resp resp;
+	char __user		       *response;
 	struct ib_qp                   *qp;
 	struct ib_qp_attr              *attr;
 	struct ib_qp_init_attr         *init_attr;
@@ -2000,6 +2030,8 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	attr      = kmalloc(sizeof *attr, GFP_KERNEL);
 	init_attr = kmalloc(sizeof *init_attr, GFP_KERNEL);
 	if (!attr || !init_attr) {
@@ -2075,8 +2107,7 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 	resp.max_inline_data        = init_attr->cap.max_inline_data;
 	resp.sq_sig_all             = init_attr->sq_sig_type == IB_SIGNAL_ALL_WR;
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp))
+	if (copy_to_user(response, &resp, sizeof(resp)))
 		ret = -EFAULT;
 
 out:
@@ -2205,6 +2236,7 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_destroy_qp      cmd;
 	struct ib_uverbs_destroy_qp_resp resp;
+	char __user			*response;
 	struct ib_uobject		*uobj;
 	struct ib_qp               	*qp;
 	struct ib_uqp_object        	*obj;
@@ -2219,6 +2251,8 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	memset(&resp, 0, sizeof resp);
 
 	uobj = idr_write_uobj(&ib_uverbs_qp_idr, cmd.qp_handle, file->ucontext);
@@ -2256,8 +2290,7 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
 
 	put_uobj(uobj);
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp))
+	if (copy_to_user(response, &resp, sizeof(resp)))
 		return -EFAULT;
 
 	return in_len;
@@ -2269,6 +2302,7 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_post_send      cmd;
 	struct ib_uverbs_post_send_resp resp;
+	char __user		       *response;
 	struct ib_uverbs_send_wr       *user_wr;
 	struct ib_send_wr              *wr = NULL, *last, *next, *bad_wr;
 	struct ib_qp                   *qp;
@@ -2285,6 +2319,8 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	if (in_len < sizeof cmd + cmd.wqe_size * cmd.wr_count +
 	    cmd.sge_count * sizeof (struct ib_uverbs_sge))
 		return -EINVAL;
@@ -2407,8 +2443,7 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file,
 				break;
 		}
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp))
+	if (copy_to_user(response, &resp, sizeof(resp)))
 		ret = -EFAULT;
 
 out_put:
@@ -2519,6 +2554,7 @@ ssize_t ib_uverbs_post_recv(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_post_recv      cmd;
 	struct ib_uverbs_post_recv_resp resp;
+	char __user		       *response;
 	struct ib_recv_wr              *wr, *next, *bad_wr;
 	struct ib_qp                   *qp;
 	ssize_t                         ret = -EINVAL;
@@ -2532,6 +2568,8 @@ ssize_t ib_uverbs_post_recv(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	wr = ib_uverbs_unmarshall_recv(buf + sizeof cmd,
 				       in_len - sizeof cmd, cmd.wr_count,
 				       cmd.sge_count, cmd.wqe_size);
@@ -2554,8 +2592,7 @@ ssize_t ib_uverbs_post_recv(struct ib_uverbs_file *file,
 				break;
 		}
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp))
+	if (copy_to_user(response, &resp, sizeof(resp)))
 		ret = -EFAULT;
 
 out:
@@ -2574,6 +2611,7 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_post_srq_recv      cmd;
 	struct ib_uverbs_post_srq_recv_resp resp;
+	char __user			   *response;
 	struct ib_recv_wr                  *wr, *next, *bad_wr;
 	struct ib_srq                      *srq;
 	ssize_t                             ret = -EINVAL;
@@ -2587,6 +2625,8 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	wr = ib_uverbs_unmarshall_recv(buf + sizeof cmd,
 				       in_len - sizeof cmd, cmd.wr_count,
 				       cmd.sge_count, cmd.wqe_size);
@@ -2609,8 +2649,7 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file,
 				break;
 		}
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp))
+	if (copy_to_user(response, &resp, sizeof(resp)))
 		ret = -EFAULT;
 
 out:
@@ -2629,6 +2668,7 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_create_ah	 cmd;
 	struct ib_uverbs_create_ah_resp	 resp;
+	char __user			*response;
 	struct ib_uobject		*uobj;
 	struct ib_pd			*pd;
 	struct ib_ah			*ah;
@@ -2644,6 +2684,8 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	uobj = kmalloc(sizeof *uobj, GFP_KERNEL);
 	if (!uobj)
 		return -ENOMEM;
@@ -2686,8 +2728,7 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
 
 	resp.ah_handle = uobj->id;
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp)) {
+	if (copy_to_user(response, &resp, sizeof(resp))) {
 		ret = -EFAULT;
 		goto err_copy;
 	}
@@ -3097,6 +3138,7 @@ static int __uverbs_create_xsrq(struct ib_uverbs_file *file,
 				struct ib_udata *udata)
 {
 	struct ib_uverbs_create_srq_resp resp;
+	char __user			*response;
 	struct ib_usrq_object           *obj;
 	struct ib_pd                    *pd;
 	struct ib_srq                   *srq;
@@ -3104,6 +3146,8 @@ static int __uverbs_create_xsrq(struct ib_uverbs_file *file,
 	struct ib_srq_init_attr          attr;
 	int ret;
 
+	response = (void __user *)(unsigned long)cmd->response;
+
 	obj = kmalloc(sizeof *obj, GFP_KERNEL);
 	if (!obj)
 		return -ENOMEM;
@@ -3179,8 +3223,7 @@ static int __uverbs_create_xsrq(struct ib_uverbs_file *file,
 	if (cmd->srq_type == IB_SRQT_XRC)
 		resp.srqn = srq->ext.xrc.srq_num;
 
-	if (copy_to_user((void __user *) (unsigned long) cmd->response,
-			 &resp, sizeof resp)) {
+	if (copy_to_user(response, &resp, sizeof(resp))) {
 		ret = -EFAULT;
 		goto err_copy;
 	}
@@ -3232,6 +3275,7 @@ ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file,
 	struct ib_uverbs_create_srq      cmd;
 	struct ib_uverbs_create_xsrq     xcmd;
 	struct ib_uverbs_create_srq_resp resp;
+	char __user			*response;
 	struct ib_udata                  udata;
 	int ret;
 
@@ -3244,6 +3288,8 @@ ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	xcmd.response	 = cmd.response;
 	xcmd.user_handle = cmd.user_handle;
 	xcmd.srq_type	 = IB_SRQT_BASIC;
@@ -3253,7 +3299,7 @@ ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file,
 	xcmd.srq_limit	 = cmd.srq_limit;
 
 	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
+		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
 
 	ret = __uverbs_create_xsrq(file, &xcmd, &udata);
@@ -3268,6 +3314,7 @@ ssize_t ib_uverbs_create_xsrq(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_create_xsrq     cmd;
 	struct ib_uverbs_create_srq_resp resp;
+	char __user			*response;
 	struct ib_udata                  udata;
 	int ret;
 
@@ -3280,8 +3327,10 @@ ssize_t ib_uverbs_create_xsrq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
+		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
 
 	ret = __uverbs_create_xsrq(file, &cmd, &udata);
@@ -3330,6 +3379,7 @@ ssize_t ib_uverbs_query_srq(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_query_srq      cmd;
 	struct ib_uverbs_query_srq_resp resp;
+	char __user		       *response;
 	struct ib_srq_attr              attr;
 	struct ib_srq                   *srq;
 	int                             ret;
@@ -3343,6 +3393,8 @@ ssize_t ib_uverbs_query_srq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	srq = idr_read_srq(cmd.srq_handle, file->ucontext);
 	if (!srq)
 		return -EINVAL;
@@ -3360,8 +3412,7 @@ ssize_t ib_uverbs_query_srq(struct ib_uverbs_file *file,
 	resp.max_sge   = attr.max_sge;
 	resp.srq_limit = attr.srq_limit;
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp))
+	if (copy_to_user(response, &resp, sizeof(resp)))
 		return -EFAULT;
 
 	return in_len;
@@ -3373,6 +3424,7 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
 {
 	struct ib_uverbs_destroy_srq      cmd;
 	struct ib_uverbs_destroy_srq_resp resp;
+	char __user			 *response;
 	struct ib_uobject		 *uobj;
 	struct ib_srq               	 *srq;
 	struct ib_uevent_object        	 *obj;
@@ -3389,6 +3441,8 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	response = (void __user *)(unsigned long)cmd.response;
+
 	uobj = idr_write_uobj(&ib_uverbs_srq_idr, cmd.srq_handle, file->ucontext);
 	if (!uobj)
 		return -EINVAL;
@@ -3423,8 +3477,7 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
 
 	put_uobj(uobj);
 
-	if (copy_to_user((void __user *) (unsigned long) cmd.response,
-			 &resp, sizeof resp))
+	if (copy_to_user(response, &resp, sizeof(resp)))
 		ret = -EFAULT;
 
 	return ret ? ret : in_len;
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 6ab287c6693c..24225d32aa55 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -653,6 +653,7 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
 		__u32 command;
 
 		struct ib_uverbs_ex_cmd_hdr ex_hdr;
+		char __user *response;
 		struct ib_udata ucore;
 		struct ib_udata uhw;
 		int err;
@@ -688,12 +689,14 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
 		if (ex_hdr.cmd_hdr_reserved)
 			return -EINVAL;
 
-		if (ex_hdr.response) {
+		response = (char __user *)(unsigned long)ex_hdr.response;
+
+		if (response) {
 			if (!hdr.out_words && !ex_hdr.provider_out_words)
 				return -EINVAL;
 
 			if (!access_ok(VERIFY_WRITE,
-				       (void __user *) (unsigned long) ex_hdr.response,
+				       response,
 				       (hdr.out_words + ex_hdr.provider_out_words) * 8))
 				return -EFAULT;
 		} else {
@@ -701,12 +704,12 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
 				return -EINVAL;
 		}
 
-		INIT_UDATA_BUF_OR_NULL(&ucore, buf, (unsigned long) ex_hdr.response,
+		INIT_UDATA_BUF_OR_NULL(&ucore, buf, response,
 				       hdr.in_words * 8, hdr.out_words * 8);
 
 		INIT_UDATA_BUF_OR_NULL(&uhw,
 				       buf + ucore.inlen,
-				       (unsigned long) ex_hdr.response + ucore.outlen,
+				       response + ucore.outlen,
 				       ex_hdr.provider_in_words * 8,
 				       ex_hdr.provider_out_words * 8);
 
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv1 6/6] IB/uverbs: check access to userspace response buffer

[Yann Droneaud]

Add checks like the one added in commit 6cc3df840a8
('IB/uverbs: Check access to userspace response buffer
in extended command'), check output buffers with
access_ok(VERIFY_WRITE,...) to ensure they are fully
in userspace memory: if the buffer or a subset of the
buffer is not valid, returns -EFAULT.

Each uverbs functions using a response buffer must have
the same check. This patch adds such checks.

Just like the check in read(2) syscall, it's a sanity
check to detect invalid parameters provided by userspace.
This particular check was added in vfs_read() by Linus
Torvalds for v2.6.12 with following commit message:

https://git.kernel.org/cgit/linux/kernel/git/tglx/history.git/commit/?id=fd770e66c9a65b14ce114e171266cf6f393df502

  Make read/write always do the full "access_ok()" tests.

  The actual user copy will do them too, but only for the
  range that ends up being actually copied. That hides
  bugs when the range has been clamped by file size or other
  issues.

Note: there's no need to check input buffer since vfs_write()
already does access_ok(VERIFY_READ, ...) as part of write()
syscall.

In order to check the output buffer in its entirety with
access_ok() to reject invalid request before any other checks,
this patch move the output size check after copy_from_user(),

It's going to slightly change the behavior of uverbs: an invalid
command partially not mapped, with a response size too short,
would return -EFAULT while previously it would have returned
-ENOSPC. Anyway, it makes more sense to return -EFAULT in such
case.

Applications using libibverbs are not going to be affected
by this patch, but unknown broken applications using directly
kernel uverbs API may fail.

Link: http://marc.info/?i=cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/core/uverbs_cmd.c | 225 +++++++++++++++++++++++------------
 1 file changed, 150 insertions(+), 75 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 046ca87480e9..4fe77814c0e8 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -300,14 +300,17 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	mutex_lock(&file->mutex);
 
 	if (file->ucontext) {
@@ -464,14 +467,17 @@ ssize_t ib_uverbs_query_device(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	ret = ib_query_device(file->device->ib_dev, &attr);
 	if (ret)
 		return ret;
@@ -498,14 +504,17 @@ ssize_t ib_uverbs_query_port(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	ret = ib_query_port(file->device->ib_dev, cmd.port_num, &attr);
 	if (ret)
 		return ret;
@@ -555,14 +564,17 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
 		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
@@ -760,14 +772,17 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
 		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof  resp);
@@ -980,14 +995,17 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
 		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
@@ -1095,14 +1113,17 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
 	if (in_len < sizeof(cmd))
 		return -EINVAL;
 
-	if (out_len < sizeof(resp))
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof(cmd)))
 		return -EFAULT;
 
 	response = (char __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof(resp))
+		return -ENOSPC;
+
 	INIT_UDATA(&udata, buf + sizeof(cmd),
 		   response + sizeof(resp),
 		   in_len - sizeof(cmd), out_len - sizeof(resp));
@@ -1232,14 +1253,17 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file,
 	if (in_len < sizeof(cmd))
 		return -EINVAL;
 
-	if (out_len < sizeof(resp))
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof(cmd)))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof(resp))
+		return -ENOSPC;
+
 	uobj = kmalloc(sizeof(*uobj), GFP_KERNEL);
 	if (!uobj)
 		return -ENOMEM;
@@ -1358,14 +1382,17 @@ ssize_t ib_uverbs_create_comp_channel(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	ret = get_unused_fd_flags(O_CLOEXEC);
 	if (ret < 0)
 		return ret;
@@ -1403,14 +1430,17 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
 		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
@@ -1507,14 +1537,17 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
 		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
@@ -1579,14 +1612,17 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if ((out_len - sizeof resp)/(sizeof(struct ib_uverbs_wc)) < cmd.ne)
 		return -ENOSPC;
 
@@ -1666,14 +1702,17 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	uobj = idr_write_uobj(&ib_uverbs_cq_idr, cmd.cq_handle, file->ucontext);
 	if (!uobj)
 		return -EINVAL;
@@ -1732,14 +1771,17 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (cmd.qp_type == IB_QPT_RAW_PACKET && !capable(CAP_NET_RAW))
 		return -EPERM;
 
@@ -1927,14 +1969,17 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
 		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
@@ -2024,14 +2069,17 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	attr      = kmalloc(sizeof *attr, GFP_KERNEL);
 	init_attr = kmalloc(sizeof *init_attr, GFP_KERNEL);
 	if (!attr || !init_attr) {
@@ -2245,14 +2293,17 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	memset(&resp, 0, sizeof resp);
 
 	uobj = idr_write_uobj(&ib_uverbs_qp_idr, cmd.qp_handle, file->ucontext);
@@ -2313,14 +2364,17 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (in_len < sizeof cmd + cmd.wqe_size * cmd.wr_count +
 	    cmd.sge_count * sizeof (struct ib_uverbs_sge))
 		return -EINVAL;
@@ -2562,14 +2616,17 @@ ssize_t ib_uverbs_post_recv(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	wr = ib_uverbs_unmarshall_recv(buf + sizeof cmd,
 				       in_len - sizeof cmd, cmd.wr_count,
 				       cmd.sge_count, cmd.wqe_size);
@@ -2619,14 +2676,17 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	wr = ib_uverbs_unmarshall_recv(buf + sizeof cmd,
 				       in_len - sizeof cmd, cmd.wr_count,
 				       cmd.sge_count, cmd.wqe_size);
@@ -2678,14 +2738,17 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	uobj = kmalloc(sizeof *uobj, GFP_KERNEL);
 	if (!uobj)
 		return -ENOMEM;
@@ -3282,14 +3345,17 @@ ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	xcmd.response	 = cmd.response;
 	xcmd.user_handle = cmd.user_handle;
 	xcmd.srq_type	 = IB_SRQT_BASIC;
@@ -3321,14 +3387,17 @@ ssize_t ib_uverbs_create_xsrq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	INIT_UDATA(&udata, buf + sizeof cmd,
 		   response + sizeof resp,
 		   in_len - sizeof cmd, out_len - sizeof resp);
@@ -3387,14 +3456,17 @@ ssize_t ib_uverbs_query_srq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	srq = idr_read_srq(cmd.srq_handle, file->ucontext);
 	if (!srq)
 		return -EINVAL;
@@ -3435,14 +3507,17 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
-	if (out_len < sizeof resp)
-		return -ENOSPC;
-
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
 	response = (void __user *)(unsigned long)cmd.response;
 
+	if (!access_ok(VERIFY_WRITE, response, out_len))
+		return -EFAULT;
+
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	uobj = idr_write_uobj(&ib_uverbs_srq_idr, cmd.srq_handle, file->ucontext);
 	if (!uobj)
 		return -EINVAL;
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv1 0/6] IB/uverbs: check request parameters

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 2718 bytes --]

On Mon, 2015-05-04 at 15:00 +0200, Yann Droneaud wrote:
> Hi,
> 
> Please find a patchset against uverbs to improve the checks done
> on uverbs request parameters. This patchset in an extract of a
> previous patchset sent some times ago[1].

I reviewed those patches last time they came around.  In general, I'm on
board with the idea of cleaning up option checking, but in your original
patch submission you point out that the patches have a non-0 chance of
breaking user applications that pass in parameters that wouldn't pass
these checks but work anyway.  Have you done any looking into that
possibility?

> I've provided some explanation of the issues partialy addressed
> by this patchset in a previous message[2].
> 
> As we're now addressing overflows, I think it's time to apply
> this patchset.
> 
> Changes since v0 [3]
> - updated against v4.1-rc2
> - incorporated patches to add check on response buffer using
>   access_ok()
> 
> [1] "[PATCH 00/22] infiniband: improve userspace input check"
> 
> http://marc.info/?i=cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> 
> [2] "Re: [PATCHv4 for-3.13 00/10] create_flow/destroy_flow fixes for v3.13"
> 
> http://marc.info/?i=1387493822.11925.217.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org
> http://mid.gmane.org/1387493822.11925.217.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org
> 
> [3] "[PATCH 0/4] IB/uverbs: check request parameters"
> 
> http://marc.info/?i=cover.1405884453.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> http://mid.gmane.org/cover.1405884453.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> 
> Yann Droneaud (6):
>   IB/uverbs: check userspace input buffer size
>   IB/uverbs: check userspace output buffer size
>   IB/uverbs: check userspace output buffer size in ib_uverbs_poll_cq()
>   IB/uverbs: subtract command header from input size
>   IB/uverbs: move cast from u64 to void __user pointer to its own
>     variable
>   IB/uverbs: check access to userspace response buffer
> 
>  drivers/infiniband/core/uverbs_cmd.c         | 449 +++++++++++++++++++++------
>  drivers/infiniband/core/uverbs_main.c        |  29 +-
>  drivers/infiniband/hw/mlx5/cq.c              |   6 +-
>  drivers/infiniband/hw/mlx5/main.c            |   2 +-
>  drivers/infiniband/hw/mlx5/srq.c             |   6 +-
>  drivers/infiniband/hw/mthca/mthca_provider.c |   2 +-
>  6 files changed, 382 insertions(+), 112 deletions(-)
> 


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCHv1 0/6] IB/uverbs: check request parameters

[Yann Droneaud]

Hi,

Le lundi 04 mai 2015 à 13:45 -0400, Doug Ledford a écrit :
> On Mon, 2015-05-04 at 15:00 +0200, Yann Droneaud wrote:
> > 
> > Please find a patchset against uverbs to improve the checks done
> > on uverbs request parameters. This patchset in an extract of a
> > previous patchset sent some times ago[1].
> 
> I reviewed those patches last time they came around.  In general, I'm on
> board with the idea of cleaning up option checking, but in your original
> patch submission you point out that the patches have a non-0 chance of
> breaking user applications that pass in parameters that wouldn't pass
> these checks but work anyway.  Have you done any looking into that
> possibility?
> 

I believe applications using libibverbs are immune of any kind of
regression.

But I cannot tell for sure for applications that access directly uverbs:
those could be doing broken things.

Anyway, we should prevent such applications to broke into the kernel:
integer over/under flow or buffer overflow are not acceptable.

> > I've provided some explanation of the issues partialy addressed
> > by this patchset in a previous message[2].
> > 
> > As we're now addressing overflows, I think it's time to apply
> > this patchset.
> > 
> > Changes since v0 [3]
> > - updated against v4.1-rc2
> > - incorporated patches to add check on response buffer using
> >   access_ok()
> > 
> > [1] "[PATCH 00/22] infiniband: improve userspace input check"
> > 
> > http://marc.info/?i=cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> > http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> > 
> > [2] "Re: [PATCHv4 for-3.13 00/10] create_flow/destroy_flow fixes for v3.13"
> > 
> > http://marc.info/?i=1387493822.11925.217.camel-bi+AKbBUZKY6gyzm1THtWdBPR1lH4CV8@public.gmane.orgain
> > http://mid.gmane.org/1387493822.11925.217.camel-bi+AKbBUZKY6gyzm1THtWbh0DlRQRlKF@public.gmane.orgin
> > 
> > [3] "[PATCH 0/4] IB/uverbs: check request parameters"
> > 
> > http://marc.info/?i=cover.1405884453.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> > http://mid.gmane.org/cover.1405884453.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> > 
> > Yann Droneaud (6):
> >   IB/uverbs: check userspace input buffer size
> >   IB/uverbs: check userspace output buffer size
> >   IB/uverbs: check userspace output buffer size in ib_uverbs_poll_cq()
> >   IB/uverbs: subtract command header from input size
> >   IB/uverbs: move cast from u64 to void __user pointer to its own
> >     variable
> >   IB/uverbs: check access to userspace response buffer
> > 
> >  drivers/infiniband/core/uverbs_cmd.c         | 449 +++++++++++++++++++++------
> >  drivers/infiniband/core/uverbs_main.c        |  29 +-
> >  drivers/infiniband/hw/mlx5/cq.c              |   6 +-
> >  drivers/infiniband/hw/mlx5/main.c            |   2 +-
> >  drivers/infiniband/hw/mlx5/srq.c             |   6 +-
> >  drivers/infiniband/hw/mthca/mthca_provider.c |   2 +-
> >  6 files changed, 382 insertions(+), 112 deletions(-)
> > 
> 
> 

Regards.

-- 
Yann Droneaud
OPTEYA


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv1 0/6] IB/uverbs: check request parameters

[Yann Droneaud]

Hi,

Le lundi 04 mai 2015 à 21:56 +0200, Yann Droneaud a écrit :
> Le lundi 04 mai 2015 à 13:45 -0400, Doug Ledford a écrit :
> > On Mon, 2015-05-04 at 15:00 +0200, Yann Droneaud wrote:
> > > 
> > > Please find a patchset against uverbs to improve the checks done
> > > on uverbs request parameters. This patchset in an extract of a
> > > previous patchset sent some times ago[1].
> > 
> > I reviewed those patches last time they came around.  In general, I'm on
> > board with the idea of cleaning up option checking, but in your original
> > patch submission you point out that the patches have a non-0 chance of
> > breaking user applications that pass in parameters that wouldn't pass
> > these checks but work anyway.  Have you done any looking into that
> > possibility?
> > 
> 
> I believe applications using libibverbs are immune of any kind of
> regression.
> 
> But I cannot tell for sure for applications that access directly uverbs:
> those could be doing broken things.
> 
> Anyway, we should prevent such applications to broke into the kernel:
> integer over/under flow or buffer overflow are not acceptable.
> 

I would also add, that, being stricter doesn't introduce a new, 
diverging behavior that newer applications could rely on, so if one 
existing application is found to be broken by the updated checks, those 
patchs could simply be reverted without falling into a compatibility 
nightmare where we would have to support two different behaviors.

> > > I've provided some explanation of the issues partialy addressed
> > > by this patchset in a previous message[2].
> > > 
> > > As we're now addressing overflows, I think it's time to apply
> > > this patchset.
> > > 
> > > Changes since v0 [3]
> > > - updated against v4.1-rc2
> > > - incorporated patches to add check on response buffer using
> > >   access_ok()
> > > 
> > > [1] "[PATCH 00/22] infiniband: improve userspace input check"
> > > 
> > > http://marc.info/?i=cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> > > http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> > > 
> > > [2] "Re: [PATCHv4 for-3.13 00/10] create_flow/destroy_flow fixes for v3.13"
> > > 
> > > http://marc.info/?i=1387493822.11925.217.camel-bi+AKbBUZKY6gyzm1THtWQ@public.gmane.orgomain
> > > http://mid.gmane.org/1387493822.11925.217.camel-bi+AKbBUZKY6gyzm1THtWWGXanvQGlWp@public.gmane.orgmain
> > > 
> > > [3] "[PATCH 0/4] IB/uverbs: check request parameters"
> > > 
> > > http://marc.info/?i=cover.1405884453.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> > > http://mid.gmane.org/cover.1405884453.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> > > 
> > > Yann Droneaud (6):
> > >   IB/uverbs: check userspace input buffer size
> > >   IB/uverbs: check userspace output buffer size
> > >   IB/uverbs: check userspace output buffer size in ib_uverbs_poll_cq()
> > >   IB/uverbs: subtract command header from input size
> > >   IB/uverbs: move cast from u64 to void __user pointer to its own
> > >     variable
> > >   IB/uverbs: check access to userspace response buffer
> > > 
> > >  drivers/infiniband/core/uverbs_cmd.c         | 449 +++++++++++++++++++++------
> > >  drivers/infiniband/core/uverbs_main.c        |  29 +-
> > >  drivers/infiniband/hw/mlx5/cq.c              |   6 +-
> > >  drivers/infiniband/hw/mlx5/main.c            |   2 +-
> > >  drivers/infiniband/hw/mlx5/srq.c             |   6 +-
> > >  drivers/infiniband/hw/mthca/mthca_provider.c |   2 +-
> > >  6 files changed, 382 insertions(+), 112 deletions(-)
> > > 
> > 
> > 
> 

Regards.

-- 
Yann Droneaud
OPTEYA



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html