On 27/04/2018 18:01, Peter Maydell wrote:
> 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.
>
> 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".
Will fix, thanks for finding it!
Marcel
>
>> + }
>> +
>> +out:
>> + pr_dbg("ret=%d\n", resp->hdr.err);
>> + return resp->hdr.err;
>> +}
>
> thanks
> -- PMM
>
