IB/uverbs: Always use the attribute size provided by the user
This fixes several bugs around the copy_to/from user path: - copy_to used the user provided size of the attribute and could copy data beyond the end of the kernel buffer into userspace. - copy_from didn't know the size of the kernel buffer and could have left kernel memory unexpectedly un-initialized. - copy_from did not use the user length to determine if the attribute data is inlined or not. Signed-off-by: Matan Barak <matanb@mellanox.com> Signed-off-by: Leon Romanovsky <leon@kernel.org> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
This commit is contained in:
parent
415bb699d7
commit
89d9e8d3f1
|
@ -323,7 +323,8 @@ static int uverbs_create_cq_handler(struct ib_device *ib_dev,
|
|||
cq->res.type = RDMA_RESTRACK_CQ;
|
||||
rdma_restrack_add(&cq->res);
|
||||
|
||||
ret = uverbs_copy_to(attrs, CREATE_CQ_RESP_CQE, &cq->cqe);
|
||||
ret = uverbs_copy_to(attrs, CREATE_CQ_RESP_CQE, &cq->cqe,
|
||||
sizeof(cq->cqe));
|
||||
if (ret)
|
||||
goto err_cq;
|
||||
|
||||
|
@ -375,7 +376,7 @@ static int uverbs_destroy_cq_handler(struct ib_device *ib_dev,
|
|||
resp.comp_events_reported = obj->comp_events_reported;
|
||||
resp.async_events_reported = obj->async_events_reported;
|
||||
|
||||
return uverbs_copy_to(attrs, DESTROY_CQ_RESP, &resp);
|
||||
return uverbs_copy_to(attrs, DESTROY_CQ_RESP, &resp, sizeof(resp));
|
||||
}
|
||||
|
||||
static DECLARE_UVERBS_METHOD(
|
||||
|
|
|
@ -351,29 +351,50 @@ static inline const struct uverbs_attr *uverbs_attr_get(const struct uverbs_attr
|
|||
}
|
||||
|
||||
static inline int uverbs_copy_to(const struct uverbs_attr_bundle *attrs_bundle,
|
||||
size_t idx, const void *from)
|
||||
size_t idx, const void *from, size_t size)
|
||||
{
|
||||
const struct uverbs_attr *attr = uverbs_attr_get(attrs_bundle, idx);
|
||||
u16 flags;
|
||||
size_t min_size;
|
||||
|
||||
if (IS_ERR(attr))
|
||||
return PTR_ERR(attr);
|
||||
|
||||
min_size = min_t(size_t, attr->ptr_attr.len, size);
|
||||
if (copy_to_user(attr->ptr_attr.ptr, from, min_size))
|
||||
return -EFAULT;
|
||||
|
||||
flags = attr->ptr_attr.flags | UVERBS_ATTR_F_VALID_OUTPUT;
|
||||
return (!copy_to_user(attr->ptr_attr.ptr, from, attr->ptr_attr.len) &&
|
||||
!put_user(flags, &attr->uattr->flags)) ? 0 : -EFAULT;
|
||||
if (put_user(flags, &attr->uattr->flags))
|
||||
return -EFAULT;
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
static inline int _uverbs_copy_from(void *to, size_t to_size,
|
||||
static inline bool uverbs_attr_ptr_is_inline(const struct uverbs_attr *attr)
|
||||
{
|
||||
return attr->ptr_attr.len <= sizeof(attr->ptr_attr.data);
|
||||
}
|
||||
|
||||
static inline int _uverbs_copy_from(void *to,
|
||||
const struct uverbs_attr_bundle *attrs_bundle,
|
||||
size_t idx)
|
||||
size_t idx,
|
||||
size_t size)
|
||||
{
|
||||
const struct uverbs_attr *attr = uverbs_attr_get(attrs_bundle, idx);
|
||||
|
||||
if (IS_ERR(attr))
|
||||
return PTR_ERR(attr);
|
||||
|
||||
if (to_size <= sizeof(((struct ib_uverbs_attr *)0)->data))
|
||||
/*
|
||||
* Validation ensures attr->ptr_attr.len >= size. If the caller is
|
||||
* using UVERBS_ATTR_SPEC_F_MIN_SZ then it must call copy_from with
|
||||
* the right size.
|
||||
*/
|
||||
if (unlikely(size < attr->ptr_attr.len))
|
||||
return -EINVAL;
|
||||
|
||||
if (uverbs_attr_ptr_is_inline(attr))
|
||||
memcpy(to, &attr->ptr_attr.data, attr->ptr_attr.len);
|
||||
else if (copy_from_user(to, attr->ptr_attr.ptr, attr->ptr_attr.len))
|
||||
return -EFAULT;
|
||||
|
@ -382,7 +403,7 @@ static inline int _uverbs_copy_from(void *to, size_t to_size,
|
|||
}
|
||||
|
||||
#define uverbs_copy_from(to, attrs_bundle, idx) \
|
||||
_uverbs_copy_from(to, sizeof(*(to)), attrs_bundle, idx)
|
||||
_uverbs_copy_from(to, attrs_bundle, idx, sizeof(*to))
|
||||
|
||||
/* =================================================
|
||||
* Definitions -> Specs infrastructure
|
||||
|
|
Loading…
Reference in New Issue