[PATCH rdma-next v3 17/17] RDMA/uverbs: Track attr consumption and warn on unused attrs

[Jiri Pirko <jiri@resnulli.us>]

From: Jiri Pirko <jiri@nvidia.com>

Catch userspace passing attributes that nothing in the kernel
reads which would be a sign that the driver doesn't support
a feature, an attr was forgotten in a refactor, or userspace is buggy.
UHW and PTR_OUT attrs are exempt; destroy attrs are marked consumed by
the framework. Gate on CONFIG_DEBUG_KERNEL to avoid overhead on
production kernels.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- new patch
---
 drivers/infiniband/core/rdma_core.h    |  2 +-
 drivers/infiniband/core/uverbs_ioctl.c | 55 +++++++++++++++++++++++---
 include/rdma/uverbs_ioctl.h            | 54 ++++++++++++++-----------
 3 files changed, 82 insertions(+), 29 deletions(-)

diff --git a/drivers/infiniband/core/rdma_core.h b/drivers/infiniband/core/rdma_core.h
index 269b393799ab..06b735f6b3ac 100644
--- a/drivers/infiniband/core/rdma_core.h
+++ b/drivers/infiniband/core/rdma_core.h
@@ -67,7 +67,7 @@ void uverbs_finalize_object(struct ib_uobject *uobj,
 			    enum uverbs_obj_access access, bool hw_obj_valid,
 			    bool commit, struct uverbs_attr_bundle *attrs);
 
-int uverbs_output_written(const struct uverbs_attr_bundle *bundle, size_t idx);
+int uverbs_output_written(struct uverbs_attr_bundle *bundle, size_t idx);
 
 void setup_ufile_idr_uobject(struct ib_uverbs_file *ufile);
 void release_ufile_idr_uobject(struct ib_uverbs_file *ufile);
diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
index a2182d3401da..e626b7f4e6eb 100644
--- a/drivers/infiniband/core/uverbs_ioctl.c
+++ b/drivers/infiniband/core/uverbs_ioctl.c
@@ -438,6 +438,40 @@ static int uverbs_set_attr(struct bundle_priv *pbundle,
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_DEBUG_KERNEL)
+static void uverbs_check_attr_consumption(struct bundle_priv *pbundle)
+{
+	const struct uverbs_api_ioctl_method *method = pbundle->method_elm;
+	struct uverbs_attr_bundle *bundle =
+		container_of(&pbundle->bundle, struct uverbs_attr_bundle, hdr);
+	unsigned int bkey;
+
+	for_each_set_bit(bkey, bundle->attr_present, method->key_bitmap_len) {
+		const struct uverbs_api_attr *attr_uapi;
+		const struct uverbs_attr_spec *spec;
+		void __rcu **slot;
+
+		if (test_bit(bkey, bundle->attr_consumed))
+			continue;
+
+		slot = uapi_get_attr_for_method(pbundle,
+						uapi_bkey_to_key_attr(bkey));
+		if (!slot)
+			continue;
+		attr_uapi = rcu_dereference_protected(*slot, true);
+		spec = &attr_uapi->spec;
+
+		if (spec->is_udata)
+			continue;
+		if (spec->type == UVERBS_ATTR_TYPE_PTR_OUT)
+			continue;
+
+		pr_warn_ratelimited("uverbs: method_key=0x%x bkey=%u attr provided by user but not consumed\n",
+				    pbundle->method_key, bkey);
+	}
+}
+#endif
+
 static int ib_uverbs_run_method(struct bundle_priv *pbundle,
 				unsigned int num_attrs)
 {
@@ -487,6 +521,9 @@ static int ib_uverbs_run_method(struct bundle_priv *pbundle,
 		if (ret)
 			return ret;
 		__clear_bit(destroy_bkey, pbundle->uobj_finalize);
+#if IS_ENABLED(CONFIG_DEBUG_KERNEL)
+		__set_bit(destroy_bkey, bundle->attr_consumed);
+#endif
 
 		ret = handler(bundle);
 		uobj_put_destroy(destroy_attr->uobject);
@@ -515,6 +552,10 @@ static int ib_uverbs_run_method(struct bundle_priv *pbundle,
 	if (WARN_ON_ONCE(ret == -EPROTONOSUPPORT))
 		return -EINVAL;
 
+#if IS_ENABLED(CONFIG_DEBUG_KERNEL)
+	if (!ret)
+		uverbs_check_attr_consumption(pbundle);
+#endif
 	return ret;
 }
 
@@ -623,6 +664,10 @@ static int ib_uverbs_cmd_verbs(struct ib_uverbs_file *ufile,
 					       sizeof(*pbundle->internal_buffer));
 	memset(pbundle->bundle.attr_present, 0,
 	       sizeof(pbundle->bundle.attr_present));
+#if IS_ENABLED(CONFIG_DEBUG_KERNEL)
+	memset(pbundle->bundle.attr_consumed, 0,
+	       sizeof(pbundle->bundle.attr_consumed));
+#endif
 	memset(pbundle->uobj_finalize, 0, sizeof(pbundle->uobj_finalize));
 	memset(pbundle->spec_finalize, 0, sizeof(pbundle->spec_finalize));
 	memset(pbundle->uobj_hw_obj_valid, 0,
@@ -662,7 +707,7 @@ long ib_uverbs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	return err;
 }
 
-int uverbs_get_flags64(u64 *to, const struct uverbs_attr_bundle *attrs_bundle,
+int uverbs_get_flags64(u64 *to, struct uverbs_attr_bundle *attrs_bundle,
 		       size_t idx, u64 allowed_bits)
 {
 	const struct uverbs_attr *attr;
@@ -695,7 +740,7 @@ int uverbs_get_flags64(u64 *to, const struct uverbs_attr_bundle *attrs_bundle,
 }
 EXPORT_SYMBOL(uverbs_get_flags64);
 
-int uverbs_get_flags32(u32 *to, const struct uverbs_attr_bundle *attrs_bundle,
+int uverbs_get_flags32(u32 *to, struct uverbs_attr_bundle *attrs_bundle,
 		       size_t idx, u64 allowed_bits)
 {
 	u64 flags;
@@ -753,7 +798,7 @@ void uverbs_fill_udata(struct uverbs_attr_bundle *bundle,
 	}
 }
 
-int uverbs_copy_to(const struct uverbs_attr_bundle *bundle, size_t idx,
+int uverbs_copy_to(struct uverbs_attr_bundle *bundle, size_t idx,
 		   const void *from, size_t size)
 {
 	const struct uverbs_attr *attr = uverbs_attr_get(bundle, idx);
@@ -775,7 +820,7 @@ EXPORT_SYMBOL(uverbs_copy_to);
  * This is only used if the caller has directly used copy_to_use to write the
  * data.  It signals to user space that the buffer is filled in.
  */
-int uverbs_output_written(const struct uverbs_attr_bundle *bundle, size_t idx)
+int uverbs_output_written(struct uverbs_attr_bundle *bundle, size_t idx)
 {
 	const struct uverbs_attr *attr = uverbs_attr_get(bundle, idx);
 
@@ -785,7 +830,7 @@ int uverbs_output_written(const struct uverbs_attr_bundle *bundle, size_t idx)
 	return uverbs_set_output(bundle, attr);
 }
 
-int uverbs_copy_to_struct_or_zero(const struct uverbs_attr_bundle *bundle,
+int uverbs_copy_to_struct_or_zero(struct uverbs_attr_bundle *bundle,
 				  size_t idx, const void *from, size_t size)
 {
 	const struct uverbs_attr *attr = uverbs_attr_get(bundle, idx);
diff --git a/include/rdma/uverbs_ioctl.h b/include/rdma/uverbs_ioctl.h
index 70caa7299dbf..386732094978 100644
--- a/include/rdma/uverbs_ioctl.h
+++ b/include/rdma/uverbs_ioctl.h
@@ -648,6 +648,9 @@ struct uverbs_attr_bundle {
 		struct ib_ucontext *context;
 		struct ib_uobject *uobject;
 		DECLARE_BITMAP(attr_present, UVERBS_API_ATTR_BKEY_LEN);
+#if IS_ENABLED(CONFIG_DEBUG_KERNEL)
+		DECLARE_BITMAP(attr_consumed, UVERBS_API_ATTR_BKEY_LEN);
+#endif
 	);
 	struct uverbs_attr attrs[];
 };
@@ -683,16 +686,20 @@ struct ib_device *rdma_udata_to_dev(struct ib_udata *udata);
 
 #define IS_UVERBS_COPY_ERR(_ret)		((_ret) && (_ret) != -ENOENT)
 
-static inline const struct uverbs_attr *uverbs_attr_get(const struct uverbs_attr_bundle *attrs_bundle,
+static inline const struct uverbs_attr *uverbs_attr_get(struct uverbs_attr_bundle *attrs_bundle,
 							u16 idx)
 {
 	if (!uverbs_attr_is_valid(attrs_bundle, idx))
 		return ERR_PTR(-ENOENT);
 
+#if IS_ENABLED(CONFIG_DEBUG_KERNEL)
+	__set_bit(uapi_bkey_attr(uapi_key_attr(idx)),
+		  attrs_bundle->attr_consumed);
+#endif
 	return &attrs_bundle->attrs[uapi_bkey_attr(uapi_key_attr(idx))];
 }
 
-static inline int uverbs_attr_get_enum_id(const struct uverbs_attr_bundle *attrs_bundle,
+static inline int uverbs_attr_get_enum_id(struct uverbs_attr_bundle *attrs_bundle,
 					  u16 idx)
 {
 	const struct uverbs_attr *attr = uverbs_attr_get(attrs_bundle, idx);
@@ -703,7 +710,7 @@ static inline int uverbs_attr_get_enum_id(const struct uverbs_attr_bundle *attrs
 	return attr->ptr_attr.enum_id;
 }
 
-static inline void *uverbs_attr_get_obj(const struct uverbs_attr_bundle *attrs_bundle,
+static inline void *uverbs_attr_get_obj(struct uverbs_attr_bundle *attrs_bundle,
 					u16 idx)
 {
 	const struct uverbs_attr *attr;
@@ -715,7 +722,7 @@ static inline void *uverbs_attr_get_obj(const struct uverbs_attr_bundle *attrs_b
 	return attr->obj_attr.uobject->object;
 }
 
-static inline struct ib_uobject *uverbs_attr_get_uobject(const struct uverbs_attr_bundle *attrs_bundle,
+static inline struct ib_uobject *uverbs_attr_get_uobject(struct uverbs_attr_bundle *attrs_bundle,
 							 u16 idx)
 {
 	const struct uverbs_attr *attr = uverbs_attr_get(attrs_bundle, idx);
@@ -727,7 +734,7 @@ static inline struct ib_uobject *uverbs_attr_get_uobject(const struct uverbs_att
 }
 
 static inline int
-uverbs_attr_get_len(const struct uverbs_attr_bundle *attrs_bundle, u16 idx)
+uverbs_attr_get_len(struct uverbs_attr_bundle *attrs_bundle, u16 idx)
 {
 	const struct uverbs_attr *attr = uverbs_attr_get(attrs_bundle, idx);
 
@@ -771,7 +778,7 @@ uverbs_attr_ptr_get_array_size(struct uverbs_attr_bundle *attrs, u16 idx,
  * Return: The array length or 0 if no attribute was provided.
  */
 static inline int uverbs_attr_get_uobjs_arr(
-	const struct uverbs_attr_bundle *attrs_bundle, u16 attr_idx,
+	struct uverbs_attr_bundle *attrs_bundle, u16 attr_idx,
 	struct ib_uobject ***arr)
 {
 	const struct uverbs_attr *attr =
@@ -793,7 +800,7 @@ static inline bool uverbs_attr_ptr_is_inline(const struct uverbs_attr *attr)
 }
 
 static inline void *uverbs_attr_get_alloced_ptr(
-	const struct uverbs_attr_bundle *attrs_bundle, u16 idx)
+	struct uverbs_attr_bundle *attrs_bundle, u16 idx)
 {
 	const struct uverbs_attr *attr = uverbs_attr_get(attrs_bundle, idx);
 
@@ -805,7 +812,7 @@ static inline void *uverbs_attr_get_alloced_ptr(
 }
 
 static inline int _uverbs_copy_from(void *to,
-				    const struct uverbs_attr_bundle *attrs_bundle,
+				    struct uverbs_attr_bundle *attrs_bundle,
 				    size_t idx,
 				    size_t size)
 {
@@ -832,7 +839,7 @@ static inline int _uverbs_copy_from(void *to,
 }
 
 static inline int _uverbs_copy_from_or_zero(void *to,
-					    const struct uverbs_attr_bundle *attrs_bundle,
+					    struct uverbs_attr_bundle *attrs_bundle,
 					    size_t idx,
 					    size_t size)
 {
@@ -869,11 +876,11 @@ ib_uverbs_get_ucontext(const struct uverbs_attr_bundle *attrs)
 }
 
 #if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS)
-int uverbs_get_flags64(u64 *to, const struct uverbs_attr_bundle *attrs_bundle,
+int uverbs_get_flags64(u64 *to, struct uverbs_attr_bundle *attrs_bundle,
 		       size_t idx, u64 allowed_bits);
-int uverbs_get_flags32(u32 *to, const struct uverbs_attr_bundle *attrs_bundle,
+int uverbs_get_flags32(u32 *to, struct uverbs_attr_bundle *attrs_bundle,
 		       size_t idx, u64 allowed_bits);
-int uverbs_copy_to(const struct uverbs_attr_bundle *attrs_bundle, size_t idx,
+int uverbs_copy_to(struct uverbs_attr_bundle *attrs_bundle, size_t idx,
 		   const void *from, size_t size);
 __malloc void *_uverbs_alloc(struct uverbs_attr_bundle *bundle, size_t size,
 			     gfp_t flags);
@@ -902,7 +909,7 @@ static inline __malloc void *uverbs_kcalloc(struct uverbs_attr_bundle *bundle,
 
 static inline int
 _uverbs_get_const_signed(s64 *to,
-			 const struct uverbs_attr_bundle *attrs_bundle,
+			 struct uverbs_attr_bundle *attrs_bundle,
 			 size_t idx, s64 lower_bound, u64 upper_bound,
 			 s64 *def_val)
 {
@@ -925,7 +932,7 @@ _uverbs_get_const_signed(s64 *to,
 
 static inline int
 _uverbs_get_const_unsigned(u64 *to,
-			   const struct uverbs_attr_bundle *attrs_bundle,
+			   struct uverbs_attr_bundle *attrs_bundle,
 			   size_t idx, u64 upper_bound, u64 *def_val)
 {
 	const struct uverbs_attr *attr;
@@ -944,7 +951,8 @@ _uverbs_get_const_unsigned(u64 *to,
 
 	return 0;
 }
-int uverbs_copy_to_struct_or_zero(const struct uverbs_attr_bundle *bundle,
+
+int uverbs_copy_to_struct_or_zero(struct uverbs_attr_bundle *bundle,
 				  size_t idx, const void *from, size_t size);
 
 int _ib_copy_validate_udata_in(struct ib_udata *udata, void *req,
@@ -952,18 +960,18 @@ int _ib_copy_validate_udata_in(struct ib_udata *udata, void *req,
 int _ib_respond_udata(struct ib_udata *udata, const void *src, size_t len);
 #else
 static inline int
-uverbs_get_flags64(u64 *to, const struct uverbs_attr_bundle *attrs_bundle,
+uverbs_get_flags64(u64 *to, struct uverbs_attr_bundle *attrs_bundle,
 		   size_t idx, u64 allowed_bits)
 {
 	return -EINVAL;
 }
 static inline int
-uverbs_get_flags32(u32 *to, const struct uverbs_attr_bundle *attrs_bundle,
+uverbs_get_flags32(u32 *to, struct uverbs_attr_bundle *attrs_bundle,
 		   size_t idx, u64 allowed_bits)
 {
 	return -EINVAL;
 }
-static inline int uverbs_copy_to(const struct uverbs_attr_bundle *attrs_bundle,
+static inline int uverbs_copy_to(struct uverbs_attr_bundle *attrs_bundle,
 				 size_t idx, const void *from, size_t size)
 {
 	return -EINVAL;
@@ -979,21 +987,21 @@ static inline __malloc void *uverbs_zalloc(struct uverbs_attr_bundle *bundle,
 	return ERR_PTR(-EINVAL);
 }
 static inline int
-_uverbs_get_const(s64 *to, const struct uverbs_attr_bundle *attrs_bundle,
+_uverbs_get_const(s64 *to, struct uverbs_attr_bundle *attrs_bundle,
 		  size_t idx, s64 lower_bound, u64 upper_bound,
 		  s64 *def_val)
 {
 	return -EINVAL;
 }
 static inline int
-uverbs_copy_to_struct_or_zero(const struct uverbs_attr_bundle *bundle,
+uverbs_copy_to_struct_or_zero(struct uverbs_attr_bundle *bundle,
 			      size_t idx, const void *from, size_t size)
 {
 	return -EINVAL;
 }
 static inline int
 _uverbs_get_const_signed(s64 *to,
-			 const struct uverbs_attr_bundle *attrs_bundle,
+			 struct uverbs_attr_bundle *attrs_bundle,
 			 size_t idx, s64 lower_bound, u64 upper_bound,
 			 s64 *def_val)
 {
@@ -1001,7 +1009,7 @@ _uverbs_get_const_signed(s64 *to,
 }
 static inline int
 _uverbs_get_const_unsigned(u64 *to,
-			   const struct uverbs_attr_bundle *attrs_bundle,
+			   struct uverbs_attr_bundle *attrs_bundle,
 			   size_t idx, u64 upper_bound, u64 *def_val)
 {
 	return -EINVAL;
@@ -1078,7 +1086,7 @@ static inline int _ib_respond_udata(struct ib_udata *udata, const void *src,
 						    _default))
 
 static inline int
-uverbs_get_raw_fd(int *to, const struct uverbs_attr_bundle *attrs_bundle,
+uverbs_get_raw_fd(int *to, struct uverbs_attr_bundle *attrs_bundle,
 		  size_t idx)
 {
 	return uverbs_get_const_signed(to, attrs_bundle, idx);
-- 
2.53.0