Blob Blame History Raw
From: Jason Gunthorpe <jgg@mellanox.com>
Date: Wed, 4 Jul 2018 08:50:31 +0300
Subject: RDMA/uverbs: Combine MIN_SZ_OR_ZERO with UVERBS_ATTR_STRUCT
Patch-mainline: v4.19-rc1
Git-commit: 422e3d37ed7ea8b421208a44913c420055334976
References: bsc#1103992 FATE#326009

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@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Acked-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
 drivers/infiniband/core/uverbs_ioctl.c                 |    4 +-
 drivers/infiniband/core/uverbs_ioctl_merge.c           |    2 -
 drivers/infiniband/core/uverbs_std_types_flow_action.c |   21 ++++-------
 include/rdma/uverbs_ioctl.h                            |   31 ++++++++++++-----
 4 files changed, 34 insertions(+), 24 deletions(-)

--- 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
 		 * 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;
 
--- 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_
 				 "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;
--- 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_
 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 uve
 	},
 	[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(
--- 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, ...)                     \