On 19 February 2018 at 11:43, Marcel Apfelbaum <mar...@redhat.com> wrote:
> From: Yuval Shaia <yuval.sh...@oracle.com>
>
> First PVRDMA sub-module - implementation of the PVRDMA device.
> - PVRDMA commands such as create CQ and create MR.
> - Data path QP operations - post_send and post_recv.
> - Completion handler.
>
> Reviewed-by: Dotan Barak <dot...@mellanox.com>
> Reviewed-by: Zhu Yanjun <yanjun....@oracle.com>
> Signed-off-by: Yuval Shaia <yuval.sh...@oracle.com>
> Signed-off-by: Marcel Apfelbaum <mar...@redhat.com>
Hi; Coverity points out an array bounds overrun in this code:
> +static int create_bind(PVRDMADev *dev, union pvrdma_cmd_req *req,
> + union pvrdma_cmd_resp *rsp)
> +{
> + struct pvrdma_cmd_create_bind *cmd = &req->create_bind;
> +#ifdef PVRDMA_DEBUG
> + __be64 *subnet = (__be64 *)&cmd->new_gid[0];
> + __be64 *if_id = (__be64 *)&cmd->new_gid[8];
> +#endif
> +
> + pr_dbg("index=%d\n", cmd->index);
> +
> + if (cmd->index > MAX_PORT_GIDS) {
> + return -EINVAL;
> + }
This bounds check allows cmd->index == MAX_PORT_GIDS...
> +
> + pr_dbg("gid[%d]=0x%llx,0x%llx\n", cmd->index,
> + (long long unsigned int)be64_to_cpu(*subnet),
> + (long long unsigned int)be64_to_cpu(*if_id));
> +
> + /* Driver forces to one port only */
> + memcpy(dev->rdma_dev_res.ports[0].gid_tbl[cmd->index].raw, &cmd->new_gid,
> + sizeof(cmd->new_gid));
...but the gid_tbl[] array we index into is declared with
union ibv_gid gid_tbl[MAX_PORT_GIDS];
so using MAX_PORT_GIDS as an index is off the end of it.
Presumably the check should be ">=".
> +static int destroy_bind(PVRDMADev *dev, union pvrdma_cmd_req *req,
> + union pvrdma_cmd_resp *rsp)
> +{
> + struct pvrdma_cmd_destroy_bind *cmd = &req->destroy_bind;
> +
> + pr_dbg("clear index %d\n", cmd->index);
> +
> + memset(dev->rdma_dev_res.ports[0].gid_tbl[cmd->index].raw, 0,
> + sizeof(dev->rdma_dev_res.ports[0].gid_tbl[cmd->index].raw));
I'm assuming this function can't be called unless create_bind()
has previously succeeded and so it doesn't need its own
bounds check.
> +
> + return 0;
> +}
thanks
-- PMM
