Re: [Qemu-devel] [PATCH v2 3/4] hw/rdma: Modify create/destroy QP to support SRQ

[Yuval Shaia <yuval.shaia@oracle.com>]

> >>  
> >> @@ -525,16 +539,21 @@ static int destroy_qp(PVRDMADev *dev, union pvrdma_cmd_req *req,
> >>      struct pvrdma_cmd_destroy_qp *cmd = &req->destroy_qp;
> >>      RdmaRmQP *qp;
> >>      PvrdmaRing *ring;
> >> +    uint8_t is_srq = 0;
> >>  
> >>      qp = rdma_rm_get_qp(&dev->rdma_dev_res, cmd->qp_handle);
> >>      if (!qp) {
> >>          return -EINVAL;
> >>      }
> >>  
> >> +    if (qp->is_srq) {
> >> +        is_srq = 1;
> >> +    }
> >> +
> > 
> > [1]
> > 
> >>      rdma_rm_dealloc_qp(&dev->rdma_dev_res, cmd->qp_handle);
> > 
> > [2]
> > 
> >>  
> >>      ring = (PvrdmaRing *)qp->opaque;
> > 
> > [3]
> > 
> >> -    destroy_qp_rings(ring);
> >> +    destroy_qp_rings(ring, is_srq);
> > 
> > Better move the call to rdma_rm_dealloc_qp ([2]) to here and get rid of the
> > block in [1].
> > 
> > In any case, the code in [3] looks like a bug to me (an existing bug), i.e.
> > qp pointer cannot be trusted after call to rdma_rm_dealloc_qp (use after
> > free).
> > What do you think?
> 
> You are right, I'll rearrange the code in v3.

Thanks just please add note for that in the commit message as you are
fixing an issue not related to SRQ.

> 
> > 
> >>  
> >>      return 0;
> >>  }
> >> -- 
> >> 2.20.1
> >>
> >>
>