Re: [Qemu-devel] [PATCH PULL v2 08/10] hw/rdma: PVRDMA commands and data-path ops

[Peter Maydell]

On 19 February 2018 at 11:43, Marcel Apfelbaum <address@hidden> wrote:
> From: Yuval Shaia <address@hidden>
>
> 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.

Coverity CID 1390620: we call munmap() on a NULL pointer.

> +static int create_mr(PVRDMADev *dev, union pvrdma_cmd_req *req,
> +                     union pvrdma_cmd_resp *rsp)
> +{
> +    struct pvrdma_cmd_create_mr *cmd = &req->create_mr;
> +    struct pvrdma_cmd_create_mr_resp *resp = &rsp->create_mr_resp;
> +    PCIDevice *pci_dev = PCI_DEVICE(dev);
> +    void *host_virt = NULL;

Here we set host_virt to NULL...

> +
> +    memset(resp, 0, sizeof(*resp));
> +    resp->hdr.response = cmd->hdr.response;
> +    resp->hdr.ack = PVRDMA_CMD_CREATE_MR_RESP;
> +
> +    pr_dbg("pd_handle=%d\n", cmd->pd_handle);
> +    pr_dbg("access_flags=0x%x\n", cmd->access_flags);
> +    pr_dbg("flags=0x%x\n", cmd->flags);
> +
> +    if (!(cmd->flags & PVRDMA_MR_FLAG_DMA)) {

...and if we don't take this if() we won't set host_virt to anything...

> +        host_virt = pvrdma_map_to_pdir(pci_dev, cmd->pdir_dma, cmd->nchunks,
> +                                       cmd->length);
> +        if (!host_virt) {
> +            pr_dbg("Failed to map to pdir\n");
> +            resp->hdr.err = -EINVAL;
> +            goto out;
> +        }
> +    }
> +
> +    resp->hdr.err = rdma_rm_alloc_mr(&dev->rdma_dev_res, cmd->pd_handle,
> +                                     cmd->start, cmd->length, host_virt,
> +                                     cmd->access_flags, &resp->mr_handle,
> +                                     &resp->lkey, &resp->rkey);
> +    if (!resp->hdr.err) {
> +        munmap(host_virt, cmd->length);

...but here we call munmap() on it without checking if it is NULL.
Unlike g_free(), munmap() isn't specified to be "do nothing if
passed a NULL pointer".

> +    }
> +
> +out:
> +    pr_dbg("ret=%d\n", resp->hdr.err);
> +    return resp->hdr.err;
> +}

thanks
-- PMM