infiniband: improve userspace input check

PATCH New Yann Droneaud 1 participant 23 messages Aug 18, 2013 1 message not in the archive InfiniBand

lore

Share & tools

lore

Yann Droneaud

author

2013-08-18 18:28 UTC #0 lore raw

cover infiniband: improve userspace input check

Hi,

Please find a few patches against branch for-next of
git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git,
current head 3e7645800a89f2adccb8d635d36f2fb648048141.

This is a collection of patches to add more checks on userspace provided input parameters to cm, cma and verbs subsystem:
The InfiniBand/RDMA subsystems are not currently making enough checks on user input data. In particular:
- they should not read past the input buffer,
- they should not write past the output buffer.
Each of these userspace APIs should use the user provided lengths for buffers and don't read/write out of those boundaries.

I hope the proposed fixes being straightforward and not disruptive.

I've made some tests, but I cannot test every code path without a proper test suite and the matching infrastructure.

I've tested on a small InfiniBand fabric, with a 8 ports switch, one node with 2 HCAs (1 port, 2 ports), a second node with 1 HCA (1 port), each port connected to the switch. I've also tested with two node connected back to back.

I've searched for regression using the following programs:

ibaddr
ibstat
ibstatus
ibportstate <lid> query
ibroute <lid>
ibswitches
sminfo
smpquery portinfo <lid>
smpquery nodeinfo <lid>
saquery
ibv_devices
ibv_devinfo
rdma_client/rdma_server
rdma_xclient/rdma_xserver
ibv_rc_pingpong
ib_{read,write}_{bw,lat}

I've tested only with Fedora 19 using:

     libipathverbs 1.2,
     libmlx4 1.0.5,
     libibverbs 1.1.17,
     librdmacm 1.0.17,
     libibmad 1.3.9,
     libibumad 1.3.8,
     opensm 3.3.15,
     infiniband-diags 1.6.1.

Testing with older/other libraries/tools could be interesting.

Please have a good review. I'm ready to anwser questions and update the patchset.

Regards.

Yann Droneaud (22):
infiniband: ib_copy_{from,to}_udata(): const'ify arguments
infiniband: ib_udata: const'ify inbuf
infiniband: ib_copy_from_udata(): check input length
infiniband: ib_copy_to_udata(): check output length
ucm: ib_ucm_event(): returns ENOSPC instead of ENOMEM
ucm: changes ib_ucm_alloc_data() src arg to be a pointer
ucm: changes ib_ucm_path_get() src arg to be a pointer
ucm: check userspace input length
ucm: use size_t for userspace buffer input and output length
ucm: ib_ucm_write(): check userspace buffer length first
ucm: ib_ucm_write(): strict userspace buffer length check
ucma: check userspace input length
ucma: ucma_migrate_id(): check output length
ucma: ucma_query_path(): check output length
ucma: use size_t for userspace buffer input and output length
ucma: ucma_write(): check userspace buffer length first
ucma: ucma_write(): strict userspace buffer length check
uverbs: check input length
uverbs: check output length
uverbs: ib_uverbs_poll_cq(): check output length against number of wc
uverbs: fix call to INIT_UDATA with no output buffer
uverbs: use size_t for userspace buffer input and output length

 drivers/infiniband/core/ucm.c         | 156 ++++++++++++++------
 drivers/infiniband/core/ucma.c        | 131 +++++++++++++----
 drivers/infiniband/core/uverbs.h      |   4 +-
 drivers/infiniband/core/uverbs_cmd.c  | 258 +++++++++++++++++++++++++---------
 drivers/infiniband/core/uverbs_main.c |   4 +-
 include/rdma/ib_verbs.h               |  12 +-
 6 files changed, 422 insertions(+), 143 deletions(-)

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

Yann Droneaud

author

2013-08-18 18:28 UTC #2 lore raw

PATCH 1/22 infiniband: ib_copy_{from,to}_udata(): const'ify arguments

Source buffers are not modified, let them be 'const'.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 include/rdma/ib_verbs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

include/rdma/ib_verbs.h +2 -2

index a84d3df..ef37c59 100644
@@ -1353,12 +1353,12 @@ void *ib_get_client_data(struct ib_device *device, struct ib_client *client);
 void  ib_set_client_data(struct ib_device *device, struct ib_client *client,
 			 void *data);
 
-static inline int ib_copy_from_udata(void *dest, struct ib_udata *udata, size_t len)
+static inline int ib_copy_from_udata(void *dest, const struct ib_udata *udata, size_t len)
 {
 	return copy_from_user(dest, udata->inbuf, len) ? -EFAULT : 0;
 }
 
-static inline int ib_copy_to_udata(struct ib_udata *udata, void *src, size_t len)
+static inline int ib_copy_to_udata(struct ib_udata *udata, const void *src, size_t len)
 {
 	return copy_to_user(udata->outbuf, src, len) ? -EFAULT : 0;
 }

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #3 lore raw

PATCH 2/22 infiniband: ib_udata: const'ify inbuf

In reply to a message not in the archive

Userspace input buffer is not modified by kernel, let it be 'const'.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/uverbs_cmd.c | 2 +-
 include/rdma/ib_verbs.h              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

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

index b105140..6ca3c86 100644
@@ -57,7 +57,7 @@ static struct uverbs_lock_class xrcd_lock_class = { .name = "XRCD-uobj" };
 
 #define INIT_UDATA(udata, ibuf, obuf, ilen, olen)			\
 	do {								\
-		(udata)->inbuf  = (void __user *) (ibuf);		\
+		(udata)->inbuf  = (const void __user *) (ibuf);		\
 		(udata)->outbuf = (void __user *) (obuf);		\
 		(udata)->inlen  = (ilen);				\
 		(udata)->outlen = (olen);				\

include/rdma/ib_verbs.h +1 -1

index ef37c59..21dbc42 100644
@@ -974,7 +974,7 @@ struct ib_uobject {
 };
 
 struct ib_udata {
-	void __user *inbuf;
+	const void __user *inbuf;
 	void __user *outbuf;
 	size_t       inlen;
 	size_t       outlen;

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #4 lore raw

PATCH 3/22 infiniband: ib_copy_from_udata(): check input length

In reply to a message not in the archive

This patch makes ib_copy_from_udata() check the input length before reading from userspace buffer to prevent out of bound access.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 include/rdma/ib_verbs.h | 3 +++
 1 file changed, 3 insertions(+)

include/rdma/ib_verbs.h +3 -0

index 21dbc42..e5551ac 100644
@@ -1355,6 +1355,9 @@ void  ib_set_client_data(struct ib_device *device, struct ib_client *client,
 
 static inline int ib_copy_from_udata(void *dest, const struct ib_udata *udata, size_t len)
 {
+	if (udata->inlen < len) {
+		return -EINVAL;
+	}
 	return copy_from_user(dest, udata->inbuf, len) ? -EFAULT : 0;
 }

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #5 lore raw

PATCH 4/22 infiniband: ib_copy_to_udata(): check output length

In reply to a message not in the archive

This patch makes ib_copy_to_udata() check the output length before writing to userspace buffer to prevent out of bound access.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 include/rdma/ib_verbs.h | 3 +++
 1 file changed, 3 insertions(+)

include/rdma/ib_verbs.h +3 -0

index e5551ac..c59a365 100644
@@ -1363,6 +1363,9 @@ static inline int ib_copy_from_udata(void *dest, const struct ib_udata *udata, s
 
 static inline int ib_copy_to_udata(struct ib_udata *udata, const void *src, size_t len)
 {
+	if (udata->outlen < len) {
+		return -ENOSPC;
+	}
 	return copy_to_user(udata->outbuf, src, len) ? -EFAULT : 0;
 }

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #6 lore raw

PATCH 5/22 ucm: ib_ucm_event(): returns ENOSPC instead of ENOMEM

In reply to a message not in the archive

When output buffer is too small, others infiniband functions returns ENOSPC.
This patch changes ib_ucm_event() to returns ENOSPC just like others functions.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/infiniband/core/ucm.c +2 -2

index f2f6393..e462208 100644
@@ -432,7 +432,7 @@ static ssize_t ib_ucm_event(struct ib_ucm_file *file,
 
 	if (uevent->data) {
 		if (cmd.data_len < uevent->data_len) {
-			result = -ENOMEM;
+			result = -ENOSPC;
 			goto done;
 		}
 		if (copy_to_user((void __user *)(unsigned long)cmd.data,

@@ -444,7 +444,7 @@ static ssize_t ib_ucm_event(struct ib_ucm_file *file,
 
 	if (uevent->info) {
 		if (cmd.info_len < uevent->info_len) {
-			result = -ENOMEM;
+			result = -ENOSPC;
 			goto done;
 		}
 		if (copy_to_user((void __user *)(unsigned long)cmd.info,

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #7 lore raw

PATCH 6/22 ucm: changes ib_ucm_alloc_data() src arg to be a pointer

In reply to a message not in the archive

ib_icm_alloc_data() is a function to allocate a buffer to copy a userspace provided buffer.
This patch makes 'src' argument to be an explicit pointer to userspace buffer, so that static analysis won't get fooled by 'src' being currently an integer without annotation.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucm.c | 43 +++++++++++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 12 deletions(-)

drivers/infiniband/core/ucm.c +31 ~~-12~~

index e462208..d336a1b 100644
@@ -685,7 +685,7 @@ static ssize_t ib_ucm_notify(struct ib_ucm_file *file,
 	return result;
 }
 
-static int ib_ucm_alloc_data(const void **dest, u64 src, u32 len)
+static int ib_ucm_alloc_data(const void **dest, const void __user *src, u32 len)
 {
 	void *data;

@@ -694,7 +694,7 @@ static int ib_ucm_alloc_data(const void **dest, u64 src, u32 len)
 	if (!len)
 		return 0;
 
-	data = memdup_user((void __user *)(unsigned long)src, len);
+	data = memdup_user(src, len);
 	if (IS_ERR(data))
 		return PTR_ERR(data);

@@ -744,7 +744,9 @@ static ssize_t ib_ucm_send_req(struct ib_ucm_file *file,
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;
 
-	result = ib_ucm_alloc_data(&param.private_data, cmd.data, cmd.len);
+	result = ib_ucm_alloc_data(&param.private_data,
+				   (const void __user *)(unsigned long)cmd.data,
+				   cmd.len);
 	if (result)
 		goto done;

@@ -800,7 +802,9 @@ static ssize_t ib_ucm_send_rep(struct ib_ucm_file *file,
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;
 
-	result = ib_ucm_alloc_data(&param.private_data, cmd.data, cmd.len);
+	result = ib_ucm_alloc_data(&param.private_data,
+				   (const void __user *)(unsigned long)cmd.data,
+				   cmd.len);
 	if (result)
 		return result;

@@ -840,7 +844,9 @@ static ssize_t ib_ucm_send_private_data(struct ib_ucm_file *file,
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;
 
-	result = ib_ucm_alloc_data(&private_data, cmd.data, cmd.len);
+	result = ib_ucm_alloc_data(&private_data,
+				   (const void __user *)(unsigned long)cmd.data,
+				   cmd.len);
 	if (result)
 		return result;

@@ -894,11 +900,15 @@ static ssize_t ib_ucm_send_info(struct ib_ucm_file *file,
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;
 
-	result = ib_ucm_alloc_data(&data, cmd.data, cmd.data_len);
+	result = ib_ucm_alloc_data(&data,
+				   (const void __user *)(unsigned long)cmd.data,
+				   cmd.data_len);
 	if (result)
 		goto done;
 
-	result = ib_ucm_alloc_data(&info, cmd.info, cmd.info_len);
+	result = ib_ucm_alloc_data(&info,
+				   (const void __user *)(unsigned long)cmd.info,
+				   cmd.info_len);
 	if (result)
 		goto done;

@@ -942,7 +952,9 @@ static ssize_t ib_ucm_send_mra(struct ib_ucm_file *file,
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;
 
-	result = ib_ucm_alloc_data(&data, cmd.data, cmd.len);
+	result = ib_ucm_alloc_data(&data,
+				   (const void __user *)(unsigned long)cmd.data,
+				   cmd.len);
 	if (result)
 		return result;

@@ -970,7 +982,9 @@ static ssize_t ib_ucm_send_lap(struct ib_ucm_file *file,
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;
 
-	result = ib_ucm_alloc_data(&data, cmd.data, cmd.len);
+	result = ib_ucm_alloc_data(&data,
+				   (const void __user *)(unsigned long)cmd.data,
+				   cmd.len);
 	if (result)
 		goto done;

@@ -1006,7 +1020,9 @@ static ssize_t ib_ucm_send_sidr_req(struct ib_ucm_file *file,
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;
 
-	result = ib_ucm_alloc_data(&param.private_data, cmd.data, cmd.len);
+	result = ib_ucm_alloc_data(&param.private_data,
+				   (const void __user *)(unsigned long)cmd.data,
+				   cmd.len);
 	if (result)
 		goto done;

@@ -1047,11 +1063,14 @@ static ssize_t ib_ucm_send_sidr_rep(struct ib_ucm_file *file,
 		return -EFAULT;
 
 	result = ib_ucm_alloc_data(&param.private_data,
-				   cmd.data, cmd.data_len);
+				   (void __user *)(unsigned long)cmd.data,
+				   cmd.data_len);
 	if (result)
 		goto done;
 
-	result = ib_ucm_alloc_data(&param.info, cmd.info, cmd.info_len);
+	result = ib_ucm_alloc_data(&param.info,
+				   (void __user *)(unsigned long)cmd.info,
+				   cmd.info_len);
 	if (result)
 		goto done;

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #8 lore raw

PATCH 7/22 ucm: changes ib_ucm_path_get() src arg to be a pointer

In reply to a message not in the archive

ib_icm_path_get() is a function that read data from userspace buffer. This patch makes 'src' argument to be an explicit pointer to userspace buffer, so that static analysis won't get fooled by 'src' being currently an integer without annotation.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucm.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

drivers/infiniband/core/ucm.c +10 -7

index d336a1b..b53e59b 100644
@@ -702,7 +702,7 @@ static int ib_ucm_alloc_data(const void **dest, const void __user *src, u32 len)
 	return 0;
 }
 
-static int ib_ucm_path_get(struct ib_sa_path_rec **path, u64 src)
+static int ib_ucm_path_get(struct ib_sa_path_rec **path, const void __user *src)
 {
 	struct ib_user_path_rec upath;
 	struct ib_sa_path_rec  *sa_path;

@@ -716,8 +716,7 @@ static int ib_ucm_path_get(struct ib_sa_path_rec **path, u64 src)
 	if (!sa_path)
 		return -ENOMEM;
 
-	if (copy_from_user(&upath, (void __user *)(unsigned long)src,
-			   sizeof(upath))) {
+	if (copy_from_user(&upath, src, sizeof(upath))) {
 
 		kfree(sa_path);
 		return -EFAULT;

@@ -750,11 +749,13 @@ static ssize_t ib_ucm_send_req(struct ib_ucm_file *file,
 	if (result)
 		goto done;
 
-	result = ib_ucm_path_get(&param.primary_path, cmd.primary_path);
+	result = ib_ucm_path_get(&param.primary_path,
+				 (const void __user *)(unsigned long)cmd.primary_path);
 	if (result)
 		goto done;
 
-	result = ib_ucm_path_get(&param.alternate_path, cmd.alternate_path);
+	result = ib_ucm_path_get(&param.alternate_path,
+				 (const void __user *)(unsigned long)cmd.alternate_path);
 	if (result)
 		goto done;

@@ -988,7 +989,8 @@ static ssize_t ib_ucm_send_lap(struct ib_ucm_file *file,
 	if (result)
 		goto done;
 
-	result = ib_ucm_path_get(&path, cmd.path);
+	result = ib_ucm_path_get(&path,
+				 (const void __user *)(unsigned long)cmd.path);
 	if (result)
 		goto done;

@@ -1026,7 +1028,8 @@ static ssize_t ib_ucm_send_sidr_req(struct ib_ucm_file *file,
 	if (result)
 		goto done;
 
-	result = ib_ucm_path_get(&param.path, cmd.path);
+	result = ib_ucm_path_get(&param.path,
+				 (const void __user *)(unsigned long)cmd.path);
 	if (result)
 		goto done;

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #9 lore raw

PATCH 8/22 ucm: check userspace input length

In reply to a message not in the archive

Makes ucm functions check the length of the input buffer before reading the command content: this will detect truncated command and will prevent ucm from reading past userspace provided buffer.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucm.c | 45 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

drivers/infiniband/core/ucm.c +45 -0

index b53e59b..7a35520 100644
@@ -390,6 +390,9 @@ static ssize_t ib_ucm_event(struct ib_ucm_file *file,
 	struct ib_ucm_event *uevent;
 	int result = 0;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(struct ib_ucm_event_resp))
 		return -ENOSPC;

@@ -475,6 +478,9 @@ static ssize_t ib_ucm_create_id(struct ib_ucm_file *file,
 	struct ib_ucm_context *ctx;
 	int result;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(resp))
 		return -ENOSPC;

@@ -522,6 +528,9 @@ static ssize_t ib_ucm_destroy_id(struct ib_ucm_file *file,
 	struct ib_ucm_context *ctx;
 	int result = 0;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(resp))
 		return -ENOSPC;

@@ -567,6 +576,9 @@ static ssize_t ib_ucm_attr_id(struct ib_ucm_file *file,
 	struct ib_ucm_context *ctx;
 	int result = 0;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(resp))
 		return -ENOSPC;

@@ -600,6 +612,9 @@ static ssize_t ib_ucm_init_qp_attr(struct ib_ucm_file *file,
 	struct ib_qp_attr qp_attr;
 	int result = 0;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(resp))
 		return -ENOSPC;

@@ -647,6 +662,9 @@ static ssize_t ib_ucm_listen(struct ib_ucm_file *file,
 	struct ib_ucm_context *ctx;
 	int result;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -673,6 +691,9 @@ static ssize_t ib_ucm_notify(struct ib_ucm_file *file,
 	struct ib_ucm_context *ctx;
 	int result;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -740,6 +761,9 @@ static ssize_t ib_ucm_send_req(struct ib_ucm_file *file,
 	param.primary_path   = NULL;
 	param.alternate_path = NULL;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -800,6 +824,9 @@ static ssize_t ib_ucm_send_rep(struct ib_ucm_file *file,
 
 	param.private_data = NULL;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -842,6 +869,9 @@ static ssize_t ib_ucm_send_private_data(struct ib_ucm_file *file,
 	const void *private_data = NULL;
 	int result;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -898,6 +928,9 @@ static ssize_t ib_ucm_send_info(struct ib_ucm_file *file,
 	const void *info = NULL;
 	int result;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -950,6 +983,9 @@ static ssize_t ib_ucm_send_mra(struct ib_ucm_file *file,
 	const void *data = NULL;
 	int result;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -980,6 +1016,9 @@ static ssize_t ib_ucm_send_lap(struct ib_ucm_file *file,
 	const void *data = NULL;
 	int result;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -1019,6 +1058,9 @@ static ssize_t ib_ucm_send_sidr_req(struct ib_ucm_file *file,
 	param.private_data = NULL;
 	param.path = NULL;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -1062,6 +1104,9 @@ static ssize_t ib_ucm_send_sidr_rep(struct ib_ucm_file *file,
 
 	param.info = NULL;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #10 lore raw

PATCH 9/22 ucm: use size_t for userspace buffer input and output length

In reply to a message not in the archive

There's no reason to use a signed type for userspace buffer length.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucm.c | 45 ++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

drivers/infiniband/core/ucm.c +23 ~~-22~~

index 7a35520..8f8a0a8 100644
@@ -383,7 +383,7 @@ err1:
 
 static ssize_t ib_ucm_event(struct ib_ucm_file *file,
 			    const char __user *inbuf,
-			    int in_len, int out_len)
+			    size_t in_len, size_t out_len)
 {
 	struct ib_ucm_context *ctx;
 	struct ib_ucm_event_get cmd;

@@ -471,7 +471,7 @@ done:
 
 static ssize_t ib_ucm_create_id(struct ib_ucm_file *file,
 				const char __user *inbuf,
-				int in_len, int out_len)
+				size_t in_len, size_t out_len)
 {
 	struct ib_ucm_create_id cmd;
 	struct ib_ucm_create_id_resp resp;

@@ -521,7 +521,7 @@ err1:
 
 static ssize_t ib_ucm_destroy_id(struct ib_ucm_file *file,
 				 const char __user *inbuf,
-				 int in_len, int out_len)
+				 size_t in_len, size_t out_len)
 {
 	struct ib_ucm_destroy_id cmd;
 	struct ib_ucm_destroy_id_resp resp;

@@ -569,7 +569,7 @@ static ssize_t ib_ucm_destroy_id(struct ib_ucm_file *file,
 
 static ssize_t ib_ucm_attr_id(struct ib_ucm_file *file,
 			      const char __user *inbuf,
-			      int in_len, int out_len)
+			      size_t in_len, size_t out_len)
 {
 	struct ib_ucm_attr_id_resp resp;
 	struct ib_ucm_attr_id cmd;

@@ -604,7 +604,7 @@ static ssize_t ib_ucm_attr_id(struct ib_ucm_file *file,
 
 static ssize_t ib_ucm_init_qp_attr(struct ib_ucm_file *file,
 				   const char __user *inbuf,
-				   int in_len, int out_len)
+				   size_t in_len, size_t out_len)
 {
 	struct ib_uverbs_qp_attr resp;
 	struct ib_ucm_init_qp_attr cmd;

@@ -656,7 +656,7 @@ static int ucm_validate_listen(__be64 service_id, __be64 service_mask)
 
 static ssize_t ib_ucm_listen(struct ib_ucm_file *file,
 			     const char __user *inbuf,
-			     int in_len, int out_len)
+			     size_t in_len, size_t out_len)
 {
 	struct ib_ucm_listen cmd;
 	struct ib_ucm_context *ctx;

@@ -685,7 +685,7 @@ out:
 
 static ssize_t ib_ucm_notify(struct ib_ucm_file *file,
 			     const char __user *inbuf,
-			     int in_len, int out_len)
+			     size_t in_len, size_t out_len)
 {
 	struct ib_ucm_notify cmd;
 	struct ib_ucm_context *ctx;

@@ -706,7 +706,7 @@ static ssize_t ib_ucm_notify(struct ib_ucm_file *file,
 	return result;
 }
 
-static int ib_ucm_alloc_data(const void **dest, const void __user *src, u32 len)
+static int ib_ucm_alloc_data(const void **dest, const void __user *src, size_t len)
 {
 	void *data;

@@ -750,7 +750,7 @@ static int ib_ucm_path_get(struct ib_sa_path_rec **path, const void __user *src)
 
 static ssize_t ib_ucm_send_req(struct ib_ucm_file *file,
 			       const char __user *inbuf,
-			       int in_len, int out_len)
+			       size_t in_len, size_t out_len)
 {
 	struct ib_cm_req_param param;
 	struct ib_ucm_context *ctx;

@@ -815,7 +815,7 @@ done:
 
 static ssize_t ib_ucm_send_rep(struct ib_ucm_file *file,
 			       const char __user *inbuf,
-			       int in_len, int out_len)
+			       size_t in_len, size_t out_len)
 {
 	struct ib_cm_rep_param param;
 	struct ib_ucm_context *ctx;

@@ -859,7 +859,7 @@ static ssize_t ib_ucm_send_rep(struct ib_ucm_file *file,
 }
 
 static ssize_t ib_ucm_send_private_data(struct ib_ucm_file *file,
-					const char __user *inbuf, int in_len,
+					const char __user *inbuf, size_t in_len,
 					int (*func)(struct ib_cm_id *cm_id,
 						    const void *private_data,
 						    u8 private_data_len))

@@ -894,27 +894,27 @@ static ssize_t ib_ucm_send_private_data(struct ib_ucm_file *file,
 
 static ssize_t ib_ucm_send_rtu(struct ib_ucm_file *file,
 			       const char __user *inbuf,
-			       int in_len, int out_len)
+			       size_t in_len, size_t out_len)
 {
 	return ib_ucm_send_private_data(file, inbuf, in_len, ib_send_cm_rtu);
 }
 
 static ssize_t ib_ucm_send_dreq(struct ib_ucm_file *file,
 				const char __user *inbuf,
-				int in_len, int out_len)
+				size_t in_len, size_t out_len)
 {
 	return ib_ucm_send_private_data(file, inbuf, in_len, ib_send_cm_dreq);
 }
 
 static ssize_t ib_ucm_send_drep(struct ib_ucm_file *file,
 				const char __user *inbuf,
-				int in_len, int out_len)
+				size_t in_len, size_t out_len)
 {
 	return ib_ucm_send_private_data(file, inbuf, in_len, ib_send_cm_drep);
 }
 
 static ssize_t ib_ucm_send_info(struct ib_ucm_file *file,
-				const char __user *inbuf, int in_len,
+				const char __user *inbuf, size_t in_len,
 				int (*func)(struct ib_cm_id *cm_id,
 					    int status,
 					    const void *info,

@@ -962,21 +962,21 @@ done:
 
 static ssize_t ib_ucm_send_rej(struct ib_ucm_file *file,
 			       const char __user *inbuf,
-			       int in_len, int out_len)
+			       size_t in_len, size_t out_len)
 {
 	return ib_ucm_send_info(file, inbuf, in_len, (void *)ib_send_cm_rej);
 }
 
 static ssize_t ib_ucm_send_apr(struct ib_ucm_file *file,
 			       const char __user *inbuf,
-			       int in_len, int out_len)
+			       size_t in_len, size_t out_len)
 {
 	return ib_ucm_send_info(file, inbuf, in_len, (void *)ib_send_cm_apr);
 }
 
 static ssize_t ib_ucm_send_mra(struct ib_ucm_file *file,
 			       const char __user *inbuf,
-			       int in_len, int out_len)
+			       size_t in_len, size_t out_len)
 {
 	struct ib_ucm_context *ctx;
 	struct ib_ucm_mra cmd;

@@ -1008,7 +1008,7 @@ static ssize_t ib_ucm_send_mra(struct ib_ucm_file *file,
 
 static ssize_t ib_ucm_send_lap(struct ib_ucm_file *file,
 			       const char __user *inbuf,
-			       int in_len, int out_len)
+			       size_t in_len, size_t out_len)
 {
 	struct ib_ucm_context *ctx;
 	struct ib_sa_path_rec *path = NULL;

@@ -1048,7 +1048,7 @@ done:
 
 static ssize_t ib_ucm_send_sidr_req(struct ib_ucm_file *file,
 				    const char __user *inbuf,
-				    int in_len, int out_len)
+				    size_t in_len, size_t out_len)
 {
 	struct ib_cm_sidr_req_param param;
 	struct ib_ucm_context *ctx;

@@ -1095,7 +1095,7 @@ done:
 
 static ssize_t ib_ucm_send_sidr_rep(struct ib_ucm_file *file,
 				    const char __user *inbuf,
-				    int in_len, int out_len)
+				    size_t in_len, size_t out_len)
 {
 	struct ib_cm_sidr_rep_param param;
 	struct ib_ucm_sidr_rep cmd;

@@ -1143,7 +1143,8 @@ done:
 
 static ssize_t (*ucm_cmd_table[])(struct ib_ucm_file *file,
 				  const char __user *inbuf,
-				  int in_len, int out_len) = {
+				  size_t in_len,
+				  size_t out_len) = {
 	[IB_USER_CM_CMD_CREATE_ID]     = ib_ucm_create_id,
 	[IB_USER_CM_CMD_DESTROY_ID]    = ib_ucm_destroy_id,
 	[IB_USER_CM_CMD_ATTR_ID]       = ib_ucm_attr_id,

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #11 lore raw

PATCH 10/22 ucm: ib_ucm_write(): check userspace buffer length first

In reply to a message not in the archive

Like uverbs, check the length parameter before the command type.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/infiniband/core/ucm.c +2 -2

index 8f8a0a8..49bf680 100644
@@ -1178,10 +1178,10 @@ static ssize_t ib_ucm_write(struct file *filp, const char __user *buf,
 	if (copy_from_user(&hdr, buf, sizeof(hdr)))
 		return -EFAULT;
 
-	if (hdr.cmd >= ARRAY_SIZE(ucm_cmd_table))
+	if (hdr.in + sizeof(hdr) > len)
 		return -EINVAL;
 
-	if (hdr.in + sizeof(hdr) > len)
+	if (hdr.cmd >= ARRAY_SIZE(ucm_cmd_table))
 		return -EINVAL;
 
 	result = ucm_cmd_table[hdr.cmd](file, buf + sizeof(hdr),

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #12 lore raw

PATCH 11/22 ucm: ib_ucm_write(): strict userspace buffer length check

In reply to a message not in the archive

Like uverbs, strictly check the length parameter:
this prevent ucm from processing truncated buffer.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

index 49bf680..81dc068 100644
@@ -1178,7 +1178,7 @@ static ssize_t ib_ucm_write(struct file *filp, const char __user *buf,
 	if (copy_from_user(&hdr, buf, sizeof(hdr)))
 		return -EFAULT;
 
-	if (hdr.in + sizeof(hdr) > len)
+	if (hdr.in + sizeof(hdr) != len)
 		return -EINVAL;
 
 	if (hdr.cmd >= ARRAY_SIZE(ucm_cmd_table))

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #13 lore raw

PATCH 12/22 ucma: check userspace input length

In reply to a message not in the archive

Makes ucma functions check the length of the input buffer before reading the command content: this will detect truncated command and will prevent ucma from reading past userspace provided buffer.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucma.c | 66 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

drivers/infiniband/core/ucma.c +66 -0

index b0f189b..9e9e899 100644
@@ -297,6 +297,9 @@ static ssize_t ucma_get_event(struct ucma_file *file, const char __user *inbuf,
 	struct ucma_event *uevent;
 	int ret = 0;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof uevent->resp)
 		return -ENOSPC;

@@ -374,6 +377,9 @@ static ssize_t ucma_create_id(struct ucma_file *file, const char __user *inbuf,
 	enum ib_qp_type qp_type;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(resp))
 		return -ENOSPC;

@@ -486,6 +492,9 @@ static ssize_t ucma_destroy_id(struct ucma_file *file, const char __user *inbuf,
 	struct ucma_context *ctx;
 	int ret = 0;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(resp))
 		return -ENOSPC;

@@ -519,6 +528,9 @@ static ssize_t ucma_bind_ip(struct ucma_file *file, const char __user *inbuf,
 	struct ucma_context *ctx;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -539,6 +551,9 @@ static ssize_t ucma_bind(struct ucma_file *file, const char __user *inbuf,
 	struct ucma_context *ctx;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -563,6 +578,9 @@ static ssize_t ucma_resolve_ip(struct ucma_file *file,
 	struct ucma_context *ctx;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -586,6 +604,9 @@ static ssize_t ucma_resolve_addr(struct ucma_file *file,
 	struct ucma_context *ctx;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -612,6 +633,9 @@ static ssize_t ucma_resolve_route(struct ucma_file *file,
 	struct ucma_context *ctx;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -708,6 +732,9 @@ static ssize_t ucma_query_route(struct ucma_file *file,
 	struct sockaddr *addr;
 	int ret = 0;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(resp))
 		return -ENOSPC;

@@ -889,6 +916,9 @@ static ssize_t ucma_query(struct ucma_file *file,
 	void __user *response;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -940,6 +970,9 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
 	struct ucma_context *ctx;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -963,6 +996,9 @@ static ssize_t ucma_listen(struct ucma_file *file, const char __user *inbuf,
 	struct ucma_context *ctx;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -985,6 +1021,9 @@ static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf,
 	struct ucma_context *ctx;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -1013,6 +1052,9 @@ static ssize_t ucma_reject(struct ucma_file *file, const char __user *inbuf,
 	struct ucma_context *ctx;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -1032,6 +1074,9 @@ static ssize_t ucma_disconnect(struct ucma_file *file, const char __user *inbuf,
 	struct ucma_context *ctx;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -1054,6 +1099,9 @@ static ssize_t ucma_init_qp_attr(struct ucma_file *file,
 	struct ib_qp_attr qp_attr;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(resp))
 		return -ENOSPC;

@@ -1187,6 +1235,9 @@ static ssize_t ucma_set_option(struct ucma_file *file, const char __user *inbuf,
 	void *optval;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -1217,6 +1268,9 @@ static ssize_t ucma_notify(struct ucma_file *file, const char __user *inbuf,
 	struct ucma_context *ctx;
 	int ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -1295,6 +1349,9 @@ static ssize_t ucma_join_ip_multicast(struct ucma_file *file,
 	struct rdma_ucm_join_ip_mcast cmd;
 	struct rdma_ucm_join_mcast join_cmd;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -1314,6 +1371,9 @@ static ssize_t ucma_join_multicast(struct ucma_file *file,
 {
 	struct rdma_ucm_join_mcast cmd;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

@@ -1329,6 +1389,9 @@ static ssize_t ucma_leave_multicast(struct ucma_file *file,
 	struct ucma_multicast *mc;
 	int ret = 0;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(resp))
 		return -ENOSPC;

@@ -1412,6 +1475,9 @@ static ssize_t ucma_migrate_id(struct ucma_file *new_file,
 	struct ucma_file *cur_file;
 	int ret = 0;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #14 lore raw

PATCH 13/22 ucma: ucma_migrate_id(): check output length

In reply to a message not in the archive

Makes ucma_migrate_id() check the length of the output buffer:
this will prevent the function from writing past userspace provided buffer.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucma.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/infiniband/core/ucma.c +3 -0

index 9e9e899..a155169 100644
@@ -1478,6 +1478,9 @@ static ssize_t ucma_migrate_id(struct ucma_file *new_file,
 	if (in_len < sizeof(cmd))
 		return -EINVAL;
 
+	if (out_len < sizeof(resp))
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
 		return -EFAULT;

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #15 lore raw

PATCH 14/22 ucma: ucma_query_path(): check output length

In reply to a message not in the archive

Makes ucma_query_path() check the length of the output buffer:
this will prevent the function from writing past userspace provided buffer.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucma.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/infiniband/core/ucma.c +3 -0

index a155169..c653b1e 100644
@@ -838,6 +838,9 @@ static ssize_t ucma_query_path(struct ucma_context *ctx,
 	if (out_len < sizeof(*resp))
 		return -ENOSPC;
 
+	if ((out_len - sizeof(*resp))/sizeof(struct ib_path_rec_data) < ctx->cm_id->route.num_paths)
+		return -ENOSPC;
+
 	resp = kzalloc(out_len, GFP_KERNEL);
 	if (!resp)
 		return -ENOMEM;

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #16 lore raw

PATCH 15/22 ucma: use size_t for userspace buffer input and output length

In reply to a message not in the archive

There's no reason to use a signed type for userspace buffer length.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucma.c | 55 +++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 27 deletions(-)

drivers/infiniband/core/ucma.c +28 ~~-27~~

index c653b1e..ecc6fad 100644
@@ -290,7 +290,7 @@ out:
 }
 
 static ssize_t ucma_get_event(struct ucma_file *file, const char __user *inbuf,
-			      int in_len, int out_len)
+			      size_t in_len, size_t out_len)
 {
 	struct ucma_context *ctx;
 	struct rdma_ucm_get_event cmd;

@@ -369,7 +369,7 @@ static int ucma_get_qp_type(struct rdma_ucm_create_id *cmd, enum ib_qp_type *qp_
 }
 
 static ssize_t ucma_create_id(struct ucma_file *file, const char __user *inbuf,
-			      int in_len, int out_len)
+			      size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_create_id cmd;
 	struct rdma_ucm_create_id_resp resp;

@@ -485,7 +485,7 @@ static int ucma_free_ctx(struct ucma_context *ctx)
 }
 
 static ssize_t ucma_destroy_id(struct ucma_file *file, const char __user *inbuf,
-			       int in_len, int out_len)
+			       size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_destroy_id cmd;
 	struct rdma_ucm_destroy_id_resp resp;

@@ -522,7 +522,7 @@ static ssize_t ucma_destroy_id(struct ucma_file *file, const char __user *inbuf,
 }
 
 static ssize_t ucma_bind_ip(struct ucma_file *file, const char __user *inbuf,
-			      int in_len, int out_len)
+			      size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_bind_ip cmd;
 	struct ucma_context *ctx;

@@ -544,7 +544,7 @@ static ssize_t ucma_bind_ip(struct ucma_file *file, const char __user *inbuf,
 }
 
 static ssize_t ucma_bind(struct ucma_file *file, const char __user *inbuf,
-			 int in_len, int out_len)
+			 size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_bind cmd;
 	struct sockaddr *addr;

@@ -572,7 +572,7 @@ static ssize_t ucma_bind(struct ucma_file *file, const char __user *inbuf,
 
 static ssize_t ucma_resolve_ip(struct ucma_file *file,
 			       const char __user *inbuf,
-			       int in_len, int out_len)
+			       size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_resolve_ip cmd;
 	struct ucma_context *ctx;

@@ -597,7 +597,7 @@ static ssize_t ucma_resolve_ip(struct ucma_file *file,
 
 static ssize_t ucma_resolve_addr(struct ucma_file *file,
 				 const char __user *inbuf,
-				 int in_len, int out_len)
+				 size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_resolve_addr cmd;
 	struct sockaddr *src, *dst;

@@ -627,7 +627,7 @@ static ssize_t ucma_resolve_addr(struct ucma_file *file,
 
 static ssize_t ucma_resolve_route(struct ucma_file *file,
 				  const char __user *inbuf,
-				  int in_len, int out_len)
+				  size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_resolve_route cmd;
 	struct ucma_context *ctx;

@@ -724,7 +724,7 @@ static void ucma_copy_iw_route(struct rdma_ucm_query_route_resp *resp,
 
 static ssize_t ucma_query_route(struct ucma_file *file,
 				const char __user *inbuf,
-				int in_len, int out_len)
+				size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_query cmd;
 	struct rdma_ucm_query_route_resp resp;

@@ -802,7 +802,7 @@ static void ucma_query_device_addr(struct rdma_cm_id *cm_id,
 }
 
 static ssize_t ucma_query_addr(struct ucma_context *ctx,
-			       void __user *response, int out_len)
+			       void __user *response, size_t out_len)
 {
 	struct rdma_ucm_query_addr_resp resp;
 	struct sockaddr *addr;

@@ -830,7 +830,7 @@ static ssize_t ucma_query_addr(struct ucma_context *ctx,
 }
 
 static ssize_t ucma_query_path(struct ucma_context *ctx,
-			       void __user *response, int out_len)
+			       void __user *response, size_t out_len)
 {
 	struct rdma_ucm_query_path_resp *resp;
 	int i, ret = 0;

@@ -865,7 +865,7 @@ static ssize_t ucma_query_path(struct ucma_context *ctx,
 }
 
 static ssize_t ucma_query_gid(struct ucma_context *ctx,
-			      void __user *response, int out_len)
+			      void __user *response, size_t out_len)
 {
 	struct rdma_ucm_query_addr_resp resp;
 	struct sockaddr_ib *addr;

@@ -912,7 +912,7 @@ static ssize_t ucma_query_gid(struct ucma_context *ctx,
 
 static ssize_t ucma_query(struct ucma_file *file,
 			  const char __user *inbuf,
-			  int in_len, int out_len)
+			  size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_query cmd;
 	struct ucma_context *ctx;

@@ -966,7 +966,7 @@ static void ucma_copy_conn_param(struct rdma_cm_id *id,
 }
 
 static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
-			    int in_len, int out_len)
+			    size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_connect cmd;
 	struct rdma_conn_param conn_param;

@@ -993,7 +993,7 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
 }
 
 static ssize_t ucma_listen(struct ucma_file *file, const char __user *inbuf,
-			   int in_len, int out_len)
+			   size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_listen cmd;
 	struct ucma_context *ctx;

@@ -1017,7 +1017,7 @@ static ssize_t ucma_listen(struct ucma_file *file, const char __user *inbuf,
 }
 
 static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf,
-			   int in_len, int out_len)
+			   size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_accept cmd;
 	struct rdma_conn_param conn_param;

@@ -1049,7 +1049,7 @@ static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf,
 }
 
 static ssize_t ucma_reject(struct ucma_file *file, const char __user *inbuf,
-			   int in_len, int out_len)
+			   size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_reject cmd;
 	struct ucma_context *ctx;

@@ -1071,7 +1071,7 @@ static ssize_t ucma_reject(struct ucma_file *file, const char __user *inbuf,
 }
 
 static ssize_t ucma_disconnect(struct ucma_file *file, const char __user *inbuf,
-			       int in_len, int out_len)
+			       size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_disconnect cmd;
 	struct ucma_context *ctx;

@@ -1094,7 +1094,7 @@ static ssize_t ucma_disconnect(struct ucma_file *file, const char __user *inbuf,
 
 static ssize_t ucma_init_qp_attr(struct ucma_file *file,
 				 const char __user *inbuf,
-				 int in_len, int out_len)
+				 size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_init_qp_attr cmd;
 	struct ib_uverbs_qp_attr resp;

@@ -1231,7 +1231,7 @@ static int ucma_set_option_level(struct ucma_context *ctx, int level,
 }
 
 static ssize_t ucma_set_option(struct ucma_file *file, const char __user *inbuf,
-			       int in_len, int out_len)
+			       size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_set_option cmd;
 	struct ucma_context *ctx;

@@ -1265,7 +1265,7 @@ out:
 }
 
 static ssize_t ucma_notify(struct ucma_file *file, const char __user *inbuf,
-			   int in_len, int out_len)
+			   size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_notify cmd;
 	struct ucma_context *ctx;

@@ -1287,7 +1287,7 @@ static ssize_t ucma_notify(struct ucma_file *file, const char __user *inbuf,
 }
 
 static ssize_t ucma_process_join(struct ucma_file *file,
-				 struct rdma_ucm_join_mcast *cmd,  int out_len)
+				 struct rdma_ucm_join_mcast *cmd, size_t out_len)
 {
 	struct rdma_ucm_create_id_resp resp;
 	struct ucma_context *ctx;

@@ -1347,7 +1347,7 @@ err1:
 
 static ssize_t ucma_join_ip_multicast(struct ucma_file *file,
 				      const char __user *inbuf,
-				      int in_len, int out_len)
+				      size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_join_ip_mcast cmd;
 	struct rdma_ucm_join_mcast join_cmd;

@@ -1370,7 +1370,7 @@ static ssize_t ucma_join_ip_multicast(struct ucma_file *file,
 
 static ssize_t ucma_join_multicast(struct ucma_file *file,
 				   const char __user *inbuf,
-				   int in_len, int out_len)
+				   size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_join_mcast cmd;

@@ -1385,7 +1385,7 @@ static ssize_t ucma_join_multicast(struct ucma_file *file,
 
 static ssize_t ucma_leave_multicast(struct ucma_file *file,
 				    const char __user *inbuf,
-				    int in_len, int out_len)
+				    size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_destroy_id cmd;
 	struct rdma_ucm_destroy_id_resp resp;

@@ -1469,7 +1469,7 @@ static void ucma_move_events(struct ucma_context *ctx, struct ucma_file *file)
 
 static ssize_t ucma_migrate_id(struct ucma_file *new_file,
 			       const char __user *inbuf,
-			       int in_len, int out_len)
+			       size_t in_len, size_t out_len)
 {
 	struct rdma_ucm_migrate_id cmd;
 	struct rdma_ucm_migrate_resp resp;

@@ -1533,7 +1533,8 @@ file_put:
 
 static ssize_t (*ucma_cmd_table[])(struct ucma_file *file,
 				   const char __user *inbuf,
-				   int in_len, int out_len) = {
+				   size_t in_len, size_t out_len) = {
+
 	[RDMA_USER_CM_CMD_CREATE_ID] 	 = ucma_create_id,
 	[RDMA_USER_CM_CMD_DESTROY_ID]	 = ucma_destroy_id,
 	[RDMA_USER_CM_CMD_BIND_IP]	 = ucma_bind_ip,

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #17 lore raw

PATCH 16/22 ucma: ucma_write(): check userspace buffer length first

In reply to a message not in the archive

Like uverbs, check the length parameter before the command type.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/infiniband/core/ucma.c +2 -2

index ecc6fad..92f2248 100644
@@ -1573,10 +1573,10 @@ static ssize_t ucma_write(struct file *filp, const char __user *buf,
 	if (copy_from_user(&hdr, buf, sizeof(hdr)))
 		return -EFAULT;
 
-	if (hdr.cmd >= ARRAY_SIZE(ucma_cmd_table))
+	if (hdr.in + sizeof(hdr) > len)
 		return -EINVAL;
 
-	if (hdr.in + sizeof(hdr) > len)
+	if (hdr.cmd >= ARRAY_SIZE(ucma_cmd_table))
 		return -EINVAL;
 
 	if (!ucma_cmd_table[hdr.cmd])

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #18 lore raw

PATCH 17/22 ucma: ucma_write(): strict userspace buffer length check

In reply to a message not in the archive

Like uverbs, strictly check the length parameter:
this prevent ucma from processing truncated buffer.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/ucma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

index 92f2248..502e2a2 100644
@@ -1573,7 +1573,7 @@ static ssize_t ucma_write(struct file *filp, const char __user *buf,
 	if (copy_from_user(&hdr, buf, sizeof(hdr)))
 		return -EFAULT;
 
-	if (hdr.in + sizeof(hdr) > len)
+	if (hdr.in + sizeof(hdr) != len)
 		return -EINVAL;
 
 	if (hdr.cmd >= ARRAY_SIZE(ucma_cmd_table))

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #19 lore raw

PATCH 18/22 uverbs: check input length

In reply to a message not in the archive

Makes verbs functions check the length of the input buffer before reading the command content: this will detect truncated command and will prevent uverbs from reading past userspace provided buffer.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/uverbs_cmd.c | 102 +++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

drivers/infiniband/core/uverbs_cmd.c +102 -0

index 6ca3c86..d0f9854 100644
@@ -298,6 +298,9 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
 	struct file			 *filp;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -392,6 +395,9 @@ ssize_t ib_uverbs_query_device(struct ib_uverbs_file *file,
 	struct ib_device_attr              attr;
 	int                                ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -461,6 +467,9 @@ ssize_t ib_uverbs_query_port(struct ib_uverbs_file *file,
 	struct ib_port_attr              attr;
 	int                              ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -513,6 +522,9 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
 	struct ib_pd                  *pd;
 	int                            ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -584,6 +596,9 @@ ssize_t ib_uverbs_dealloc_pd(struct ib_uverbs_file *file,
 	struct ib_uobject          *uobj;
 	int                         ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -710,6 +725,9 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file,
 	int				ret = 0;
 	int				new_xrcd = 0;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -845,6 +863,9 @@ ssize_t ib_uverbs_close_xrcd(struct ib_uverbs_file *file,
 	int                         live;
 	int                         ret = 0;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -922,6 +943,9 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
 	struct ib_mr                *mr;
 	int                          ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -1020,6 +1044,9 @@ ssize_t ib_uverbs_dereg_mr(struct ib_uverbs_file *file,
 	struct ib_uobject	 *uobj;
 	int                       ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -1060,6 +1087,9 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file,
 	struct ib_mw                  *mw;
 	int                            ret;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (out_len < sizeof(resp))
 		return -ENOSPC;

@@ -1140,6 +1170,9 @@ ssize_t ib_uverbs_dealloc_mw(struct ib_uverbs_file *file,
 	struct ib_uobject	   *uobj;
 	int                         ret = -EINVAL;
 
+	if (in_len < sizeof(cmd))
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof(cmd)))
 		return -EFAULT;

@@ -1178,6 +1211,9 @@ ssize_t ib_uverbs_create_comp_channel(struct ib_uverbs_file *file,
 	struct file				  *filp;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -1218,6 +1254,9 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
 	struct ib_cq                   *cq;
 	int                             ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -1317,6 +1356,9 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
 	struct ib_cq			*cq;
 	int				ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -1382,6 +1424,9 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 	struct ib_wc                   wc;
 	int                            ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -1428,6 +1473,9 @@ ssize_t ib_uverbs_req_notify_cq(struct ib_uverbs_file *file,
 	struct ib_uverbs_req_notify_cq cmd;
 	struct ib_cq                  *cq;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -1455,6 +1503,9 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
 	struct ib_uverbs_event_file	*ev_file;
 	int                        	 ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -1513,6 +1564,9 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
 	struct ib_qp_init_attr          attr;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -1703,6 +1757,9 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
 	struct ib_qp_open_attr          attr;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -1795,6 +1852,9 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 	struct ib_qp_init_attr         *init_attr;
 	int                            ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -1908,6 +1968,9 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
 	struct ib_qp_attr         *attr;
 	int                        ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2001,6 +2064,9 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
 	struct ib_uqp_object        	*obj;
 	int                        	 ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2061,6 +2127,9 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file,
 	int				is_ud;
 	ssize_t                         ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2299,6 +2368,9 @@ ssize_t ib_uverbs_post_recv(struct ib_uverbs_file *file,
 	struct ib_qp                   *qp;
 	ssize_t                         ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2348,6 +2420,9 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file,
 	struct ib_srq                      *srq;
 	ssize_t                             ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2399,6 +2474,9 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
 	struct ib_ah_attr		attr;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -2485,6 +2563,9 @@ ssize_t ib_uverbs_destroy_ah(struct ib_uverbs_file *file,
 	struct ib_uobject	   *uobj;
 	int			    ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2523,6 +2604,9 @@ ssize_t ib_uverbs_attach_mcast(struct ib_uverbs_file *file,
 	struct ib_uverbs_mcast_entry *mcast;
 	int                           ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2570,6 +2654,9 @@ ssize_t ib_uverbs_detach_mcast(struct ib_uverbs_file *file,
 	struct ib_uverbs_mcast_entry *mcast;
 	int                           ret = -EINVAL;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2740,6 +2827,9 @@ ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file,
 	struct ib_udata                  udata;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -2773,6 +2863,9 @@ ssize_t ib_uverbs_create_xsrq(struct ib_uverbs_file *file,
 	struct ib_udata                  udata;
 	int ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -2800,6 +2893,9 @@ ssize_t ib_uverbs_modify_srq(struct ib_uverbs_file *file,
 	struct ib_srq_attr          attr;
 	int                         ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2830,6 +2926,9 @@ ssize_t ib_uverbs_query_srq(struct ib_uverbs_file *file,
 	struct ib_srq                   *srq;
 	int                             ret;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (out_len < sizeof resp)
 		return -ENOSPC;

@@ -2873,6 +2972,9 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
 	struct ib_usrq_object		 *us;
 	enum ib_srq_type		  srq_type;
 
+	if (in_len < sizeof cmd)
+		return -EINVAL;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #20 lore raw

PATCH 19/22 uverbs: check output length

In reply to a message not in the archive

Makes uverbs functions check the length of the output buffer:
this will prevent uverbs from writing past userspace provided buffer.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/uverbs_cmd.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

drivers/infiniband/core/uverbs_cmd.c +27 -0

index d0f9854..dc0574f 100644
@@ -1359,6 +1359,9 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -1427,6 +1430,9 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -1506,6 +1512,9 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -1855,6 +1864,9 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2067,6 +2079,9 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2130,6 +2145,9 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2371,6 +2389,9 @@ ssize_t ib_uverbs_post_recv(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2423,6 +2444,9 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

@@ -2975,6 +2999,9 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
 	if (in_len < sizeof cmd)
 		return -EINVAL;
 
+	if (out_len < sizeof resp)
+		return -ENOSPC;
+
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #21 lore raw

PATCH 20/22 uverbs: ib_uverbs_poll_cq(): check output length against number of wc

In reply to a message not in the archive

Makes ib_uverbs_poll_cq() check the length of the output buffer:
this will prevent the function from writing past userspace provided buffer.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/uverbs_cmd.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/infiniband/core/uverbs_cmd.c +3 -0

index dc0574f..5981604 100644
@@ -1436,6 +1436,9 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
+	if ((out_len - sizeof resp)/(sizeof(struct ib_uverbs_wc)) < cmd.ne)
+		return -ENOSPC;
+
 	cq = idr_read_cq(cmd.cq_handle, file->ucontext, 0);
 	if (!cq)
 		return -EINVAL;

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #22 lore raw

PATCH 21/22 uverbs: fix call to INIT_UDATA with no output buffer

In reply to a message not in the archive

When using INIT_UDATA() with an output buffer set to NULL, there's no space available, so output buffer length should be set to 0.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/uverbs_cmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/infiniband/core/uverbs_cmd.c +2 -2

index 5981604..df50e81 100644
@@ -1990,7 +1990,7 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
 		return -EFAULT;
 
 	INIT_UDATA(&udata, buf + sizeof cmd, NULL, in_len - sizeof cmd,
-		   out_len);
+		   0);
 
 	attr = kmalloc(sizeof *attr, GFP_KERNEL);
 	if (!attr)

@@ -2927,7 +2927,7 @@ ssize_t ib_uverbs_modify_srq(struct ib_uverbs_file *file,
 		return -EFAULT;
 
 	INIT_UDATA(&udata, buf + sizeof cmd, NULL, in_len - sizeof cmd,
-		   out_len);
+		   0);
 
 	srq = idr_read_srq(cmd.srq_handle, file->ucontext);
 	if (!srq)

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yann Droneaud

author

2013-08-18 18:28 UTC #23 lore raw

PATCH 22/22 uverbs: use size_t for userspace buffer input and output length

In reply to a message not in the archive

There's no reason to use a signed type for userspace buffer length.

Signed-off-by Yann Droneaud <[email protected]>

Link http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

---
 drivers/infiniband/core/uverbs.h      |   4 +-
 drivers/infiniband/core/uverbs_cmd.c  | 120 +++++++++++++++++-----------------
 drivers/infiniband/core/uverbs_main.c |   4 +-
 3 files changed, 64 insertions(+), 64 deletions(-)

drivers/infiniband/core/uverbs.h +2 -2

index b8431d6..9286fca 100644
@@ -179,8 +179,8 @@ void ib_uverbs_dealloc_xrcd(struct ib_uverbs_device *dev, struct ib_xrcd *xrcd);
 
 #define IB_UVERBS_DECLARE_CMD(name)					\
 	ssize_t ib_uverbs_##name(struct ib_uverbs_file *file,		\
-				 const char __user *buf, int in_len,	\
-				 int out_len)
+				 const char __user *buf, size_t in_len,	\
+				 size_t out_len)
 
 IB_UVERBS_DECLARE_CMD(get_context);
 IB_UVERBS_DECLARE_CMD(query_device);

drivers/infiniband/core/uverbs_cmd.c +60 ~~-60~~

330 lines not on this page · raw message

drivers/infiniband/core/uverbs_main.c +2 -2

index 2c6f0f2..05557da 100644
@@ -78,8 +78,8 @@ static DEFINE_SPINLOCK(map_lock);
 static DECLARE_BITMAP(dev_map, IB_UVERBS_MAX_DEVICES);
 
 static ssize_t (*uverbs_cmd_table[])(struct ib_uverbs_file *file,
-				     const char __user *buf, int in_len,
-				     int out_len) = {
+				     const char __user *buf, size_t in_len,
+				     size_t out_len) = {
 	[IB_USER_VERBS_CMD_GET_CONTEXT]		= ib_uverbs_get_context,
 	[IB_USER_VERBS_CMD_QUERY_DEVICE]	= ib_uverbs_query_device,
 	[IB_USER_VERBS_CMD_QUERY_PORT]		= ib_uverbs_query_port,

Signature · 6 lines

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html