* [PATCHv1 0/6] IB/uverbs: check request parameters
@ 2015-05-04 13:00 Yann Droneaud
[not found] ` <cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Yann Droneaud @ 2015-05-04 13:00 UTC (permalink / raw)
To: Doug Ledford, Roland Dreier
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCHv1 1/6] IB/uverbs: check userspace input buffer size
[not found] ` <cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
@ 2015-05-04 13:00 ` Yann Droneaud
2015-05-04 13:00 ` [PATCHv1 2/6] IB/uverbs: check userspace output " Yann Droneaud
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Yann Droneaud @ 2015-05-04 13:00 UTC (permalink / raw)
To: Doug Ledford, Roland Dreier
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCHv1 2/6] IB/uverbs: check userspace output buffer size
[not found] ` <cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
2015-05-04 13:00 ` [PATCHv1 1/6] IB/uverbs: check userspace input buffer size Yann Droneaud
@ 2015-05-04 13:00 ` Yann Droneaud
2015-05-04 13:00 ` [PATCHv1 3/6] IB/uverbs: check userspace output buffer size in ib_uverbs_poll_cq() Yann Droneaud
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Yann Droneaud @ 2015-05-04 13:00 UTC (permalink / raw)
To: Doug Ledford, Roland Dreier
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCHv1 3/6] IB/uverbs: check userspace output buffer size in ib_uverbs_poll_cq()
[not found] ` <cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
2015-05-04 13:00 ` [PATCHv1 1/6] IB/uverbs: check userspace input buffer size Yann Droneaud
2015-05-04 13:00 ` [PATCHv1 2/6] IB/uverbs: check userspace output " Yann Droneaud
@ 2015-05-04 13:00 ` Yann Droneaud
2015-05-04 13:00 ` [PATCHv1 4/6] IB/uverbs: subtract command header from input size Yann Droneaud
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Yann Droneaud @ 2015-05-04 13:00 UTC (permalink / raw)
To: Doug Ledford, Roland Dreier
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCHv1 4/6] IB/uverbs: subtract command header from input size
[not found] ` <cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
` (2 preceding siblings ...)
2015-05-04 13:00 ` [PATCHv1 3/6] IB/uverbs: check userspace output buffer size in ib_uverbs_poll_cq() Yann Droneaud
@ 2015-05-04 13:00 ` Yann Droneaud
2015-05-04 13:00 ` [PATCHv1 5/6] IB/uverbs: move cast from u64 to void __user pointer to its own variable Yann Droneaud
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Yann Droneaud @ 2015-05-04 13:00 UTC (permalink / raw)
To: Doug Ledford, Roland Dreier
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCHv1 5/6] IB/uverbs: move cast from u64 to void __user pointer to its own variable
[not found] ` <cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
` (3 preceding siblings ...)
2015-05-04 13:00 ` [PATCHv1 4/6] IB/uverbs: subtract command header from input size Yann Droneaud
@ 2015-05-04 13:00 ` Yann Droneaud
2015-05-04 13:00 ` [PATCHv1 6/6] IB/uverbs: check access to userspace response buffer Yann Droneaud
2015-05-04 17:45 ` [PATCHv1 0/6] IB/uverbs: check request parameters Doug Ledford
6 siblings, 0 replies; 10+ messages in thread
From: Yann Droneaud @ 2015-05-04 13:00 UTC (permalink / raw)
To: Doug Ledford, Roland Dreier
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCHv1 6/6] IB/uverbs: check access to userspace response buffer
[not found] ` <cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
` (4 preceding siblings ...)
2015-05-04 13:00 ` [PATCHv1 5/6] IB/uverbs: move cast from u64 to void __user pointer to its own variable Yann Droneaud
@ 2015-05-04 13:00 ` Yann Droneaud
2015-05-04 17:45 ` [PATCHv1 0/6] IB/uverbs: check request parameters Doug Ledford
6 siblings, 0 replies; 10+ messages in thread
From: Yann Droneaud @ 2015-05-04 13:00 UTC (permalink / raw)
To: Doug Ledford, Roland Dreier
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCHv1 0/6] IB/uverbs: check request parameters
[not found] ` <cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
` (5 preceding siblings ...)
2015-05-04 13:00 ` [PATCHv1 6/6] IB/uverbs: check access to userspace response buffer Yann Droneaud
@ 2015-05-04 17:45 ` Doug Ledford
[not found] ` <1430761519.2407.87.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
6 siblings, 1 reply; 10+ messages in thread
From: Doug Ledford @ 2015-05-04 17:45 UTC (permalink / raw)
To: Yann Droneaud; +Cc: Roland Dreier, linux-rdma-u79uwXL29TY76Z2rM5mHXA
[-- 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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv1 0/6] IB/uverbs: check request parameters
[not found] ` <1430761519.2407.87.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2015-05-04 19:56 ` Yann Droneaud
[not found] ` <1430769382.19516.9.camel-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Yann Droneaud @ 2015-05-04 19:56 UTC (permalink / raw)
To: Doug Ledford; +Cc: Roland Dreier, linux-rdma-u79uwXL29TY76Z2rM5mHXA
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv1 0/6] IB/uverbs: check request parameters
[not found] ` <1430769382.19516.9.camel-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
@ 2015-05-05 19:55 ` Yann Droneaud
0 siblings, 0 replies; 10+ messages in thread
From: Yann Droneaud @ 2015-05-05 19:55 UTC (permalink / raw)
To: Doug Ledford; +Cc: Roland Dreier, linux-rdma-u79uwXL29TY76Z2rM5mHXA
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-05-05 19:55 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-04 13:00 [PATCHv1 0/6] IB/uverbs: check request parameters Yann Droneaud
[not found] ` <cover.1430743694.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
2015-05-04 13:00 ` [PATCHv1 1/6] IB/uverbs: check userspace input buffer size Yann Droneaud
2015-05-04 13:00 ` [PATCHv1 2/6] IB/uverbs: check userspace output " Yann Droneaud
2015-05-04 13:00 ` [PATCHv1 3/6] IB/uverbs: check userspace output buffer size in ib_uverbs_poll_cq() Yann Droneaud
2015-05-04 13:00 ` [PATCHv1 4/6] IB/uverbs: subtract command header from input size Yann Droneaud
2015-05-04 13:00 ` [PATCHv1 5/6] IB/uverbs: move cast from u64 to void __user pointer to its own variable Yann Droneaud
2015-05-04 13:00 ` [PATCHv1 6/6] IB/uverbs: check access to userspace response buffer Yann Droneaud
2015-05-04 17:45 ` [PATCHv1 0/6] IB/uverbs: check request parameters Doug Ledford
[not found] ` <1430761519.2407.87.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-05-04 19:56 ` Yann Droneaud
[not found] ` <1430769382.19516.9.camel-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
2015-05-05 19:55 ` Yann Droneaud
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.