[PATCH 00/10] IB/core: more fixes for create_flow uverbs (for v3.12)

[PATCH 00/10] IB/core: more fixes for create_flow uverbs (for v3.12)

[Yann Droneaud]

Hi,

I've suggested reverting temporarily commit 22878db, 436f2ad, 400dbc9
for v3.12 in a previous message [1]. This proposition doesn't sound
acceptable for Or.

So I continue to improve them to be in a better shape for v3.12.

Please find some important changes against create_flow uverbs
implementation.

The most notable changes that should go to v3.12:
- structure renaming to match others uverbs public structs;
- changes usage of the flow_attr.size to not count the
  "extended command header" but to describe only the size
  of the flow specs following flow_attr;
- removed unneeded flow_spec structure that don't need to be
  exposed to userspace.
- ensure 64bits alignement (regarding patchset [2])

Others changes are mostly improving the uverbs function to protect
from malformed flow_spec. Those could be added later (v3.13)
but I haven't ordered the patches this way: all of this is a bit
developped in a hurry to catch v3.12.

Please review and tell me which patchset should go first
to rebase one on the other: this one, or the "improved infrastructure"
patchset[2].

Links:

[1]: http://marc.info/?i=0d695256ac989e7b1bcccd3a2bfafcbf-zgzEX58YAwA@public.gmane.org
     http://mid.gmane.org/0d695256ac989e7b1bcccd3a2bfafcbf-zgzEX58YAwA@public.gmane.org

[2]: http://marc.info/?i=cover.1381177342.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
     http://mid.gmane.org/cover.1381177342.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

Regards.

Yann Droneaud (10):
  IB/core: Rename 'flow' structs to match other uverbs structs
  IB/core: Makes uverbs flow structure using names more similar to verbs
    ones
  IB/core: Don't include command size in uverbs create_flow
  IB/core: Allocate memory only for flow_spec in create_flow uverbs
  IB/core: Remove checks for negative values in create_flow uverbs
  IB/core: Check against uverbs structure size while parsing flow_spec
  IB/core: Uses a common header for uverbs flow_specs
  IB/core: Use flow_spec common header in uverbs flow_spec conversion
    function
  IB/core: Remove ib_uverbs_flow_spec structure from userspace
  IB/core: Add proper bound checking in flow_spec uverbs conversion to
    verbs

 drivers/infiniband/core/uverbs.h     |  16 +++++
 drivers/infiniband/core/uverbs_cmd.c | 136 +++++++++++++++++++++--------------
 include/uapi/rdma/ib_user_verbs.h    |  69 ++++++++++--------
 3 files changed, 139 insertions(+), 82 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 01/10] IB/core: Rename 'flow' structs to match other uverbs structs

[Yann Droneaud]

Commit 436f2ad added public data structures to support
receive flow steering. The new structs are not following
the 'uverbs' pattern: they're lacking the common prefix
'ib_uverbs'.

This patch replaces ib_kern prefix by ib_uverbs.

Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Link: http://mid.gmane.org/cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
 drivers/infiniband/core/uverbs_cmd.c | 76 ++++++++++++++++++------------------
 include/uapi/rdma/ib_user_verbs.h    | 36 ++++++++---------
 2 files changed, 56 insertions(+), 56 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index f2b81b9..e16e22e 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2599,38 +2599,38 @@ out_put:
 	return ret ? ret : in_len;
 }
 
-static int kern_spec_to_ib_spec(struct ib_kern_spec *kern_spec,
+static int uverbs_spec_to_ib_spec(struct ib_uverbs_spec *uverbs_spec,
 				union ib_flow_spec *ib_spec)
 {
-	ib_spec->type = kern_spec->type;
+	ib_spec->type = uverbs_spec->type;
 
 	switch (ib_spec->type) {
 	case IB_FLOW_SPEC_ETH:
 		ib_spec->eth.size = sizeof(struct ib_flow_spec_eth);
-		if (ib_spec->eth.size != kern_spec->eth.size)
+		if (ib_spec->eth.size != uverbs_spec->eth.size)
 			return -EINVAL;
-		memcpy(&ib_spec->eth.val, &kern_spec->eth.val,
+		memcpy(&ib_spec->eth.val, &uverbs_spec->eth.val,
 		       sizeof(struct ib_flow_eth_filter));
-		memcpy(&ib_spec->eth.mask, &kern_spec->eth.mask,
+		memcpy(&ib_spec->eth.mask, &uverbs_spec->eth.mask,
 		       sizeof(struct ib_flow_eth_filter));
 		break;
 	case IB_FLOW_SPEC_IPV4:
 		ib_spec->ipv4.size = sizeof(struct ib_flow_spec_ipv4);
-		if (ib_spec->ipv4.size != kern_spec->ipv4.size)
+		if (ib_spec->ipv4.size != uverbs_spec->ipv4.size)
 			return -EINVAL;
-		memcpy(&ib_spec->ipv4.val, &kern_spec->ipv4.val,
+		memcpy(&ib_spec->ipv4.val, &uverbs_spec->ipv4.val,
 		       sizeof(struct ib_flow_ipv4_filter));
-		memcpy(&ib_spec->ipv4.mask, &kern_spec->ipv4.mask,
+		memcpy(&ib_spec->ipv4.mask, &uverbs_spec->ipv4.mask,
 		       sizeof(struct ib_flow_ipv4_filter));
 		break;
 	case IB_FLOW_SPEC_TCP:
 	case IB_FLOW_SPEC_UDP:
 		ib_spec->tcp_udp.size = sizeof(struct ib_flow_spec_tcp_udp);
-		if (ib_spec->tcp_udp.size != kern_spec->tcp_udp.size)
+		if (ib_spec->tcp_udp.size != uverbs_spec->tcp_udp.size)
 			return -EINVAL;
-		memcpy(&ib_spec->tcp_udp.val, &kern_spec->tcp_udp.val,
+		memcpy(&ib_spec->tcp_udp.val, &uverbs_spec->tcp_udp.val,
 		       sizeof(struct ib_flow_tcp_udp_filter));
-		memcpy(&ib_spec->tcp_udp.mask, &kern_spec->tcp_udp.mask,
+		memcpy(&ib_spec->tcp_udp.mask, &uverbs_spec->tcp_udp.mask,
 		       sizeof(struct ib_flow_tcp_udp_filter));
 		break;
 	default:
@@ -2647,14 +2647,14 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 	struct ib_uverbs_create_flow_resp resp;
 	struct ib_uobject		  *uobj;
 	struct ib_flow			  *flow_id;
-	struct ib_kern_flow_attr	  *kern_flow_attr;
+	struct ib_uverbs_flow_attr	  *uverbs_flow_attr;
 	struct ib_flow_attr		  *flow_attr;
 	struct ib_qp			  *qp;
 	int err = 0;
-	void *kern_spec;
+	void *uverbs_spec;
 	void *ib_spec;
 	int i;
-	int kern_attr_size;
+	int uverbs_attr_size;
 
 	if (out_len < sizeof(resp))
 		return -ENOSPC;
@@ -2673,28 +2673,28 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 	    cmd.flow_attr.num_of_specs > IB_FLOW_SPEC_SUPPORT_LAYERS)
 		return -EINVAL;
 
-	kern_attr_size = cmd.flow_attr.size - sizeof(cmd) -
+	uverbs_attr_size = cmd.flow_attr.size - sizeof(cmd) -
 			 sizeof(struct ib_uverbs_cmd_hdr_ex);
 
 	if (cmd.flow_attr.size < 0 || cmd.flow_attr.size > in_len ||
-	    kern_attr_size < 0 || kern_attr_size >
-	    (cmd.flow_attr.num_of_specs * sizeof(struct ib_kern_spec)))
+	    uverbs_attr_size < 0 || uverbs_attr_size >
+	    (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_spec)))
 		return -EINVAL;
 
 	if (cmd.flow_attr.num_of_specs) {
-		kern_flow_attr = kmalloc(cmd.flow_attr.size, GFP_KERNEL);
-		if (!kern_flow_attr)
+		uverbs_flow_attr = kmalloc(cmd.flow_attr.size, GFP_KERNEL);
+		if (!uverbs_flow_attr)
 			return -ENOMEM;
 
-		memcpy(kern_flow_attr, &cmd.flow_attr, sizeof(*kern_flow_attr));
-		if (copy_from_user(kern_flow_attr + 1, buf + sizeof(cmd),
-				   kern_attr_size)) {
+		memcpy(uverbs_flow_attr, &cmd.flow_attr, sizeof(*uverbs_flow_attr));
+		if (copy_from_user(uverbs_flow_attr + 1, buf + sizeof(cmd),
+				   uverbs_attr_size)) {
 			err = -EFAULT;
 			goto err_free_attr;
 		}
 	} else {
-		kern_flow_attr = &cmd.flow_attr;
-		kern_attr_size = sizeof(cmd.flow_attr);
+		uverbs_flow_attr = &cmd.flow_attr;
+		uverbs_attr_size = sizeof(cmd.flow_attr);
 	}
 
 	uobj = kmalloc(sizeof(*uobj), GFP_KERNEL);
@@ -2717,28 +2717,28 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 		goto err_put;
 	}
 
-	flow_attr->type = kern_flow_attr->type;
-	flow_attr->priority = kern_flow_attr->priority;
-	flow_attr->num_of_specs = kern_flow_attr->num_of_specs;
-	flow_attr->port = kern_flow_attr->port;
-	flow_attr->flags = kern_flow_attr->flags;
+	flow_attr->type = uverbs_flow_attr->type;
+	flow_attr->priority = uverbs_flow_attr->priority;
+	flow_attr->num_of_specs = uverbs_flow_attr->num_of_specs;
+	flow_attr->port = uverbs_flow_attr->port;
+	flow_attr->flags = uverbs_flow_attr->flags;
 	flow_attr->size = sizeof(*flow_attr);
 
-	kern_spec = kern_flow_attr + 1;
+	uverbs_spec = uverbs_flow_attr + 1;
 	ib_spec = flow_attr + 1;
-	for (i = 0; i < flow_attr->num_of_specs && kern_attr_size > 0; i++) {
-		err = kern_spec_to_ib_spec(kern_spec, ib_spec);
+	for (i = 0; i < flow_attr->num_of_specs && uverbs_attr_size > 0; i++) {
+		err = uverbs_spec_to_ib_spec(uverbs_spec, ib_spec);
 		if (err)
 			goto err_free;
 		flow_attr->size +=
 			((union ib_flow_spec *) ib_spec)->size;
-		kern_attr_size -= ((struct ib_kern_spec *) kern_spec)->size;
-		kern_spec += ((struct ib_kern_spec *) kern_spec)->size;
+		uverbs_attr_size -= ((struct ib_uverbs_spec *) uverbs_spec)->size;
+		uverbs_spec += ((struct ib_uverbs_spec *) uverbs_spec)->size;
 		ib_spec += ((union ib_flow_spec *) ib_spec)->size;
 	}
-	if (kern_attr_size) {
+	if (uverbs_attr_size) {
 		pr_warn("create flow failed, %d bytes left from uverb cmd\n",
-			kern_attr_size);
+			uverbs_attr_size);
 		goto err_free;
 	}
 	flow_id = ib_create_flow(qp, flow_attr, IB_FLOW_DOMAIN_USER);
@@ -2773,7 +2773,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 	up_write(&uobj->mutex);
 	kfree(flow_attr);
 	if (cmd.flow_attr.num_of_specs)
-		kfree(kern_flow_attr);
+		kfree(uverbs_flow_attr);
 	return in_len;
 err_copy:
 	idr_remove_uobj(&ib_uverbs_rule_idr, uobj);
@@ -2787,7 +2787,7 @@ err_uobj:
 	put_uobj_write(uobj);
 err_free_attr:
 	if (cmd.flow_attr.num_of_specs)
-		kfree(kern_flow_attr);
+		kfree(uverbs_flow_attr);
 	return err;
 }
 
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index 0b233c5..f8fea78 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -696,61 +696,61 @@ struct ib_uverbs_detach_mcast {
 	__u64 driver_data[0];
 };
 
-struct ib_kern_eth_filter {
+struct ib_uverbs_eth_filter {
 	__u8  dst_mac[6];
 	__u8  src_mac[6];
 	__be16 ether_type;
 	__be16 vlan_tag;
 };
 
-struct ib_kern_spec_eth {
+struct ib_uverbs_spec_eth {
 	__u32  type;
 	__u16  size;
 	__u16  reserved;
-	struct ib_kern_eth_filter val;
-	struct ib_kern_eth_filter mask;
+	struct ib_uverbs_eth_filter val;
+	struct ib_uverbs_eth_filter mask;
 };
 
-struct ib_kern_ipv4_filter {
+struct ib_uverbs_ipv4_filter {
 	__be32 src_ip;
 	__be32 dst_ip;
 };
 
-struct ib_kern_spec_ipv4 {
+struct ib_uverbs_spec_ipv4 {
 	__u32  type;
 	__u16  size;
 	__u16  reserved;
-	struct ib_kern_ipv4_filter val;
-	struct ib_kern_ipv4_filter mask;
+	struct ib_uverbs_ipv4_filter val;
+	struct ib_uverbs_ipv4_filter mask;
 };
 
-struct ib_kern_tcp_udp_filter {
+struct ib_uverbs_tcp_udp_filter {
 	__be16 dst_port;
 	__be16 src_port;
 };
 
-struct ib_kern_spec_tcp_udp {
+struct ib_uverbs_spec_tcp_udp {
 	__u32  type;
 	__u16  size;
 	__u16  reserved;
-	struct ib_kern_tcp_udp_filter val;
-	struct ib_kern_tcp_udp_filter mask;
+	struct ib_uverbs_tcp_udp_filter val;
+	struct ib_uverbs_tcp_udp_filter mask;
 };
 
-struct ib_kern_spec {
+struct ib_uverbs_spec {
 	union {
 		struct {
 			__u32 type;
 			__u16 size;
 			__u16 reserved;
 		};
-		struct ib_kern_spec_eth	    eth;
-		struct ib_kern_spec_ipv4    ipv4;
-		struct ib_kern_spec_tcp_udp tcp_udp;
+		struct ib_uverbs_spec_eth	    eth;
+		struct ib_uverbs_spec_ipv4    ipv4;
+		struct ib_uverbs_spec_tcp_udp tcp_udp;
 	};
 };
 
-struct ib_kern_flow_attr {
+struct ib_uverbs_flow_attr {
 	__u32 type;
 	__u16 size;
 	__u16 priority;
@@ -768,7 +768,7 @@ struct ib_uverbs_create_flow  {
 	__u32 comp_mask;
 	__u64 response;
 	__u32 qp_handle;
-	struct ib_kern_flow_attr flow_attr;
+	struct ib_uverbs_flow_attr flow_attr;
 };
 
 struct ib_uverbs_create_flow_resp {
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/10] IB/core: Makes uverbs flow structure using names more similar to verbs ones

[Yann Droneaud]

This patch add "flow" prefix to most of data structure added as part
of commit 436f2ad to keep those names in sync with the data structures
added in commit 319a441.

It's just a matter of translating 'ib_flow' to 'ib_uverbs_flow'.

Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Link: http://mid.gmane.org/cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
 drivers/infiniband/core/uverbs_cmd.c |  8 ++++----
 include/uapi/rdma/ib_user_verbs.h    | 32 ++++++++++++++++----------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index e16e22e..a4e387d 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2599,7 +2599,7 @@ out_put:
 	return ret ? ret : in_len;
 }
 
-static int uverbs_spec_to_ib_spec(struct ib_uverbs_spec *uverbs_spec,
+static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec *uverbs_spec,
 				union ib_flow_spec *ib_spec)
 {
 	ib_spec->type = uverbs_spec->type;
@@ -2678,7 +2678,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 
 	if (cmd.flow_attr.size < 0 || cmd.flow_attr.size > in_len ||
 	    uverbs_attr_size < 0 || uverbs_attr_size >
-	    (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_spec)))
+	    (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_flow_spec)))
 		return -EINVAL;
 
 	if (cmd.flow_attr.num_of_specs) {
@@ -2732,8 +2732,8 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 			goto err_free;
 		flow_attr->size +=
 			((union ib_flow_spec *) ib_spec)->size;
-		uverbs_attr_size -= ((struct ib_uverbs_spec *) uverbs_spec)->size;
-		uverbs_spec += ((struct ib_uverbs_spec *) uverbs_spec)->size;
+		uverbs_attr_size -= ((struct ib_uverbs_flow_spec *) uverbs_spec)->size;
+		uverbs_spec += ((struct ib_uverbs_flow_spec *) uverbs_spec)->size;
 		ib_spec += ((union ib_flow_spec *) ib_spec)->size;
 	}
 	if (uverbs_attr_size) {
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index f8fea78..61ecd59 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -696,57 +696,57 @@ struct ib_uverbs_detach_mcast {
 	__u64 driver_data[0];
 };
 
-struct ib_uverbs_eth_filter {
+struct ib_uverbs_flow_eth_filter {
 	__u8  dst_mac[6];
 	__u8  src_mac[6];
 	__be16 ether_type;
 	__be16 vlan_tag;
 };
 
-struct ib_uverbs_spec_eth {
+struct ib_uverbs_flow_spec_eth {
 	__u32  type;
 	__u16  size;
 	__u16  reserved;
-	struct ib_uverbs_eth_filter val;
-	struct ib_uverbs_eth_filter mask;
+	struct ib_uverbs_flow_eth_filter val;
+	struct ib_uverbs_flow_eth_filter mask;
 };
 
-struct ib_uverbs_ipv4_filter {
+struct ib_uverbs_flow_ipv4_filter {
 	__be32 src_ip;
 	__be32 dst_ip;
 };
 
-struct ib_uverbs_spec_ipv4 {
+struct ib_uverbs_flow_spec_ipv4 {
 	__u32  type;
 	__u16  size;
 	__u16  reserved;
-	struct ib_uverbs_ipv4_filter val;
-	struct ib_uverbs_ipv4_filter mask;
+	struct ib_uverbs_flow_ipv4_filter val;
+	struct ib_uverbs_flow_ipv4_filter mask;
 };
 
-struct ib_uverbs_tcp_udp_filter {
+struct ib_uverbs_flow_tcp_udp_filter {
 	__be16 dst_port;
 	__be16 src_port;
 };
 
-struct ib_uverbs_spec_tcp_udp {
+struct ib_uverbs_flow_spec_tcp_udp {
 	__u32  type;
 	__u16  size;
 	__u16  reserved;
-	struct ib_uverbs_tcp_udp_filter val;
-	struct ib_uverbs_tcp_udp_filter mask;
+	struct ib_uverbs_flow_tcp_udp_filter val;
+	struct ib_uverbs_flow_tcp_udp_filter mask;
 };
 
-struct ib_uverbs_spec {
+struct ib_uverbs_flow_spec {
 	union {
 		struct {
 			__u32 type;
 			__u16 size;
 			__u16 reserved;
 		};
-		struct ib_uverbs_spec_eth	    eth;
-		struct ib_uverbs_spec_ipv4    ipv4;
-		struct ib_uverbs_spec_tcp_udp tcp_udp;
+		struct ib_uverbs_flow_spec_eth	    eth;
+		struct ib_uverbs_flow_spec_ipv4    ipv4;
+		struct ib_uverbs_flow_spec_tcp_udp tcp_udp;
 	};
 };
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/10] IB/core: Don't include command size in uverbs create_flow

[Yann Droneaud]

This patch changes the semantic of the size field of create_flow command
to be the size of the flow_spec's list instead of the total size of
the command, counting command header, command and flow_spec's.

Flow attributes must be independent of command header:
they are part of different layers.

Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Link: http://mid.gmane.org/cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
 drivers/infiniband/core/uverbs_cmd.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index a4e387d..54ee607 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2673,10 +2673,9 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 	    cmd.flow_attr.num_of_specs > IB_FLOW_SPEC_SUPPORT_LAYERS)
 		return -EINVAL;
 
-	uverbs_attr_size = cmd.flow_attr.size - sizeof(cmd) -
-			 sizeof(struct ib_uverbs_cmd_hdr_ex);
+	uverbs_attr_size = cmd.flow_attr.size;
 
-	if (cmd.flow_attr.size < 0 || cmd.flow_attr.size > in_len ||
+	if (cmd.flow_attr.size < 0 || cmd.flow_attr.size > (in_len - sizeof(cmd)) ||
 	    uverbs_attr_size < 0 || uverbs_attr_size >
 	    (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_flow_spec)))
 		return -EINVAL;
@@ -2711,7 +2710,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 		goto err_uobj;
 	}
 
-	flow_attr = kmalloc(cmd.flow_attr.size, GFP_KERNEL);
+	flow_attr = kmalloc(sizeof(*flow_attr) + cmd.flow_attr.size, GFP_KERNEL);
 	if (!flow_attr) {
 		err = -ENOMEM;
 		goto err_put;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 04/10] IB/core: Allocate memory only for flow_spec in create_flow uverbs

[Yann Droneaud]

Instead of allocating memory for the flow attribute structure
and all the flow specs, this patch makes use of the flow attribute
structure already read as part of the command, allocating memory
only for the flow_specs.

Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Link: http://mid.gmane.org/cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
 drivers/infiniband/core/uverbs_cmd.c | 52 +++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 54ee607..29c89a3 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2647,14 +2647,14 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 	struct ib_uverbs_create_flow_resp resp;
 	struct ib_uobject		  *uobj;
 	struct ib_flow			  *flow_id;
-	struct ib_uverbs_flow_attr	  *uverbs_flow_attr;
+	struct ib_uverbs_flow_spec	  *uverbs_flow_spec = NULL;
 	struct ib_flow_attr		  *flow_attr;
 	struct ib_qp			  *qp;
 	int err = 0;
 	void *uverbs_spec;
 	void *ib_spec;
 	int i;
-	int uverbs_attr_size;
+	int uverbs_spec_size;
 
 	if (out_len < sizeof(resp))
 		return -ENOSPC;
@@ -2673,33 +2673,29 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 	    cmd.flow_attr.num_of_specs > IB_FLOW_SPEC_SUPPORT_LAYERS)
 		return -EINVAL;
 
-	uverbs_attr_size = cmd.flow_attr.size;
+	uverbs_spec_size = cmd.flow_attr.size;
 
 	if (cmd.flow_attr.size < 0 || cmd.flow_attr.size > (in_len - sizeof(cmd)) ||
-	    uverbs_attr_size < 0 || uverbs_attr_size >
+	    uverbs_spec_size < 0 || uverbs_spec_size >
 	    (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_flow_spec)))
 		return -EINVAL;
 
 	if (cmd.flow_attr.num_of_specs) {
-		uverbs_flow_attr = kmalloc(cmd.flow_attr.size, GFP_KERNEL);
-		if (!uverbs_flow_attr)
+		uverbs_flow_spec = kmalloc(uverbs_spec_size, GFP_KERNEL);
+		if (!uverbs_flow_spec)
 			return -ENOMEM;
 
-		memcpy(uverbs_flow_attr, &cmd.flow_attr, sizeof(*uverbs_flow_attr));
-		if (copy_from_user(uverbs_flow_attr + 1, buf + sizeof(cmd),
-				   uverbs_attr_size)) {
+		if (copy_from_user(uverbs_flow_spec, buf + sizeof(cmd),
+				   uverbs_spec_size)) {
 			err = -EFAULT;
-			goto err_free_attr;
+			goto err_free_spec;
 		}
-	} else {
-		uverbs_flow_attr = &cmd.flow_attr;
-		uverbs_attr_size = sizeof(cmd.flow_attr);
 	}
 
 	uobj = kmalloc(sizeof(*uobj), GFP_KERNEL);
 	if (!uobj) {
 		err = -ENOMEM;
-		goto err_free_attr;
+		goto err_free_spec;
 	}
 	init_uobj(uobj, 0, file->ucontext, &rule_lock_class);
 	down_write(&uobj->mutex);
@@ -2710,34 +2706,34 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 		goto err_uobj;
 	}
 
-	flow_attr = kmalloc(sizeof(*flow_attr) + cmd.flow_attr.size, GFP_KERNEL);
+	flow_attr = kmalloc(sizeof(*flow_attr) + uverbs_spec_size, GFP_KERNEL);
 	if (!flow_attr) {
 		err = -ENOMEM;
 		goto err_put;
 	}
 
-	flow_attr->type = uverbs_flow_attr->type;
-	flow_attr->priority = uverbs_flow_attr->priority;
-	flow_attr->num_of_specs = uverbs_flow_attr->num_of_specs;
-	flow_attr->port = uverbs_flow_attr->port;
-	flow_attr->flags = uverbs_flow_attr->flags;
+	flow_attr->type = cmd.flow_attr.type;
+	flow_attr->priority = cmd.flow_attr.priority;
+	flow_attr->num_of_specs = cmd.flow_attr.num_of_specs;
+	flow_attr->port = cmd.flow_attr.port;
+	flow_attr->flags = cmd.flow_attr.flags;
 	flow_attr->size = sizeof(*flow_attr);
 
-	uverbs_spec = uverbs_flow_attr + 1;
+	uverbs_spec = uverbs_flow_spec;
 	ib_spec = flow_attr + 1;
-	for (i = 0; i < flow_attr->num_of_specs && uverbs_attr_size > 0; i++) {
+	for (i = 0; i < flow_attr->num_of_specs && uverbs_spec_size > 0; i++) {
 		err = uverbs_spec_to_ib_spec(uverbs_spec, ib_spec);
 		if (err)
 			goto err_free;
 		flow_attr->size +=
 			((union ib_flow_spec *) ib_spec)->size;
-		uverbs_attr_size -= ((struct ib_uverbs_flow_spec *) uverbs_spec)->size;
+		uverbs_spec_size -= ((struct ib_uverbs_flow_spec *) uverbs_spec)->size;
 		uverbs_spec += ((struct ib_uverbs_flow_spec *) uverbs_spec)->size;
 		ib_spec += ((union ib_flow_spec *) ib_spec)->size;
 	}
-	if (uverbs_attr_size) {
+	if (uverbs_spec_size) {
 		pr_warn("create flow failed, %d bytes left from uverb cmd\n",
-			uverbs_attr_size);
+			uverbs_spec_size);
 		goto err_free;
 	}
 	flow_id = ib_create_flow(qp, flow_attr, IB_FLOW_DOMAIN_USER);
@@ -2772,7 +2768,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 	up_write(&uobj->mutex);
 	kfree(flow_attr);
 	if (cmd.flow_attr.num_of_specs)
-		kfree(uverbs_flow_attr);
+		kfree(uverbs_flow_spec);
 	return in_len;
 err_copy:
 	idr_remove_uobj(&ib_uverbs_rule_idr, uobj);
@@ -2784,9 +2780,9 @@ err_put:
 	put_qp_read(qp);
 err_uobj:
 	put_uobj_write(uobj);
-err_free_attr:
+err_free_spec:
 	if (cmd.flow_attr.num_of_specs)
-		kfree(uverbs_flow_attr);
+		kfree(uverbs_flow_spec);
 	return err;
 }
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 05/10] IB/core: Remove checks for negative values in create_flow uverbs

[Yann Droneaud]

Fields in flow_attr structure are unsigned, so it's not
necessary to check for negative values.

Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Link: http://mid.gmane.org/cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
 drivers/infiniband/core/uverbs_cmd.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 29c89a3..0ea5529 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2669,17 +2669,18 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 	     !capable(CAP_NET_ADMIN)) || !capable(CAP_NET_RAW))
 		return -EPERM;
 
-	if (cmd.flow_attr.num_of_specs < 0 ||
-	    cmd.flow_attr.num_of_specs > IB_FLOW_SPEC_SUPPORT_LAYERS)
+	if (cmd.flow_attr.num_of_specs > IB_FLOW_SPEC_SUPPORT_LAYERS)
 		return -EINVAL;
 
-	uverbs_spec_size = cmd.flow_attr.size;
+	if (cmd.flow_attr.size > (in_len - sizeof(cmd)))
+		return -EINVAL;
 
-	if (cmd.flow_attr.size < 0 || cmd.flow_attr.size > (in_len - sizeof(cmd)) ||
-	    uverbs_spec_size < 0 || uverbs_spec_size >
+	if (cmd.flow_attr.size >
 	    (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_flow_spec)))
 		return -EINVAL;
 
+	uverbs_spec_size = cmd.flow_attr.size;
+
 	if (cmd.flow_attr.num_of_specs) {
 		uverbs_flow_spec = kmalloc(uverbs_spec_size, GFP_KERNEL);
 		if (!uverbs_flow_spec)
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/10] IB/core: Check against uverbs structure size while parsing flow_spec

[Yann Droneaud]

Instead of checking input size against the kernel verbs
data structure, it's safer to check input size against
uverbs data structure.

Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Link: http://mid.gmane.org/cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
 drivers/infiniband/core/uverbs_cmd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 0ea5529..106e997 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2606,18 +2606,18 @@ static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec *uverbs_spec,
 
 	switch (ib_spec->type) {
 	case IB_FLOW_SPEC_ETH:
-		ib_spec->eth.size = sizeof(struct ib_flow_spec_eth);
-		if (ib_spec->eth.size != uverbs_spec->eth.size)
+		if (uverbs_spec->eth.size != sizeof(struct ib_uverbs_flow_spec_eth))
 			return -EINVAL;
+		ib_spec->eth.size = sizeof(struct ib_flow_spec_eth);
 		memcpy(&ib_spec->eth.val, &uverbs_spec->eth.val,
 		       sizeof(struct ib_flow_eth_filter));
 		memcpy(&ib_spec->eth.mask, &uverbs_spec->eth.mask,
 		       sizeof(struct ib_flow_eth_filter));
 		break;
 	case IB_FLOW_SPEC_IPV4:
-		ib_spec->ipv4.size = sizeof(struct ib_flow_spec_ipv4);
-		if (ib_spec->ipv4.size != uverbs_spec->ipv4.size)
+		if (uverbs_spec->ipv4.size != sizeof(struct ib_uverbs_flow_spec_ipv4))
 			return -EINVAL;
+		ib_spec->ipv4.size = sizeof(struct ib_flow_spec_ipv4);
 		memcpy(&ib_spec->ipv4.val, &uverbs_spec->ipv4.val,
 		       sizeof(struct ib_flow_ipv4_filter));
 		memcpy(&ib_spec->ipv4.mask, &uverbs_spec->ipv4.mask,
@@ -2625,9 +2625,9 @@ static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec *uverbs_spec,
 		break;
 	case IB_FLOW_SPEC_TCP:
 	case IB_FLOW_SPEC_UDP:
-		ib_spec->tcp_udp.size = sizeof(struct ib_flow_spec_tcp_udp);
-		if (ib_spec->tcp_udp.size != uverbs_spec->tcp_udp.size)
+		if (uverbs_spec->tcp_udp.size != sizeof(struct ib_uverbs_flow_spec_tcp_udp))
 			return -EINVAL;
+		ib_spec->tcp_udp.size = sizeof(struct ib_flow_spec_tcp_udp);
 		memcpy(&ib_spec->tcp_udp.val, &uverbs_spec->tcp_udp.val,
 		       sizeof(struct ib_flow_tcp_udp_filter));
 		memcpy(&ib_spec->tcp_udp.mask, &uverbs_spec->tcp_udp.mask,
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 07/10] IB/core: Uses a common header for uverbs flow_specs

[Yann Droneaud]

A common header allows better checking of flow specs
size, while ensuring strict alignment to 64bits.

Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Link: http://mid.gmane.org/cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
 drivers/infiniband/core/uverbs_cmd.c | 10 +++++--
 include/uapi/rdma/ib_user_verbs.h    | 53 +++++++++++++++++++++++++++---------
 2 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 106e997..6b066a3 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2647,7 +2647,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 	struct ib_uverbs_create_flow_resp resp;
 	struct ib_uobject		  *uobj;
 	struct ib_flow			  *flow_id;
-	struct ib_uverbs_flow_spec	  *uverbs_flow_spec = NULL;
+	struct ib_uverbs_flow_spec_hdr	  *uverbs_flow_spec = NULL;
 	struct ib_flow_attr		  *flow_attr;
 	struct ib_qp			  *qp;
 	int err = 0;
@@ -2675,6 +2675,10 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 	if (cmd.flow_attr.size > (in_len - sizeof(cmd)))
 		return -EINVAL;
 
+	if (cmd.flow_attr.size <
+	    (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_flow_spec_hdr)))
+		return -EINVAL;
+
 	if (cmd.flow_attr.size >
 	    (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_flow_spec)))
 		return -EINVAL;
@@ -2728,8 +2732,8 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 			goto err_free;
 		flow_attr->size +=
 			((union ib_flow_spec *) ib_spec)->size;
-		uverbs_spec_size -= ((struct ib_uverbs_flow_spec *) uverbs_spec)->size;
-		uverbs_spec += ((struct ib_uverbs_flow_spec *) uverbs_spec)->size;
+		uverbs_spec_size -= ((struct ib_uverbs_flow_spec_hdr *) uverbs_spec)->size;
+		uverbs_spec += ((struct ib_uverbs_flow_spec_hdr *) uverbs_spec)->size;
 		ib_spec += ((union ib_flow_spec *) ib_spec)->size;
 	}
 	if (uverbs_spec_size) {
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index 61ecd59..8be941e 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -696,6 +696,14 @@ struct ib_uverbs_detach_mcast {
 	__u64 driver_data[0];
 };
 
+struct ib_uverbs_flow_spec_hdr {
+	__u32 type;
+	__u16 size;
+	__u16 reserved;
+	/* followed by flow_spec */
+	__u64 flow_spec_data[0];
+};
+
 struct ib_uverbs_flow_eth_filter {
 	__u8  dst_mac[6];
 	__u8  src_mac[6];
@@ -704,9 +712,14 @@ struct ib_uverbs_flow_eth_filter {
 };
 
 struct ib_uverbs_flow_spec_eth {
-	__u32  type;
-	__u16  size;
-	__u16  reserved;
+	union {
+		struct ib_uverbs_flow_spec_hdr hdr;
+		struct {
+			__u32 type;
+			__u16 size;
+			__u16 reserved;
+		};
+	};
 	struct ib_uverbs_flow_eth_filter val;
 	struct ib_uverbs_flow_eth_filter mask;
 };
@@ -717,9 +730,14 @@ struct ib_uverbs_flow_ipv4_filter {
 };
 
 struct ib_uverbs_flow_spec_ipv4 {
-	__u32  type;
-	__u16  size;
-	__u16  reserved;
+	union {
+		struct ib_uverbs_flow_spec_hdr hdr;
+		struct {
+			__u32 type;
+			__u16 size;
+			__u16 reserved;
+		};
+	};
 	struct ib_uverbs_flow_ipv4_filter val;
 	struct ib_uverbs_flow_ipv4_filter mask;
 };
@@ -730,19 +748,27 @@ struct ib_uverbs_flow_tcp_udp_filter {
 };
 
 struct ib_uverbs_flow_spec_tcp_udp {
-	__u32  type;
-	__u16  size;
-	__u16  reserved;
+	union {
+		struct ib_uverbs_flow_spec_hdr hdr;
+		struct {
+			__u32 type;
+			__u16 size;
+			__u16 reserved;
+		};
+	};
 	struct ib_uverbs_flow_tcp_udp_filter val;
 	struct ib_uverbs_flow_tcp_udp_filter mask;
 };
 
 struct ib_uverbs_flow_spec {
 	union {
-		struct {
-			__u32 type;
-			__u16 size;
-			__u16 reserved;
+		union {
+			struct ib_uverbs_flow_spec_hdr hdr;
+			struct {
+				__u32 type;
+				__u16 size;
+				__u16 reserved;
+			};
 		};
 		struct ib_uverbs_flow_spec_eth	    eth;
 		struct ib_uverbs_flow_spec_ipv4    ipv4;
@@ -762,6 +788,7 @@ struct ib_uverbs_flow_attr {
 	 * struct ib_flow_spec_xxx
 	 * struct ib_flow_spec_yyy
 	 */
+	struct ib_uverbs_flow_spec_hdr flow_specs[0];
 };
 
 struct ib_uverbs_create_flow  {
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 08/10] IB/core: Use flow_spec common header in uverbs flow_spec conversion function

[Yann Droneaud]

While the validity (and type) of the flow_spec is not known before calling
the conversion function, it's make more sense to use a pointer
to the header structure.

Once the header parameters are validated, the flow_spec can be processed.

Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Link: http://mid.gmane.org/cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
 drivers/infiniband/core/uverbs_cmd.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 6b066a3..8079383 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2599,15 +2599,18 @@ out_put:
 	return ret ? ret : in_len;
 }
 
-static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec *uverbs_spec,
+static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec_hdr *uverbs_spec_hdr,
 				union ib_flow_spec *ib_spec)
 {
-	ib_spec->type = uverbs_spec->type;
+	struct ib_uverbs_flow_spec *uverbs_spec;
+
+	ib_spec->type = uverbs_spec_hdr->type;
 
 	switch (ib_spec->type) {
 	case IB_FLOW_SPEC_ETH:
-		if (uverbs_spec->eth.size != sizeof(struct ib_uverbs_flow_spec_eth))
+		if (uverbs_spec_hdr->size != sizeof(struct ib_uverbs_flow_spec_eth))
 			return -EINVAL;
+		uverbs_spec = (struct ib_uverbs_flow_spec *)uverbs_spec_hdr;
 		ib_spec->eth.size = sizeof(struct ib_flow_spec_eth);
 		memcpy(&ib_spec->eth.val, &uverbs_spec->eth.val,
 		       sizeof(struct ib_flow_eth_filter));
@@ -2615,8 +2618,9 @@ static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec *uverbs_spec,
 		       sizeof(struct ib_flow_eth_filter));
 		break;
 	case IB_FLOW_SPEC_IPV4:
-		if (uverbs_spec->ipv4.size != sizeof(struct ib_uverbs_flow_spec_ipv4))
+		if (uverbs_spec_hdr->size != sizeof(struct ib_uverbs_flow_spec_ipv4))
 			return -EINVAL;
+		uverbs_spec = (struct ib_uverbs_flow_spec *)uverbs_spec_hdr;
 		ib_spec->ipv4.size = sizeof(struct ib_flow_spec_ipv4);
 		memcpy(&ib_spec->ipv4.val, &uverbs_spec->ipv4.val,
 		       sizeof(struct ib_flow_ipv4_filter));
@@ -2625,8 +2629,9 @@ static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec *uverbs_spec,
 		break;
 	case IB_FLOW_SPEC_TCP:
 	case IB_FLOW_SPEC_UDP:
-		if (uverbs_spec->tcp_udp.size != sizeof(struct ib_uverbs_flow_spec_tcp_udp))
+		if (uverbs_spec_hdr->size != sizeof(struct ib_uverbs_flow_spec_tcp_udp))
 			return -EINVAL;
+		uverbs_spec = (struct ib_uverbs_flow_spec *)uverbs_spec_hdr;
 		ib_spec->tcp_udp.size = sizeof(struct ib_flow_spec_tcp_udp);
 		memcpy(&ib_spec->tcp_udp.val, &uverbs_spec->tcp_udp.val,
 		       sizeof(struct ib_flow_tcp_udp_filter));
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/10] IB/core: Remove ib_uverbs_flow_spec structure from userspace

[Yann Droneaud]

The structure holding any types of flow_spec is of no use to userspace.
It would be wrong for userspace to do:

  struct ib_uverbs_flow_spec flow_spec;

  flow_spec.type = IB_FLOW_SPEC_TCP;
  flow_spec.size = sizeof(flow_spec);

Instead, userspace should use the dedicated flow_spec structure for
  - Ethernet : struct ib_uverbs_flow_spec_eth,
  - IPv4     : struct ib_uverbs_flow_spec_ipv4,
  - TCP/UDP  : struct ib_uverbs_flow_spec_tcp_udp.

In other words, struct ib_uverbs_flow_spec is a "virtual"
data structure that can only be use by the kernel as an alias
to the other.

Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Link: http://mid.gmane.org/cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
 drivers/infiniband/core/uverbs.h     | 16 ++++++++++++++++
 drivers/infiniband/core/uverbs_cmd.c | 10 +++++-----
 include/uapi/rdma/ib_user_verbs.h    | 16 ----------------
 3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index d040b87..b16e9fb 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -178,6 +178,22 @@ void ib_uverbs_event_handler(struct ib_event_handler *handler,
 			     struct ib_event *event);
 void ib_uverbs_dealloc_xrcd(struct ib_uverbs_device *dev, struct ib_xrcd *xrcd);
 
+struct ib_uverbs_flow_spec_storage {
+	union {
+		union {
+			struct ib_uverbs_flow_spec_hdr hdr;
+			struct {
+				__u32 type;
+				__u16 size;
+				__u16 reserved;
+			};
+		};
+		struct ib_uverbs_flow_spec_eth     eth;
+		struct ib_uverbs_flow_spec_ipv4    ipv4;
+		struct ib_uverbs_flow_spec_tcp_udp tcp_udp;
+	};
+};
+
 #define IB_UVERBS_DECLARE_CMD(name)					\
 	ssize_t ib_uverbs_##name(struct ib_uverbs_file *file,		\
 				 const char __user *buf, int in_len,	\
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 8079383..902dfd3 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2602,7 +2602,7 @@ out_put:
 static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec_hdr *uverbs_spec_hdr,
 				union ib_flow_spec *ib_spec)
 {
-	struct ib_uverbs_flow_spec *uverbs_spec;
+	struct ib_uverbs_flow_spec_storage *uverbs_spec;
 
 	ib_spec->type = uverbs_spec_hdr->type;
 
@@ -2610,7 +2610,7 @@ static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec_hdr *uverbs_spec_hd
 	case IB_FLOW_SPEC_ETH:
 		if (uverbs_spec_hdr->size != sizeof(struct ib_uverbs_flow_spec_eth))
 			return -EINVAL;
-		uverbs_spec = (struct ib_uverbs_flow_spec *)uverbs_spec_hdr;
+		uverbs_spec = (struct ib_uverbs_flow_spec_storage *)uverbs_spec_hdr;
 		ib_spec->eth.size = sizeof(struct ib_flow_spec_eth);
 		memcpy(&ib_spec->eth.val, &uverbs_spec->eth.val,
 		       sizeof(struct ib_flow_eth_filter));
@@ -2620,7 +2620,7 @@ static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec_hdr *uverbs_spec_hd
 	case IB_FLOW_SPEC_IPV4:
 		if (uverbs_spec_hdr->size != sizeof(struct ib_uverbs_flow_spec_ipv4))
 			return -EINVAL;
-		uverbs_spec = (struct ib_uverbs_flow_spec *)uverbs_spec_hdr;
+		uverbs_spec = (struct ib_uverbs_flow_spec_storage *)uverbs_spec_hdr;
 		ib_spec->ipv4.size = sizeof(struct ib_flow_spec_ipv4);
 		memcpy(&ib_spec->ipv4.val, &uverbs_spec->ipv4.val,
 		       sizeof(struct ib_flow_ipv4_filter));
@@ -2631,7 +2631,7 @@ static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec_hdr *uverbs_spec_hd
 	case IB_FLOW_SPEC_UDP:
 		if (uverbs_spec_hdr->size != sizeof(struct ib_uverbs_flow_spec_tcp_udp))
 			return -EINVAL;
-		uverbs_spec = (struct ib_uverbs_flow_spec *)uverbs_spec_hdr;
+		uverbs_spec = (struct ib_uverbs_flow_spec_storage *)uverbs_spec_hdr;
 		ib_spec->tcp_udp.size = sizeof(struct ib_flow_spec_tcp_udp);
 		memcpy(&ib_spec->tcp_udp.val, &uverbs_spec->tcp_udp.val,
 		       sizeof(struct ib_flow_tcp_udp_filter));
@@ -2685,7 +2685,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 		return -EINVAL;
 
 	if (cmd.flow_attr.size >
-	    (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_flow_spec)))
+	    (cmd.flow_attr.num_of_specs * sizeof(struct ib_uverbs_flow_spec_storage)))
 		return -EINVAL;
 
 	uverbs_spec_size = cmd.flow_attr.size;
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index 8be941e..cd12dd7 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -760,22 +760,6 @@ struct ib_uverbs_flow_spec_tcp_udp {
 	struct ib_uverbs_flow_tcp_udp_filter mask;
 };
 
-struct ib_uverbs_flow_spec {
-	union {
-		union {
-			struct ib_uverbs_flow_spec_hdr hdr;
-			struct {
-				__u32 type;
-				__u16 size;
-				__u16 reserved;
-			};
-		};
-		struct ib_uverbs_flow_spec_eth	    eth;
-		struct ib_uverbs_flow_spec_ipv4    ipv4;
-		struct ib_uverbs_flow_spec_tcp_udp tcp_udp;
-	};
-};
-
 struct ib_uverbs_flow_attr {
 	__u32 type;
 	__u16 size;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/10] IB/core: Add proper bound checking in flow_spec uverbs conversion to verbs

[Yann Droneaud]

Relying on the size sent by userspace as the size of kernel
internal data structure is not safe.

This patch add some level of protections to
- not read past uverbs command in case of truncated uverbs flow spec
- not write past flow_attr in case of changes in kernel flow spec
  data structure format: at some point the format of internal flow_spec
  may change while the userspace format is kept the same.

Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
Link: http://marc.info/?i=cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
Link: http://mid.gmane.org/cover.1381351016.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
---
 drivers/infiniband/core/uverbs_cmd.c | 41 +++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 902dfd3..1887b8a 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2600,17 +2600,25 @@ out_put:
 }
 
 static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec_hdr *uverbs_spec_hdr,
-				union ib_flow_spec *ib_spec)
+				  size_t uverbs_spec_size,
+				  union ib_flow_spec *ib_spec,
+				  size_t ib_spec_size)
 {
 	struct ib_uverbs_flow_spec_storage *uverbs_spec;
 
-	ib_spec->type = uverbs_spec_hdr->type;
+	if (uverbs_spec_size < sizeof(struct ib_uverbs_flow_spec_hdr))
+		return -EINVAL;
 
-	switch (ib_spec->type) {
+	switch (uverbs_spec_hdr->type) {
 	case IB_FLOW_SPEC_ETH:
+		if (uverbs_spec_size < sizeof(struct ib_uverbs_flow_spec_eth))
+			return -EINVAL;
 		if (uverbs_spec_hdr->size != sizeof(struct ib_uverbs_flow_spec_eth))
 			return -EINVAL;
 		uverbs_spec = (struct ib_uverbs_flow_spec_storage *)uverbs_spec_hdr;
+		if (ib_spec_size < sizeof(struct ib_flow_spec_eth))
+			return -EINVAL;
+		ib_spec->type = IB_FLOW_SPEC_ETH;
 		ib_spec->eth.size = sizeof(struct ib_flow_spec_eth);
 		memcpy(&ib_spec->eth.val, &uverbs_spec->eth.val,
 		       sizeof(struct ib_flow_eth_filter));
@@ -2618,9 +2626,14 @@ static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec_hdr *uverbs_spec_hd
 		       sizeof(struct ib_flow_eth_filter));
 		break;
 	case IB_FLOW_SPEC_IPV4:
+		if (uverbs_spec_size < sizeof(struct ib_uverbs_flow_spec_ipv4))
+			return -EINVAL;
 		if (uverbs_spec_hdr->size != sizeof(struct ib_uverbs_flow_spec_ipv4))
 			return -EINVAL;
 		uverbs_spec = (struct ib_uverbs_flow_spec_storage *)uverbs_spec_hdr;
+		if (ib_spec_size < sizeof(struct ib_flow_spec_ipv4))
+			return -EINVAL;
+		ib_spec->type = IB_FLOW_SPEC_IPV4;
 		ib_spec->ipv4.size = sizeof(struct ib_flow_spec_ipv4);
 		memcpy(&ib_spec->ipv4.val, &uverbs_spec->ipv4.val,
 		       sizeof(struct ib_flow_ipv4_filter));
@@ -2629,9 +2642,14 @@ static int uverbs_spec_to_ib_spec(struct ib_uverbs_flow_spec_hdr *uverbs_spec_hd
 		break;
 	case IB_FLOW_SPEC_TCP:
 	case IB_FLOW_SPEC_UDP:
+		if (uverbs_spec_size < sizeof(struct ib_uverbs_flow_spec_tcp_udp))
+			return -EINVAL;
 		if (uverbs_spec_hdr->size != sizeof(struct ib_uverbs_flow_spec_tcp_udp))
 			return -EINVAL;
 		uverbs_spec = (struct ib_uverbs_flow_spec_storage *)uverbs_spec_hdr;
+		if (ib_spec_size < sizeof(struct ib_flow_spec_tcp_udp))
+			return -EINVAL;
+		ib_spec->type = uverbs_spec_hdr->type;
 		ib_spec->tcp_udp.size = sizeof(struct ib_flow_spec_tcp_udp);
 		memcpy(&ib_spec->tcp_udp.val, &uverbs_spec->tcp_udp.val,
 		       sizeof(struct ib_flow_tcp_udp_filter));
@@ -2659,7 +2677,8 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 	void *uverbs_spec;
 	void *ib_spec;
 	int i;
-	int uverbs_spec_size;
+	size_t uverbs_spec_size;
+	size_t ib_spec_size;
 
 	if (out_len < sizeof(resp))
 		return -ENOSPC;
@@ -2716,7 +2735,9 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 		goto err_uobj;
 	}
 
-	flow_attr = kmalloc(sizeof(*flow_attr) + uverbs_spec_size, GFP_KERNEL);
+	ib_spec_size = min(uverbs_spec_size,
+			   cmd.flow_attr.num_of_specs * sizeof(union ib_flow_spec));
+	flow_attr = kmalloc(sizeof(*flow_attr) + ib_spec_size, GFP_KERNEL);
 	if (!flow_attr) {
 		err = -ENOMEM;
 		goto err_put;
@@ -2731,18 +2752,22 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
 
 	uverbs_spec = uverbs_flow_spec;
 	ib_spec = flow_attr + 1;
-	for (i = 0; i < flow_attr->num_of_specs && uverbs_spec_size > 0; i++) {
-		err = uverbs_spec_to_ib_spec(uverbs_spec, ib_spec);
+	for (i = 0;
+	     i < flow_attr->num_of_specs && ib_spec_size > 0 &&	uverbs_spec_size > 0;
+	     i++) {
+		err = uverbs_spec_to_ib_spec(uverbs_spec, uverbs_spec_size,
+					     ib_spec, ib_spec_size);
 		if (err)
 			goto err_free;
 		flow_attr->size +=
 			((union ib_flow_spec *) ib_spec)->size;
 		uverbs_spec_size -= ((struct ib_uverbs_flow_spec_hdr *) uverbs_spec)->size;
 		uverbs_spec += ((struct ib_uverbs_flow_spec_hdr *) uverbs_spec)->size;
+		ib_spec_size -= ((union ib_flow_spec *) ib_spec)->size;
 		ib_spec += ((union ib_flow_spec *) ib_spec)->size;
 	}
 	if (uverbs_spec_size) {
-		pr_warn("create flow failed, %d bytes left from uverb cmd\n",
+		pr_warn("create flow failed, %zu bytes left from uverb cmd\n",
 			uverbs_spec_size);
 		goto err_free;
 	}
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/10] IB/core: more fixes for create_flow uverbs (for v3.12)

[Or Gerlitz]

On 10/10/2013 00:12, Yann Droneaud wrote:
> I've suggested reverting temporarily commit 22878db, 436f2ad, 400dbc9
> for v3.12 in a previous message [1]. This proposition doesn't sound
> acceptable for Or.
>
> So I continue to improve them to be in a better shape for v3.12.

Yan, we are 3-4 days before rc5 and as such renaming/beautifying things 
or removing checks for negative values
against unsigned variables et al cleanups are not acceptable. You need 
to re-arrange the series such that ONLY the fixes
to the user/kernel API are targeted to 3.12 and appear as the first 2-3 
patches, the rest will go to 3.13 subject to review.

Also - does git am removes the "Link: yyy" line  you added in your 
patches while applying them? if not, please place
below the --- line so we will not carry them in the kernel log.

Or.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/10] IB/core: more fixes for create_flow uverbs (for v3.12)

[Yann Droneaud]

Le 10.10.2013 10:02, Or Gerlitz a écrit :
> On 10/10/2013 00:12, Yann Droneaud wrote:
>> I've suggested reverting temporarily commit 22878db, 436f2ad, 400dbc9
>> for v3.12 in a previous message [1]. This proposition doesn't sound
>> acceptable for Or.
>> 
>> So I continue to improve them to be in a better shape for v3.12.
> 
> Yan, we are 3-4 days before rc5 and as such renaming/beautifying
> things or removing checks for negative values
> against unsigned variables et al cleanups are not acceptable. You need
> to re-arrange the series such that ONLY the fixes
> to the user/kernel API are targeted to 3.12 and appear as the first
> 2-3 patches, the rest will go to 3.13 subject to review.
> 

It does not only "beautifying" the code: it actually fix it.
In its current form, the code is not acceptable and should not go in 
v3.12.

I've made a review of this code but no patch actually fix all the size
computation issues.

http://marc.info/?i=1378908269.2258.31.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org
http://marc.info/?i=fe9240e87dd3b48d363be6ce8dcac3b3-zgzEX58YAwA@public.gmane.org

Even if I move the "renaming" patches later, the data structure name 
must
be updated throughout the code, this will make the patchset still a bit 
large
for v3.12-rc5.

I really understand it's too big for v3.12, that's why I've proposed to 
revert
the current code for v3.12 and push all of this in v3.13.

> Also - does git am removes the "Link: yyy" line  you added in your
> patches while applying them? if not, please place
> below the --- line so we will not carry them in the kernel log.
> 

I've added them on purpose so they are kept in the kernel log.
I'm following a good advice found in comments[1] of
"What's missing from our changelogs" article[2]

[1] http://lwn.net/Articles/560392/
[2] http://lwn.net/Articles/560617/

Regards.

-- 
Yann Droneaud
OPTEYA

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/10] IB/core: more fixes for create_flow uverbs (for v3.12)

[Or Gerlitz]

On 10/10/2013 11:32, Yann Droneaud wrote:
> Le 10.10.2013 10:02, Or Gerlitz a écrit :
>> On 10/10/2013 00:12, Yann Droneaud wrote:
>>> I've suggested reverting temporarily commit 22878db, 436f2ad, 400dbc9
>>> for v3.12 in a previous message [1]. This proposition doesn't sound
>>> acceptable for Or.
>>>
>>> So I continue to improve them to be in a better shape for v3.12.
>>
>> Yan, we are 3-4 days before rc5 and as such renaming/beautifying
>> things or removing checks for negative values
>> against unsigned variables et al cleanups are not acceptable. You need
>> to re-arrange the series such that ONLY the fixes
>> to the user/kernel API are targeted to 3.12 and appear as the first
>> 2-3 patches, the rest will go to 3.13 subject to review.
>>
>
> It does not only "beautifying" the code: it actually fix it.

IB/core: Rename 'flow' structs to match other uverbs structs
IB/core: Makes uverbs flow structure using names more similar to verbs ones
IB/core: Remove checks for negative values in create_flow uverbs

You must agree that these three patches can go to 3.13 -- also, if there are
other small fixes which do not touch the user/kernel API they can go to 
3.13
too and subject to review into -stable

What you need to prepare for 3.12 are fixes that relate to user/kernel API
and are not fixed in the patch Matan sent on Sep 22nd 
https://patchwork.kernel.org/patch/2924341
and relative to that patch


Or.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/10] IB/core: more fixes for create_flow uverbs (for v3.12)

[Yann Droneaud]

Hi

Le 10.10.2013 11:01, Or Gerlitz a écrit :
> On 10/10/2013 11:32, Yann Droneaud wrote:
>> Le 10.10.2013 10:02, Or Gerlitz a écrit :
>>> On 10/10/2013 00:12, Yann Droneaud wrote:
>>>> I've suggested reverting temporarily commit 22878db, 436f2ad, 
>>>> 400dbc9
>>>> for v3.12 in a previous message [1]. This proposition doesn't sound
>>>> acceptable for Or.
>>>> 
>>>> So I continue to improve them to be in a better shape for v3.12.
>>> 
>>> Yan, we are 3-4 days before rc5 and as such renaming/beautifying
>>> things or removing checks for negative values
>>> against unsigned variables et al cleanups are not acceptable. You 
>>> need
>>> to re-arrange the series such that ONLY the fixes
>>> to the user/kernel API are targeted to 3.12 and appear as the first
>>> 2-3 patches, the rest will go to 3.13 subject to review.
>>> 
>> 
>> It does not only "beautifying" the code: it actually fix it.
> 
> IB/core: Rename 'flow' structs to match other uverbs structs

Public API, exposed to userspace

> IB/core: Makes uverbs flow structure using names more similar to verbs 
> ones

Public API, exposed to userspace

> IB/core: Remove checks for negative values in create_flow uverbs
> 

This one could go for v3.13. (But probably to discard if Matan's one is 
applied
first, see below).

> You must agree that these three patches can go to 3.13 -- also, if 
> there are
> other small fixes which do not touch the user/kernel API they can go to 
> 3.13
> too and subject to review into -stable
> 
> What you need to prepare for 3.12 are fixes that relate to user/kernel 
> API
> and are not fixed in the patch Matan sent on Sep 22nd
> https://patchwork.kernel.org/patch/2924341
> and relative to that patch
> 

This patch could go for 3.13 too: it's not altering (fixing) the public 
API.

In the meantime, I've sent a patch that will lower the urge to fix for 
v3.12,
and reduce the size of the merge request to be proposed to Linus.

Regards.

-- 
Yann Droneaud
OPTEYA

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html