From: Jason Gunthorpe <jgg@xxxxxxxxxxxx> After all the rework is done it is now possible to include single flags in the type macros. Any user of UVERBS_ATTR_STRUCT needs to zero check data past the end of the known struct to be correct, so make this mandatory, and get rid of MIN_SZ_OR_ZERO as a user flag. This changes UVERBS_ATTR_TYPE to refer to a struct of exact size with not possibility of extension, convert the few users of UVERBS_ATTR_TYPE and MIN_SZ_OR_ZERO to use UVERBS_ATTR_STRUCT. The one user of UVERBS_ATTR_STRUCT without MIN_SZ_OR_ZERO is just confused. There is some padding at the end of that struct, but userspace always provides it with the padding. The construction doesn't test if the padding is zero, so it is pointless. Just use UVERBS_ATTR_TYPE. Finally, rename min_sz_or_zero to zero_trailing to better reflect what it does and hopefully avoid such mis-uses in the future. Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx> --- drivers/infiniband/core/uverbs_ioctl.c | 4 +-- drivers/infiniband/core/uverbs_ioctl_merge.c | 2 +- .../infiniband/core/uverbs_std_types_flow_action.c | 21 +++++++-------- include/rdma/uverbs_ioctl.h | 31 +++++++++++++++------- 4 files changed, 34 insertions(+), 24 deletions(-) diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c index cb6109036129..5b59c6f0feed 100644 --- a/drivers/infiniband/core/uverbs_ioctl.c +++ b/drivers/infiniband/core/uverbs_ioctl.c @@ -98,14 +98,14 @@ static int uverbs_process_attr(struct ib_uverbs_file *ufile, * non-zero content, making ABI compat/discovery simpler. */ if (uattr->len > val_spec->u.ptr.len && - val_spec->min_sz_or_zero && + val_spec->zero_trailing && !uverbs_is_attr_cleared(uattr, val_spec->u.ptr.len)) return -EOPNOTSUPP; /* fall through */ case UVERBS_ATTR_TYPE_PTR_OUT: if (uattr->len < val_spec->u.ptr.min_len || - (!val_spec->min_sz_or_zero && + (!val_spec->zero_trailing && uattr->len > val_spec->u.ptr.len)) return -EINVAL; diff --git a/drivers/infiniband/core/uverbs_ioctl_merge.c b/drivers/infiniband/core/uverbs_ioctl_merge.c index ece5c9463dbe..f81aa888ce5c 100644 --- a/drivers/infiniband/core/uverbs_ioctl_merge.c +++ b/drivers/infiniband/core/uverbs_ioctl_merge.c @@ -378,7 +378,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->min_sz_or_zero, + attr->zero_trailing, "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_flow_action.c b/drivers/infiniband/core/uverbs_std_types_flow_action.c index 143dbfdfda6f..c753a34cd984 100644 --- a/drivers/infiniband/core/uverbs_std_types_flow_action.c +++ b/drivers/infiniband/core/uverbs_std_types_flow_action.c @@ -367,8 +367,9 @@ static int UVERBS_HANDLER(UVERBS_METHOD_FLOW_ACTION_ESP_MODIFY)(struct ib_device static const struct uverbs_attr_spec uverbs_flow_action_esp_keymat[] = { [IB_UVERBS_FLOW_ACTION_ESP_KEYMAT_AES_GCM] = { .type = UVERBS_ATTR_TYPE_PTR_IN, - UVERBS_ATTR_TYPE(struct ib_uverbs_flow_action_esp_keymat_aes_gcm), - UA_MIN_SZ_OR_ZERO + UVERBS_ATTR_STRUCT( + struct ib_uverbs_flow_action_esp_keymat_aes_gcm, + aes_key), }, }; @@ -380,8 +381,8 @@ static const struct uverbs_attr_spec uverbs_flow_action_esp_replay[] = { }, [IB_UVERBS_FLOW_ACTION_ESP_REPLAY_BMP] = { .type = UVERBS_ATTR_TYPE_PTR_IN, - UVERBS_ATTR_STRUCT(struct ib_uverbs_flow_action_esp_replay_bmp, size), - UA_MIN_SZ_OR_ZERO + UVERBS_ATTR_STRUCT(struct ib_uverbs_flow_action_esp_replay_bmp, + size), }, }; @@ -394,8 +395,7 @@ DECLARE_UVERBS_NAMED_METHOD( UVERBS_ATTR_PTR_IN(UVERBS_ATTR_FLOW_ACTION_ESP_ATTRS, UVERBS_ATTR_STRUCT(struct ib_uverbs_flow_action_esp, hard_limit_pkts), - UA_MANDATORY, - UA_MIN_SZ_OR_ZERO), + UA_MANDATORY), UVERBS_ATTR_PTR_IN(UVERBS_ATTR_FLOW_ACTION_ESP_ESN, UVERBS_ATTR_TYPE(__u32), UA_OPTIONAL), @@ -407,8 +407,7 @@ DECLARE_UVERBS_NAMED_METHOD( UA_OPTIONAL), UVERBS_ATTR_PTR_IN( UVERBS_ATTR_FLOW_ACTION_ESP_ENCAP, - UVERBS_ATTR_STRUCT(struct ib_uverbs_flow_action_esp_encap, - type), + UVERBS_ATTR_TYPE(struct ib_uverbs_flow_action_esp_encap), UA_OPTIONAL)); DECLARE_UVERBS_NAMED_METHOD( @@ -420,8 +419,7 @@ DECLARE_UVERBS_NAMED_METHOD( UVERBS_ATTR_PTR_IN(UVERBS_ATTR_FLOW_ACTION_ESP_ATTRS, UVERBS_ATTR_STRUCT(struct ib_uverbs_flow_action_esp, hard_limit_pkts), - UA_OPTIONAL, - UA_MIN_SZ_OR_ZERO), + UA_OPTIONAL), UVERBS_ATTR_PTR_IN(UVERBS_ATTR_FLOW_ACTION_ESP_ESN, UVERBS_ATTR_TYPE(__u32), UA_OPTIONAL), @@ -433,8 +431,7 @@ DECLARE_UVERBS_NAMED_METHOD( UA_OPTIONAL), UVERBS_ATTR_PTR_IN( UVERBS_ATTR_FLOW_ACTION_ESP_ENCAP, - UVERBS_ATTR_STRUCT(struct ib_uverbs_flow_action_esp_encap, - type), + UVERBS_ATTR_TYPE(struct ib_uverbs_flow_action_esp_encap), UA_OPTIONAL)); DECLARE_UVERBS_NAMED_METHOD_DESTROY( diff --git a/include/rdma/uverbs_ioctl.h b/include/rdma/uverbs_ioctl.h index 0b46ef8f0b4c..017ccf75890c 100644 --- a/include/rdma/uverbs_ioctl.h +++ b/include/rdma/uverbs_ioctl.h @@ -67,10 +67,11 @@ struct uverbs_attr_spec { u8 type; /* - * Support extending attributes by length, validate all - * unknown size == zero + * Support extending attributes by length. Allow the user to provide + * more bytes than ptr.len, but check that everything after is zero'd + * by the user. */ - u8 min_sz_or_zero:1; + u8 zero_trailing:1; /* * Valid only for PTR_IN. Allocate and copy the data inside * the parser @@ -200,13 +201,26 @@ struct uverbs_object_tree_def { * ======================================= */ -/* Use in the _type parameter for attribute specifications */ -#define UVERBS_ATTR_TYPE(_type) \ - .u.ptr.min_len = sizeof(_type), .u.ptr.len = sizeof(_type) -#define UVERBS_ATTR_STRUCT(_type, _last) \ - .u.ptr.min_len = ((uintptr_t)(&((_type *)0)->_last + 1)), .u.ptr.len = sizeof(_type) #define UVERBS_ATTR_SIZE(_min_len, _len) \ .u.ptr.min_len = _min_len, .u.ptr.len = _len + +/* + * Specifies a uapi structure that cannot be extended. The user must always + * supply the whole structure and nothing more. The structure must be declared + * in a header under include/uapi/rdma. + */ +#define UVERBS_ATTR_TYPE(_type) \ + .u.ptr.min_len = sizeof(_type), .u.ptr.len = sizeof(_type) +/* + * Specifies a uapi structure where the user must provide at least up to + * member 'last'. Anything after last and up until the end of the structure + * can be non-zero, anything longer than the end of the structure must be + * zero. The structure must be declared in a header under include/uapi/rdma. + */ +#define UVERBS_ATTR_STRUCT(_type, _last) \ + .zero_trailing = 1, \ + UVERBS_ATTR_SIZE(((uintptr_t)(&((_type *)0)->_last + 1)), \ + sizeof(_type)) /* * Specifies at least min_len bytes must be passed in, but the amount can be * larger, up to the protocol maximum size. No check for zeroing is done. @@ -216,7 +230,6 @@ struct uverbs_object_tree_def { /* Must be used in the '...' of any UVERBS_ATTR */ #define UA_ALLOC_AND_COPY .alloc_and_copy = 1 #define UA_MANDATORY .mandatory = 1 -#define UA_MIN_SZ_OR_ZERO .min_sz_or_zero = 1 #define UA_OPTIONAL .mandatory = 0 #define UVERBS_ATTR_IDR(_attr_id, _idr_type, _access, ...) \ -- 2.14.4 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html