Various fixes collected over time

drivers/infiniband/core/uverbs_std_types_device.c +1 -1

@@ -336,19 +335,16 @@ static int UVERBS_HANDLER(UVERBS_METHOD_QUERY_GID_TABLE)(
 		attrs, UVERBS_ATTR_QUERY_GID_TABLE_RESP_ENTRIES,
 		user_entry_size);
 	if (max_entries <= 0)
-		return -EINVAL;
+		return max_entries ?: -EINVAL;
 
 	ucontext = ib_uverbs_get_ucontext(attrs);
 	if (IS_ERR(ucontext))
 		return PTR_ERR(ucontext);
 	ib_dev = ucontext->device;

drivers/infiniband/core/uverbs_std_types_device.c +0 -0

 	num_entries = rdma_query_gid_table(ib_dev, entries, max_entries);
 	if (num_entries < 0)

include/rdma/uverbs_ioctl.h +10 -0

index bf167ef6c688..39ef204753ec 100644
@@ -865,6 +865,16 @@ static inline __malloc void *uverbs_zalloc(struct uverbs_attr_bundle *bundle,
 {
 	return _uverbs_alloc(bundle, size, GFP_KERNEL | __GFP_ZERO);
 }
+
+static inline __malloc void *uverbs_kcalloc(struct uverbs_attr_bundle *bundle,
+					    size_t n, size_t size)
+{
+	size_t bytes;
+
+	if (unlikely(check_mul_overflow(n, size, &bytes)))
+		return ERR_PTR(-EOVERFLOW);
+	return uverbs_zalloc(bundle, bytes);
+}
 int _uverbs_get_const(s64 *to, const struct uverbs_attr_bundle *attrs_bundle,
 		      size_t idx, s64 lower_bound, u64 upper_bound,
 		      s64 *def_val);

Signature · 1 line

2.28.0

Quoted 12 lines · Leon Romanovsky

drivers/infiniband/core/uverbs_std_types_device.c +1 -1

@@ -336,19 +335,16 @@ static int UVERBS_HANDLER(UVERBS_METHOD_QUERY_GID_TABLE)(
 		attrs, UVERBS_ATTR_QUERY_GID_TABLE_RESP_ENTRIES,
 		user_entry_size);
 	if (max_entries <= 0)
-		return -EINVAL;
+		return max_entries ?: -EINVAL;
 
 	ucontext = ib_uverbs_get_ucontext(attrs);
 	if (IS_ERR(ucontext))
 		return PTR_ERR(ucontext);
 	ib_dev = ucontext->device;
 

drivers/infiniband/core/uverbs_std_types_device.c static int UVERBS_HANDLER(UVERBS_METHOD_QUERY_GID_TABLE)( · line 347

Quoted 9 lines

drivers/infiniband/core/uverbs_std_types_device.c +3 -6

-	if (check_mul_overflow(max_entries, sizeof(*entries), &num_bytes))
-		return -EINVAL;
-
-	entries = uverbs_zalloc(attrs, num_bytes);
-	if (!entries)
-		return -ENOMEM;
+	entries = uverbs_kcalloc(attrs, max_entries, sizeof(*entries));
+	if (IS_ERR(entries))
+		return PTR_ERR(entries);

This isn't right. The uverbs_kcalloc() should match every other kcalloc() function and return NULL on error. This actually buggy because it returns both is error pointers and NULL so it will lead to a NULL dereference.

Btw, when a function returns both error pointers and NULL the NULL return means that the feature has been deliberately disabled. It's not an error pointer because it's deliberate.

regards,
dan carpenter

Quoted 26 lines

drivers/infiniband/core/uverbs_std_types_device.c +0 -0

 	num_entries = rdma_query_gid_table(ib_dev, entries, max_entries);
 	if (num_entries < 0)

include/rdma/uverbs_ioctl.h +10 -1

index bf167ef6c688..39ef204753ec 100644
@@ -865,6 +865,16 @@ static inline __malloc void *uverbs_zalloc(struct uverbs_attr_bundle *bundle,
 {
 	return _uverbs_alloc(bundle, size, GFP_KERNEL | __GFP_ZERO);
 }
+
+static inline __malloc void *uverbs_kcalloc(struct uverbs_attr_bundle *bundle,
+					    size_t n, size_t size)
+{
+	size_t bytes;
+
+	if (unlikely(check_mul_overflow(n, size, &bytes)))
+		return ERR_PTR(-EOVERFLOW);
+	return uverbs_zalloc(bundle, bytes);
+}
 int _uverbs_get_const(s64 *to, const struct uverbs_attr_bundle *attrs_bundle,
 		      size_t idx, s64 lower_bound, u64 upper_bound,
 		      s64 *def_val);
--

2.28.0

Quoted 13 lines · Dan Carpenter

Quoted 12 lines · Leon Romanovsky

drivers/infiniband/core/uverbs_std_types_device.c +1 -1

@@ -336,19 +335,16 @@ static int UVERBS_HANDLER(UVERBS_METHOD_QUERY_GID_TABLE)(
 		attrs, UVERBS_ATTR_QUERY_GID_TABLE_RESP_ENTRIES,
 		user_entry_size);
 	if (max_entries <= 0)
-		return -EINVAL;
+		return max_entries ?: -EINVAL;
 
 	ucontext = ib_uverbs_get_ucontext(attrs);
 	if (IS_ERR(ucontext))
 		return PTR_ERR(ucontext);
 	ib_dev = ucontext->device;
 

Quoted 14 lines

drivers/infiniband/core/uverbs_std_types_device.c static int UVERBS_HANDLER(UVERBS_METHOD_QUERY_GID_TABLE)( · line 347

Quoted 9 lines

drivers/infiniband/core/uverbs_std_types_device.c +3 -6

-	if (check_mul_overflow(max_entries, sizeof(*entries), &num_bytes))
-		return -EINVAL;
-
-	entries = uverbs_zalloc(attrs, num_bytes);
-	if (!entries)
-		return -ENOMEM;
+	entries = uverbs_kcalloc(attrs, max_entries, sizeof(*entries));
+	if (IS_ERR(entries))
+		return PTR_ERR(entries);

This isn't right. The uverbs_kcalloc() should match every other kcalloc() function and return NULL on error. This actually buggy because it returns both is error pointers and NULL so it will lead to a NULL dereference.

The actual bug was before, when an error result from uverbs_zalloc() was treated as NULL. The uverbs_kcalloc/uverbs_zalloc will call to _uverbs_alloc() that doesn't return NULL.

Quoted 12 lines · Leon Romanovsky

drivers/infiniband/core/uverbs_std_types_device.c +1 -1

@@ -336,19 +335,16 @@ static int UVERBS_HANDLER(UVERBS_METHOD_QUERY_GID_TABLE)(
 		attrs, UVERBS_ATTR_QUERY_GID_TABLE_RESP_ENTRIES,
 		user_entry_size);
 	if (max_entries <= 0)
-		return -EINVAL;
+		return max_entries ?: -EINVAL;
 
 	ucontext = ib_uverbs_get_ucontext(attrs);
 	if (IS_ERR(ucontext))
 		return PTR_ERR(ucontext);
 	ib_dev = ucontext->device;
 

drivers/infiniband/core/uverbs_std_types_device.c static int UVERBS_HANDLER(UVERBS_METHOD_QUERY_GID_TABLE)( · line 347

Quoted 9 lines

drivers/infiniband/core/uverbs_std_types_device.c +3 -6

-	if (check_mul_overflow(max_entries, sizeof(*entries), &num_bytes))
-		return -EINVAL;
-
-	entries = uverbs_zalloc(attrs, num_bytes);
-	if (!entries)
-		return -ENOMEM;
+	entries = uverbs_kcalloc(attrs, max_entries, sizeof(*entries));
+	if (IS_ERR(entries))
+		return PTR_ERR(entries);

This isn't right. The uverbs_kcalloc() should match every other kcalloc() function and return NULL on error. This actually buggy because it returns both is error pointers and NULL so it will lead to a NULL dereference.

From: Leon Romanovsky <[email protected]>

Hi,

This is set of various and unrelated fixes that we collected over time.

Thanks

Avihai Horon (1):
RDMA/uverbs: Fix incorrect variable type

Jack Morgenstein (2):
RDMA/core: Clean up cq pool mechanism
RDMA/core: Do not indicate device ready when device enablement fails

 drivers/infiniband/core/core_priv.h              |  3 +--
 drivers/infiniband/core/cq.c                     | 12 ++----------
 drivers/infiniband/core/device.c                 | 16 ++++++++++------
 .../infiniband/core/uverbs_std_types_device.c    | 14 +++++---------
 include/rdma/uverbs_ioctl.h                      | 10 ++++++++++
 5 files changed, 28 insertions(+), 27 deletions(-)