From: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
There are several cases where input to fill_attr comes from the untrusted
libibverbs user, and may not be marshalable to the kernel. We already have
a case like this when passing the flow counters array, but there are a few
others too.
Provide a general way for the fill_attr functions to signal un-marshaled
data and cause execute to automatically fail with EINVAL.
Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
Signed-off-by: Yishai Hadas <yishaih@xxxxxxxxxxxx>
---
libibverbs/cmd_counters.c | 7 ++-----
libibverbs/cmd_ioctl.c | 11 ++++++++++-
libibverbs/cmd_ioctl.h | 28 ++++++++++++++++++++--------
3 files changed, 32 insertions(+), 14 deletions(-)
diff --git a/libibverbs/cmd_counters.c b/libibverbs/cmd_counters.c
index ed57f25..8964fed 100644
--- a/libibverbs/cmd_counters.c
+++ b/libibverbs/cmd_counters.c
@@ -87,12 +87,9 @@ int ibv_cmd_read_counters(struct verbs_counters *vcounters,
3,
link);
- if (!is_attr_size_valid(ncounters, sizeof(uint64_t)))
- return EINVAL;
-
fill_attr_in_obj(cmd, UVERBS_ATTR_READ_COUNTERS_HANDLE, vcounters->handle);
- fill_attr_out(cmd, UVERBS_ATTR_READ_COUNTERS_BUFF, counters_value,
- ncounters * sizeof(uint64_t));
+ fill_attr_out_ptr_array(cmd, UVERBS_ATTR_READ_COUNTERS_BUFF, counters_value,
+ ncounters);
fill_attr_in_uint32(cmd, UVERBS_ATTR_READ_COUNTERS_FLAGS, flags);
return execute_ioctl(vcounters->counters.context, cmd);
diff --git a/libibverbs/cmd_ioctl.c b/libibverbs/cmd_ioctl.c
index f53f83a..c12442f 100644
--- a/libibverbs/cmd_ioctl.c
+++ b/libibverbs/cmd_ioctl.c
@@ -128,6 +128,14 @@ int execute_ioctl(struct ibv_context *context, struct ibv_command_buffer *cmd)
{
struct verbs_context *vctx = verbs_get_ctx(context);
+ /*
+ * One of the fill functions was given input that cannot be marshaled
+ */
+ if (unlikely(cmd->buffer_error)) {
+ errno = EINVAL;
+ return errno;
+ }
+
prepare_attrs(cmd);
cmd->hdr.length = sizeof(cmd->hdr) +
sizeof(cmd->hdr.attrs[0]) * cmd->hdr.num_attrs;
@@ -154,7 +162,8 @@ _fill_attr_in_uhw(struct ibv_command_buffer *cmd, uint16_t attr_id,
{
struct ib_uverbs_attr *attr = _ioctl_next_attr(cmd, attr_id);
- assert(len <= UINT16_MAX);
+ if (unlikely(len > UINT16_MAX))
+ cmd->buffer_error = 1;
attr->len = len;
attr->data = ioctl_ptr_to_u64(data);
diff --git a/libibverbs/cmd_ioctl.h b/libibverbs/cmd_ioctl.h
index 488c9e8..d58d890 100644
--- a/libibverbs/cmd_ioctl.h
+++ b/libibverbs/cmd_ioctl.h
@@ -40,6 +40,7 @@
#include <rdma/rdma_user_ioctl_cmds.h>
#include <infiniband/verbs.h>
#include <ccan/container_of.h>
+#include <util/compiler.h>
static inline uint64_t ioctl_ptr_to_u64(const void *ptr)
{
@@ -95,6 +96,8 @@ struct ibv_command_buffer {
uint8_t uhw_out_idx;
uint8_t uhw_in_headroom_dwords;
uint8_t uhw_out_headroom_dwords;
+
+ uint8_t buffer_error:1;
/*
* These flags control what execute_ioctl_fallback does if the kernel
* does not support ioctl
@@ -268,7 +271,8 @@ fill_attr_in(struct ibv_command_buffer *cmd, uint16_t attr_id, const void *data,
{
struct ib_uverbs_attr *attr = _ioctl_next_attr(cmd, attr_id);
- assert(len <= UINT16_MAX);
+ if (unlikely(len > UINT16_MAX))
+ cmd->buffer_error = 1;
attr->len = len;
if (len <= sizeof(uint64_t))
@@ -348,7 +352,9 @@ fill_attr_out(struct ibv_command_buffer *cmd, uint16_t attr_id, void *data,
{
struct ib_uverbs_attr *attr = _ioctl_next_attr(cmd, attr_id);
- assert(len <= UINT16_MAX);
+ if (unlikely(len > UINT16_MAX))
+ cmd->buffer_error = 1;
+
attr->len = len;
attr->data = ioctl_ptr_to_u64(data);
@@ -358,6 +364,18 @@ fill_attr_out(struct ibv_command_buffer *cmd, uint16_t attr_id, void *data,
#define fill_attr_out_ptr(cmd, attr_id, ptr) \
fill_attr_out(cmd, attr_id, ptr, sizeof(*(ptr)))
+/* If size*nelems overflows size_t this returns SIZE_MAX */
+static inline size_t _array_len(size_t size, size_t nelems)
+{
+ if (size != 0 &&
+ SIZE_MAX / size <= nelems)
+ return SIZE_MAX;
+ return size * nelems;
+}
+
+#define fill_attr_out_ptr_array(cmd, attr_id, ptr, nelems) \
+ fill_attr_out(cmd, attr_id, ptr, _array_len(sizeof(*ptr), nelems))
+
static inline size_t __check_divide(size_t val, unsigned int div)
{
assert(val % div == 0);
@@ -376,10 +394,4 @@ fill_attr_in_enum(struct ibv_command_buffer *cmd, uint16_t attr_id,
return attr;
}
-static inline int is_attr_size_valid(size_t num, size_t ent_size)
-{
- /* check multiplication overflow */
- return (!ent_size || UINT16_MAX / ent_size >= num);
-}
-
#endif
--
1.8.3.1
