diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c index 82a1775ba657..1e6bf2488584 100644 --- a/drivers/infiniband/core/uverbs_ioctl.c +++ b/drivers/infiniband/core/uverbs_ioctl.c @@ -35,6 +35,17 @@ #include "rdma_core.h" #include "uverbs.h" +static bool uverbs_is_attr_cleared(const struct ib_uverbs_attr *uattr, + u16 len) +{ + if (uattr->len > sizeof(((struct ib_uverbs_attr *)0)->data)) + return ib_is_buffer_cleared(u64_to_user_ptr(uattr->data) + len, + uattr->len - len); + + return !memchr_inv((const void *)&uattr->data + len, + 0, uattr->len - len); +} + static int uverbs_process_attr(struct ib_device *ibdev, struct ib_ucontext *ucontext, const struct ib_uverbs_attr *uattr, @@ -68,9 +79,20 @@ static int uverbs_process_attr(struct ib_device *ibdev, switch (spec->type) { case UVERBS_ATTR_TYPE_PTR_IN: + /* Ensure that any data provided by userspace beyond the known + * struct is zero. Userspace that knows how to use some future + * longer struct will fail here if used with an old kernel and + * non-zero content, making ABI compat/discovery simpler. + */ + if (uattr->len > spec->ptr.len && + spec->flags & UVERBS_ATTR_SPEC_F_MIN_SZ_OR_ZERO && + !uverbs_is_attr_cleared(uattr, spec->ptr.len)) + return -EOPNOTSUPP; + + /* fall through */ case UVERBS_ATTR_TYPE_PTR_OUT: - if (uattr->len < spec->ptr.len || - (!(spec->flags & UVERBS_ATTR_SPEC_F_MIN_SZ) && + if (uattr->len < spec->ptr.min_len || + (!(spec->flags & UVERBS_ATTR_SPEC_F_MIN_SZ_OR_ZERO) && uattr->len > spec->ptr.len)) return -EINVAL; diff --git a/drivers/infiniband/core/uverbs_ioctl_merge.c b/drivers/infiniband/core/uverbs_ioctl_merge.c index 62e1eb1d2a28..0f88a1919d51 100644 --- a/drivers/infiniband/core/uverbs_ioctl_merge.c +++ b/drivers/infiniband/core/uverbs_ioctl_merge.c @@ -379,7 +379,7 @@ static struct uverbs_method_spec *build_method_with_attrs(const struct uverbs_me "ib_uverbs: Tried to merge attr (%d) but it's an object with new/destroy access but isn't mandatory\n", min_id) || WARN(IS_ATTR_OBJECT(attr) && - attr->flags & UVERBS_ATTR_SPEC_F_MIN_SZ, + attr->flags & UVERBS_ATTR_SPEC_F_MIN_SZ_OR_ZERO, "ib_uverbs: Tried to merge attr (%d) but it's an object with min_sz flag\n", min_id)) { res = -EINVAL; diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c index e4a4b184a6bc..0a2d8532de21 100644 --- a/drivers/infiniband/core/uverbs_std_types.c +++ b/drivers/infiniband/core/uverbs_std_types.c @@ -216,9 +216,11 @@ static int uverbs_hot_unplug_completion_event_file(struct ib_uobject_file *uobj_ * spec. */ static const struct uverbs_attr_def uverbs_uhw_compat_in = - UVERBS_ATTR_PTR_IN_SZ(UVERBS_ATTR_UHW_IN, 0, UA_FLAGS(UVERBS_ATTR_SPEC_F_MIN_SZ)); + UVERBS_ATTR_PTR_IN_SZ(UVERBS_ATTR_UHW_IN, UVERBS_ATTR_SIZE(0, USHRT_MAX), + UA_FLAGS(UVERBS_ATTR_SPEC_F_MIN_SZ_OR_ZERO)); static const struct uverbs_attr_def uverbs_uhw_compat_out = - UVERBS_ATTR_PTR_OUT_SZ(UVERBS_ATTR_UHW_OUT, 0, UA_FLAGS(UVERBS_ATTR_SPEC_F_MIN_SZ)); + UVERBS_ATTR_PTR_OUT_SZ(UVERBS_ATTR_UHW_OUT, UVERBS_ATTR_SIZE(0, USHRT_MAX), + UA_FLAGS(UVERBS_ATTR_SPEC_F_MIN_SZ_OR_ZERO)); static void create_udata(struct uverbs_attr_bundle *ctx, struct ib_udata *udata) @@ -350,17 +352,19 @@ static DECLARE_UVERBS_NAMED_METHOD(UVERBS_METHOD_CQ_CREATE, &UVERBS_ATTR_IDR(UVERBS_ATTR_CREATE_CQ_HANDLE, UVERBS_OBJECT_CQ, UVERBS_ACCESS_NEW, UA_FLAGS(UVERBS_ATTR_SPEC_F_MANDATORY)), - &UVERBS_ATTR_PTR_IN(UVERBS_ATTR_CREATE_CQ_CQE, u32, + &UVERBS_ATTR_PTR_IN(UVERBS_ATTR_CREATE_CQ_CQE, + UVERBS_ATTR_TYPE(u32), UA_FLAGS(UVERBS_ATTR_SPEC_F_MANDATORY)), - &UVERBS_ATTR_PTR_IN(UVERBS_ATTR_CREATE_CQ_USER_HANDLE, u64, + &UVERBS_ATTR_PTR_IN(UVERBS_ATTR_CREATE_CQ_USER_HANDLE, + UVERBS_ATTR_TYPE(u64), UA_FLAGS(UVERBS_ATTR_SPEC_F_MANDATORY)), &UVERBS_ATTR_FD(UVERBS_ATTR_CREATE_CQ_COMP_CHANNEL, UVERBS_OBJECT_COMP_CHANNEL, UVERBS_ACCESS_READ), - &UVERBS_ATTR_PTR_IN(UVERBS_ATTR_CREATE_CQ_COMP_VECTOR, u32, + &UVERBS_ATTR_PTR_IN(UVERBS_ATTR_CREATE_CQ_COMP_VECTOR, UVERBS_ATTR_TYPE(u32), UA_FLAGS(UVERBS_ATTR_SPEC_F_MANDATORY)), - &UVERBS_ATTR_PTR_IN(UVERBS_ATTR_CREATE_CQ_FLAGS, u32), - &UVERBS_ATTR_PTR_OUT(UVERBS_ATTR_CREATE_CQ_RESP_CQE, u32, + &UVERBS_ATTR_PTR_IN(UVERBS_ATTR_CREATE_CQ_FLAGS, UVERBS_ATTR_TYPE(u32)), + &UVERBS_ATTR_PTR_OUT(UVERBS_ATTR_CREATE_CQ_RESP_CQE, UVERBS_ATTR_TYPE(u32), UA_FLAGS(UVERBS_ATTR_SPEC_F_MANDATORY)), &uverbs_uhw_compat_in, &uverbs_uhw_compat_out); @@ -394,7 +398,7 @@ static DECLARE_UVERBS_NAMED_METHOD(UVERBS_METHOD_CQ_DESTROY, UVERBS_ACCESS_DESTROY, UA_FLAGS(UVERBS_ATTR_SPEC_F_MANDATORY)), &UVERBS_ATTR_PTR_OUT(UVERBS_ATTR_DESTROY_CQ_RESP, - struct ib_uverbs_destroy_cq_resp, + UVERBS_ATTR_TYPE(struct ib_uverbs_destroy_cq_resp), UA_FLAGS(UVERBS_ATTR_SPEC_F_MANDATORY))); DECLARE_UVERBS_NAMED_OBJECT(UVERBS_OBJECT_COMP_CHANNEL, diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index 2357f2b29610..e9288d0f627e 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -2446,11 +2446,9 @@ static inline int ib_copy_to_udata(struct ib_udata *udata, void *src, size_t len return copy_to_user(udata->outbuf, src, len) ? -EFAULT : 0; } -static inline bool ib_is_udata_cleared(struct ib_udata *udata, - size_t offset, - size_t len) +static inline bool ib_is_buffer_cleared(const void __user *p, + size_t len) { - const void __user *p = udata->inbuf + offset; bool ret; u8 *buf; @@ -2466,6 +2464,13 @@ static inline bool ib_is_udata_cleared(struct ib_udata *udata, return ret; } +static inline bool ib_is_udata_cleared(struct ib_udata *udata, + size_t offset, + size_t len) +{ + return ib_is_buffer_cleared(udata->inbuf + offset, len); +} + /** * ib_modify_qp_is_ok - Check that the supplied attribute mask * contains all required attributes and no attributes not allowed for diff --git a/include/rdma/uverbs_ioctl.h b/include/rdma/uverbs_ioctl.h index cd7c3e40c6cc..c4ee65b20bb7 100644 --- a/include/rdma/uverbs_ioctl.h +++ b/include/rdma/uverbs_ioctl.h @@ -62,8 +62,8 @@ enum uverbs_obj_access { enum { UVERBS_ATTR_SPEC_F_MANDATORY = 1U << 0, - /* Support extending attributes by length */ - UVERBS_ATTR_SPEC_F_MIN_SZ = 1U << 1, + /* Support extending attributes by length, validate all unknown size == zero */ + UVERBS_ATTR_SPEC_F_MIN_SZ_OR_ZERO = 1U << 1, }; /* Specification of a single attribute inside the ioctl message */ @@ -79,7 +79,10 @@ struct uverbs_attr_spec { enum uverbs_attr_type type; /* Combination of bits from enum UVERBS_ATTR_SPEC_F_XXXX */ u8 flags; + /* Current known size to kernel */ u16 len; + /* User isn't allowed to provide something < min_len */ + u16 min_len; } ptr; struct { enum uverbs_attr_type type; @@ -177,30 +180,41 @@ struct uverbs_object_tree_def { }; #define UA_FLAGS(_flags) .flags = _flags -#define __UVERBS_ATTR0(_id, _len, _type, ...) \ +#define __UVERBS_ATTR0(_id, _type, _fld, _attr, ...) \ ((const struct uverbs_attr_def) \ - {.id = _id, .attr = {{.ptr = {.type = _type, .len = _len, .flags = 0, } }, } }) -#define __UVERBS_ATTR1(_id, _len, _type, _flags) \ + {.id = _id, .attr = {{._fld = {.type = _type, _attr, .flags = 0, } }, } }) +#define __UVERBS_ATTR1(_id, _type, _fld, _attr, _extra1, ...) \ ((const struct uverbs_attr_def) \ - {.id = _id, .attr = {{.ptr = {.type = _type, .len = _len, _flags } },} }) -#define __UVERBS_ATTR(_id, _len, _type, _flags, _n, ...) \ - __UVERBS_ATTR##_n(_id, _len, _type, _flags) + {.id = _id, .attr = {{._fld = {.type = _type, _attr, _extra1 } },} }) +#define __UVERBS_ATTR2(_id, _type, _fld, _attr, _extra1, _extra2) \ + ((const struct uverbs_attr_def) \ + {.id = _id, .attr = {{._fld = {.type = _type, _attr, _extra1, _extra2 } },} }) +#define __UVERBS_ATTR(_id, _type, _fld, _attr, _extra1, _extra2, _n, ...) \ + __UVERBS_ATTR##_n(_id, _type, _fld, _attr, _extra1, _extra2) + +#define UVERBS_ATTR_TYPE(_type) \ + .min_len = sizeof(_type), .len = sizeof(_type) +#define UVERBS_ATTR_STRUCT(_type, _last) \ + .min_len = ((uintptr_t)(&((_type *)0)->_last + 1)), .len = sizeof(_type) +#define UVERBS_ATTR_SIZE(_min_len, _len) \ + .min_len = _min_len, .len = _len + /* * In new compiler, UVERBS_ATTR could be simplified by declaring it as * [_id] = {.type = _type, .len = _len, ##__VA_ARGS__} * But since we support older compilers too, we need the more complex code. */ -#define UVERBS_ATTR(_id, _len, _type, ...) \ - __UVERBS_ATTR(_id, _len, _type, ##__VA_ARGS__, 1, 0) +#define UVERBS_ATTR(_id, _type, _fld, _attr, ...) \ + __UVERBS_ATTR(_id, _type, _fld, _attr, ##__VA_ARGS__, 2, 1, 0) #define UVERBS_ATTR_PTR_IN_SZ(_id, _len, ...) \ - UVERBS_ATTR(_id, _len, UVERBS_ATTR_TYPE_PTR_IN, ##__VA_ARGS__) + UVERBS_ATTR(_id, UVERBS_ATTR_TYPE_PTR_IN, ptr, _len, ##__VA_ARGS__) /* If sizeof(_type) <= sizeof(u64), this will be inlined rather than a pointer */ #define UVERBS_ATTR_PTR_IN(_id, _type, ...) \ - UVERBS_ATTR_PTR_IN_SZ(_id, sizeof(_type), ##__VA_ARGS__) + UVERBS_ATTR_PTR_IN_SZ(_id, _type, ##__VA_ARGS__) #define UVERBS_ATTR_PTR_OUT_SZ(_id, _len, ...) \ - UVERBS_ATTR(_id, _len, UVERBS_ATTR_TYPE_PTR_OUT, ##__VA_ARGS__) + UVERBS_ATTR(_id, UVERBS_ATTR_TYPE_PTR_OUT, ptr, _len, ##__VA_ARGS__) #define UVERBS_ATTR_PTR_OUT(_id, _type, ...) \ - UVERBS_ATTR_PTR_OUT_SZ(_id, sizeof(_type), ##__VA_ARGS__) + UVERBS_ATTR_PTR_OUT_SZ(_id, _type, ##__VA_ARGS__) /* * In new compiler, UVERBS_ATTR_IDR (and FD) could be simplified by declaring @@ -396,8 +410,8 @@ static inline int _uverbs_copy_from(void *to, /* * 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. + * using UVERBS_ATTR_SPEC_F_MIN_SZ_OR_ZERO then it must call + * uverbs_copy_from_or_zero. */ if (unlikely(size < attr->ptr_attr.len)) return -EINVAL; @@ -411,9 +425,37 @@ static inline int _uverbs_copy_from(void *to, return 0; } +static inline int _uverbs_copy_from_or_zero(void *to, + const struct uverbs_attr_bundle *attrs_bundle, + size_t idx, + size_t size) +{ + const struct uverbs_attr *attr = uverbs_attr_get(attrs_bundle, idx); + size_t min_size; + + if (IS_ERR(attr)) + return PTR_ERR(attr); + + min_size = min_t(size_t, size, attr->ptr_attr.len); + + if (uverbs_attr_ptr_is_inline(attr)) + memcpy(to, &attr->ptr_attr.data, min_size); + else if (copy_from_user(to, u64_to_user_ptr(attr->ptr_attr.data), + min_size)) + return -EFAULT; + + if (size > min_size) + memset(to + min_size, 0, size - min_size); + + return 0; +} + #define uverbs_copy_from(to, attrs_bundle, idx) \ _uverbs_copy_from(to, attrs_bundle, idx, sizeof(*to)) +#define uverbs_copy_from_or_zero(to, attrs_bundle, idx) \ + _uverbs_copy_from_or_zero(to, attrs_bundle, idx, sizeof(*to)) + /* ================================================= * Definitions -> Specs infrastructure * =================================================