[PATCH 9/9] RDMA/uverbs: Combine MIN_SZ_OR_ZERO with UVERBS_ATTR_STRUCT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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>
---
 drivers/infiniband/core/uverbs_ioctl.c        |  4 +--
 drivers/infiniband/core/uverbs_ioctl_merge.c  |  2 +-
 .../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 cb61090361294d..5b59c6f0feed78 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 ece5c9463dbe15..f81aa888ce5c4c 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 e2d5663f655ece..82af4ec6d287c0 100644
--- a/drivers/infiniband/core/uverbs_std_types_flow_action.c
+++ b/drivers/infiniband/core/uverbs_std_types_flow_action.c
@@ -366,8 +366,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),
 	},
 };
 
@@ -379,8 +380,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),
 	},
 };
 
@@ -393,8 +394,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),
@@ -406,8 +406,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(
@@ -419,8 +418,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),
@@ -432,8 +430,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 8d4007a1361b9c..790d76297cd9a6 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 {
 /* Use 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.17.1

--
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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux