Le jeudi 11 décembre 2014 à 17:04 +0200, Haggai Eran a écrit : > From: Eli Cohen <eli@xxxxxxxxxxxxxxxxxx> > > Add extensible query device capabilities verb to allow adding new features. > ib_uverbs_ex_query_device is added and copy_query_dev_fields is used to copy > capability fields to be used by both ib_uverbs_query_device and > ib_uverbs_ex_query_device. > > Signed-off-by: Eli Cohen <eli@xxxxxxxxxxxx> > Signed-off-by: Haggai Eran <haggaie@xxxxxxxxxxxx> > --- > drivers/infiniband/core/uverbs.h | 1 + > drivers/infiniband/core/uverbs_cmd.c | 124 +++++++++++++++++++++++----------- > drivers/infiniband/core/uverbs_main.c | 3 +- > include/rdma/ib_verbs.h | 5 +- > include/uapi/rdma/ib_user_verbs.h | 14 +++- > 5 files changed, 103 insertions(+), 44 deletions(-) > > diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h > index 643c08a025a5..b716b0815644 100644 > --- a/drivers/infiniband/core/uverbs.h > +++ b/drivers/infiniband/core/uverbs.h > @@ -258,5 +258,6 @@ IB_UVERBS_DECLARE_CMD(close_xrcd); > > IB_UVERBS_DECLARE_EX_CMD(create_flow); > IB_UVERBS_DECLARE_EX_CMD(destroy_flow); > +IB_UVERBS_DECLARE_EX_CMD(query_device); > > #endif /* UVERBS_H */ > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c > index 5ba2a86aab6a..c7a43624c96b 100644 > --- a/drivers/infiniband/core/uverbs_cmd.c > +++ b/drivers/infiniband/core/uverbs_cmd.c > @@ -378,6 +378,52 @@ err: > return ret; > } > > +static void copy_query_dev_fields(struct ib_uverbs_file *file, > + struct ib_uverbs_query_device_resp *resp, > + struct ib_device_attr *attr) > +{ > + resp->fw_ver = attr->fw_ver; > + resp->node_guid = file->device->ib_dev->node_guid; > + resp->sys_image_guid = attr->sys_image_guid; > + resp->max_mr_size = attr->max_mr_size; > + resp->page_size_cap = attr->page_size_cap; > + resp->vendor_id = attr->vendor_id; > + resp->vendor_part_id = attr->vendor_part_id; > + resp->hw_ver = attr->hw_ver; > + resp->max_qp = attr->max_qp; > + resp->max_qp_wr = attr->max_qp_wr; > + resp->device_cap_flags = attr->device_cap_flags; > + resp->max_sge = attr->max_sge; > + resp->max_sge_rd = attr->max_sge_rd; > + resp->max_cq = attr->max_cq; > + resp->max_cqe = attr->max_cqe; > + resp->max_mr = attr->max_mr; > + resp->max_pd = attr->max_pd; > + resp->max_qp_rd_atom = attr->max_qp_rd_atom; > + resp->max_ee_rd_atom = attr->max_ee_rd_atom; > + resp->max_res_rd_atom = attr->max_res_rd_atom; > + resp->max_qp_init_rd_atom = attr->max_qp_init_rd_atom; > + resp->max_ee_init_rd_atom = attr->max_ee_init_rd_atom; > + resp->atomic_cap = attr->atomic_cap; > + resp->max_ee = attr->max_ee; > + resp->max_rdd = attr->max_rdd; > + resp->max_mw = attr->max_mw; > + resp->max_raw_ipv6_qp = attr->max_raw_ipv6_qp; > + resp->max_raw_ethy_qp = attr->max_raw_ethy_qp; > + resp->max_mcast_grp = attr->max_mcast_grp; > + resp->max_mcast_qp_attach = attr->max_mcast_qp_attach; > + resp->max_total_mcast_qp_attach = attr->max_total_mcast_qp_attach; > + resp->max_ah = attr->max_ah; > + resp->max_fmr = attr->max_fmr; > + resp->max_map_per_fmr = attr->max_map_per_fmr; > + resp->max_srq = attr->max_srq; > + resp->max_srq_wr = attr->max_srq_wr; > + resp->max_srq_sge = attr->max_srq_sge; > + resp->max_pkeys = attr->max_pkeys; > + resp->local_ca_ack_delay = attr->local_ca_ack_delay; > + resp->phys_port_cnt = file->device->ib_dev->phys_port_cnt; > +} > + > ssize_t ib_uverbs_query_device(struct ib_uverbs_file *file, > const char __user *buf, > int in_len, int out_len) > @@ -398,47 +444,7 @@ ssize_t ib_uverbs_query_device(struct ib_uverbs_file *file, > return ret; > > memset(&resp, 0, sizeof resp); > - > - resp.fw_ver = attr.fw_ver; > - resp.node_guid = file->device->ib_dev->node_guid; > - resp.sys_image_guid = attr.sys_image_guid; > - resp.max_mr_size = attr.max_mr_size; > - resp.page_size_cap = attr.page_size_cap; > - resp.vendor_id = attr.vendor_id; > - resp.vendor_part_id = attr.vendor_part_id; > - resp.hw_ver = attr.hw_ver; > - resp.max_qp = attr.max_qp; > - resp.max_qp_wr = attr.max_qp_wr; > - resp.device_cap_flags = attr.device_cap_flags; > - resp.max_sge = attr.max_sge; > - resp.max_sge_rd = attr.max_sge_rd; > - resp.max_cq = attr.max_cq; > - resp.max_cqe = attr.max_cqe; > - resp.max_mr = attr.max_mr; > - resp.max_pd = attr.max_pd; > - resp.max_qp_rd_atom = attr.max_qp_rd_atom; > - resp.max_ee_rd_atom = attr.max_ee_rd_atom; > - resp.max_res_rd_atom = attr.max_res_rd_atom; > - resp.max_qp_init_rd_atom = attr.max_qp_init_rd_atom; > - resp.max_ee_init_rd_atom = attr.max_ee_init_rd_atom; > - resp.atomic_cap = attr.atomic_cap; > - resp.max_ee = attr.max_ee; > - resp.max_rdd = attr.max_rdd; > - resp.max_mw = attr.max_mw; > - resp.max_raw_ipv6_qp = attr.max_raw_ipv6_qp; > - resp.max_raw_ethy_qp = attr.max_raw_ethy_qp; > - resp.max_mcast_grp = attr.max_mcast_grp; > - resp.max_mcast_qp_attach = attr.max_mcast_qp_attach; > - resp.max_total_mcast_qp_attach = attr.max_total_mcast_qp_attach; > - resp.max_ah = attr.max_ah; > - resp.max_fmr = attr.max_fmr; > - resp.max_map_per_fmr = attr.max_map_per_fmr; > - resp.max_srq = attr.max_srq; > - resp.max_srq_wr = attr.max_srq_wr; > - resp.max_srq_sge = attr.max_srq_sge; > - resp.max_pkeys = attr.max_pkeys; > - resp.local_ca_ack_delay = attr.local_ca_ack_delay; > - resp.phys_port_cnt = file->device->ib_dev->phys_port_cnt; > + copy_query_dev_fields(file, &resp, &attr); > > if (copy_to_user((void __user *) (unsigned long) cmd.response, > &resp, sizeof resp)) > @@ -3253,3 +3259,39 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file, > > return ret ? ret : in_len; > } > + > +int ib_uverbs_ex_query_device(struct ib_uverbs_file *file, > + struct ib_udata *ucore, > + struct ib_udata *uhw) > +{ > + struct ib_uverbs_ex_query_device_resp resp; > + struct ib_uverbs_ex_query_device cmd; > + struct ib_device_attr attr; > + struct ib_device *device; > + int err; > + > + device = file->device->ib_dev; > + if (ucore->inlen < sizeof(cmd)) > + return -EINVAL; > + > + err = ib_copy_from_udata(&cmd, ucore, sizeof(cmd)); > + if (err) > + return err; > + > + if (cmd.reserved) > + return -EINVAL; > + > + err = device->query_device(device, &attr); > + if (err) > + return err; > + > + memset(&resp, 0, sizeof(resp)); > + copy_query_dev_fields(file, &resp.base, &attr); > + resp.comp_mask = 0; > + > + err = ib_copy_to_udata(ucore, &resp, sizeof(resp)); > + if (err) > + return err; > + > + return 0; > +} > diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c > index 71ab83fde472..974025028790 100644 > --- a/drivers/infiniband/core/uverbs_main.c > +++ b/drivers/infiniband/core/uverbs_main.c > @@ -122,7 +122,8 @@ static int (*uverbs_ex_cmd_table[])(struct ib_uverbs_file *file, > struct ib_udata *ucore, > struct ib_udata *uhw) = { > [IB_USER_VERBS_EX_CMD_CREATE_FLOW] = ib_uverbs_ex_create_flow, > - [IB_USER_VERBS_EX_CMD_DESTROY_FLOW] = ib_uverbs_ex_destroy_flow > + [IB_USER_VERBS_EX_CMD_DESTROY_FLOW] = ib_uverbs_ex_destroy_flow, > + [IB_USER_VERBS_EX_CMD_QUERY_DEVICE] = ib_uverbs_ex_query_device > }; > > static void ib_uverbs_add_one(struct ib_device *device); > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index 470a011d6fa4..97a999f9e4d8 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -1662,7 +1662,10 @@ static inline int ib_copy_from_udata(void *dest, struct ib_udata *udata, size_t > > static inline int ib_copy_to_udata(struct ib_udata *udata, void *src, size_t len) > { > - return copy_to_user(udata->outbuf, src, len) ? -EFAULT : 0; > + size_t copy_sz; > + > + copy_sz = min_t(size_t, len, udata->outlen); > + return copy_to_user(udata->outbuf, src, copy_sz) ? -EFAULT : 0; > } This is not the place to do this: as I'm guessing the purpose of this change from the patch in '[PATCH v3 07/17] IB/core: Add flags for on demand paging support', you're trying to handle uverbs call from a userspace program using a previous, shorter ABI. But that's hidding bug where userspace will get it wrong at passing the correct buffer / size for all others uverb calls. That cannot work that way. In a previous patchset [1], I've suggested to add a check in ib_copy_{from,to}_udata()[2][3] in order to check the input/output buffer size to not read/write past userspace provided buffer boundaries: in case of mismatch an error would be returned to userspace. With the suggested change here, buffer overflow won't happen, but the error is silently ignored, allowing uverb to return a partial result, which is likely not expected by userspace as it's a bit difficult to handle it gracefully. So this has to be removed, and a check on userspace response buffer must be added to ib_uverbs_ex_query_device() instead. [1] "[PATCH 00/22] infiniband: improve userspace input check" http://marc.info/?i=cover.1376847403.git.ydroneaud@xxxxxxxxxx http://mid.gmane.org/cover.1376847403.git.ydroneaud@xxxxxxxxxx [2] "[PATCH 03/22] infiniband: ib_copy_from_udata(): check input length" http://mid.gmane.org/2bf102a41c51f61965ee09df827abe8fefb523a9.1376847403.git.ydroneaud@xxxxxxxxxx [3] "[PATCH 04/22] infiniband: ib_copy_to_udata(): check output length" http://mid.gmane.org/d27716a3a1c180f832d153a7402f65ea8a75b734.1376847403.git.ydroneaud@xxxxxxxxxx Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html