Commit f8848274 authored by Matan Barak's avatar Matan Barak Committed by Roland Dreier

IB/core: clarify overflow/underflow checks on ib_create/destroy_flow

This patch fixes the following issues:

1. Unneeded checks were removed

2. Removed the fixed size out of flow_attr.size, thus simplifying the checks.

3. Remove a 32bit hole on 64bit systems with strict alignment in
   struct ib_kern_flow_att by adding a reserved field.
Signed-off-by: default avatarMatan Barak <matanb@mellanox.com>
Signed-off-by: default avatarRoland Dreier <roland@purestorage.com>
parent 959f5854
...@@ -2657,7 +2657,6 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file, ...@@ -2657,7 +2657,6 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
void *kern_spec; void *kern_spec;
void *ib_spec; void *ib_spec;
int i; int i;
int kern_attr_size;
if (out_len < sizeof(resp)) if (out_len < sizeof(resp))
return -ENOSPC; return -ENOSPC;
...@@ -2672,32 +2671,28 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file, ...@@ -2672,32 +2671,28 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
!capable(CAP_NET_ADMIN)) || !capable(CAP_NET_RAW)) !capable(CAP_NET_ADMIN)) || !capable(CAP_NET_RAW))
return -EPERM; return -EPERM;
if (cmd.flow_attr.num_of_specs < 0 || if (cmd.flow_attr.num_of_specs > IB_FLOW_SPEC_SUPPORT_LAYERS)
cmd.flow_attr.num_of_specs > IB_FLOW_SPEC_SUPPORT_LAYERS)
return -EINVAL; return -EINVAL;
kern_attr_size = cmd.flow_attr.size - sizeof(cmd) - if (cmd.flow_attr.size > (in_len - sizeof(cmd)) ||
sizeof(struct ib_uverbs_cmd_hdr_ex); cmd.flow_attr.size >
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))) (cmd.flow_attr.num_of_specs * sizeof(struct ib_kern_spec)))
return -EINVAL; return -EINVAL;
if (cmd.flow_attr.num_of_specs) { if (cmd.flow_attr.num_of_specs) {
kern_flow_attr = kmalloc(cmd.flow_attr.size, GFP_KERNEL); kern_flow_attr = kmalloc(sizeof(*kern_flow_attr) + cmd.flow_attr.size,
GFP_KERNEL);
if (!kern_flow_attr) if (!kern_flow_attr)
return -ENOMEM; return -ENOMEM;
memcpy(kern_flow_attr, &cmd.flow_attr, sizeof(*kern_flow_attr)); memcpy(kern_flow_attr, &cmd.flow_attr, sizeof(*kern_flow_attr));
if (copy_from_user(kern_flow_attr + 1, buf + sizeof(cmd), if (copy_from_user(kern_flow_attr + 1, buf + sizeof(cmd),
kern_attr_size)) { cmd.flow_attr.size)) {
err = -EFAULT; err = -EFAULT;
goto err_free_attr; goto err_free_attr;
} }
} else { } else {
kern_flow_attr = &cmd.flow_attr; kern_flow_attr = &cmd.flow_attr;
kern_attr_size = sizeof(cmd.flow_attr);
} }
uobj = kmalloc(sizeof(*uobj), GFP_KERNEL); uobj = kmalloc(sizeof(*uobj), GFP_KERNEL);
...@@ -2714,7 +2709,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file, ...@@ -2714,7 +2709,7 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
goto err_uobj; 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) { if (!flow_attr) {
err = -ENOMEM; err = -ENOMEM;
goto err_put; goto err_put;
...@@ -2729,19 +2724,22 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file, ...@@ -2729,19 +2724,22 @@ ssize_t ib_uverbs_create_flow(struct ib_uverbs_file *file,
kern_spec = kern_flow_attr + 1; kern_spec = kern_flow_attr + 1;
ib_spec = flow_attr + 1; ib_spec = flow_attr + 1;
for (i = 0; i < flow_attr->num_of_specs && kern_attr_size > 0; i++) { for (i = 0; i < flow_attr->num_of_specs &&
cmd.flow_attr.size > offsetof(struct ib_kern_spec, reserved) &&
cmd.flow_attr.size >=
((struct ib_kern_spec *)kern_spec)->size; i++) {
err = kern_spec_to_ib_spec(kern_spec, ib_spec); err = kern_spec_to_ib_spec(kern_spec, ib_spec);
if (err) if (err)
goto err_free; goto err_free;
flow_attr->size += flow_attr->size +=
((union ib_flow_spec *) ib_spec)->size; ((union ib_flow_spec *) ib_spec)->size;
kern_attr_size -= ((struct ib_kern_spec *) kern_spec)->size; cmd.flow_attr.size -= ((struct ib_kern_spec *)kern_spec)->size;
kern_spec += ((struct ib_kern_spec *) kern_spec)->size; kern_spec += ((struct ib_kern_spec *) kern_spec)->size;
ib_spec += ((union ib_flow_spec *) ib_spec)->size; ib_spec += ((union ib_flow_spec *) ib_spec)->size;
} }
if (kern_attr_size) { if (cmd.flow_attr.size || (i != flow_attr->num_of_specs)) {
pr_warn("create flow failed, %d bytes left from uverb cmd\n", pr_warn("create flow failed, flow %d: %d bytes left from uverb cmd\n",
kern_attr_size); i, cmd.flow_attr.size);
goto err_free; goto err_free;
} }
flow_id = ib_create_flow(qp, flow_attr, IB_FLOW_DOMAIN_USER); flow_id = ib_create_flow(qp, flow_attr, IB_FLOW_DOMAIN_USER);
......
...@@ -771,6 +771,7 @@ struct ib_kern_flow_attr { ...@@ -771,6 +771,7 @@ struct ib_kern_flow_attr {
struct ib_uverbs_create_flow { struct ib_uverbs_create_flow {
__u32 comp_mask; __u32 comp_mask;
__u32 reserved;
__u64 response; __u64 response;
__u32 qp_handle; __u32 qp_handle;
struct ib_kern_flow_attr flow_attr; struct ib_kern_flow_attr flow_attr;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment