On 12/20/24 10:06 AM, Cheng Xu wrote: > > > On 12/20/24 3:02 AM, Kees Bakker wrote: >> Hi, >> >> As identified by Coverity Scan (CID 1602609) there is a potential >> use of a NULL pointer. >> >> It was introduced in commit 6534de1fe385 >> Author: Cheng Xu <chengyou@xxxxxxxxxxxxxxxxx> >> Date: Tue Jun 6 13:50:04 2023 +0800 >> >> RDMA/erdma: Associate QPs/CQs with doorbells for authorization >> >> For the isolation requirement, each QP/CQ can only issue doorbells from the >> allocated mmio space. Configure the relationship between QPs/CQs and >> mmio doorbell spaces to hardware in create_qp/create_cq interfaces. >> >> Signed-off-by: Cheng Xu <chengyou@xxxxxxxxxxxxxxxxx> >> Link: https://lore.kernel.org/r/20230606055005.80729-4-chengyou@xxxxxxxxxxxxxxxxx >> Signed-off-by: Leon Romanovsky <leon@xxxxxxxxxx> >> >> int erdma_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs, >> struct ib_udata *udata) >> { >> struct erdma_qp *qp = to_eqp(ibqp); >> struct erdma_dev *dev = to_edev(ibqp->device); >> struct erdma_ucontext *uctx = rdma_udata_to_drv_context( >> udata, struct erdma_ucontext, ibucontext); >> [...] >> if (uctx) { >> ret = ib_copy_from_udata(&ureq, udata, >> min(sizeof(ureq), udata->inlen)); >> [...] >> } else { >> init_kernel_qp(dev, qp, attrs); >> } >> >> qp->attrs.max_send_sge = attrs->cap.max_send_sge; >> qp->attrs.max_recv_sge = attrs->cap.max_recv_sge; >> >> if (erdma_device_iwarp(qp->dev)) >> qp->attrs.iwarp.state = ERDMA_QPS_IWARP_IDLE; >> else >> qp->attrs.rocev2.state = ERDMA_QPS_ROCEV2_RESET; >> >> INIT_DELAYED_WORK(&qp->reflush_dwork, erdma_flush_worker); >> >> ret = create_qp_cmd(uctx, qp); >> [...] >> This shows that `uctx` can be NULL. The problem is that `uctx` can be >> dereferenced in create_qp_cmd(). There is no guard against NULL. >> >> Can one of you have a look and say that it OK to potential pass a >> NULL pointer in `uctx`? >> > > static int create_qp_cmd(struct erdma_ucontext *uctx, struct erdma_qp *qp) > { > [...] > > if (rdma_is_kernel_res(&qp->ibqp.res)) { > [...] > } else { > // uctx used here... > } > > [...] > } > > When uctx == NULL, rdma_is_kernel_res() will always return false and uctx will not be ^^^^^ true > dereferenced. So the current implementation is OK. > Sorry for this typo. Cheng Xu > Thanks, > Cheng Xu > > >> Kind regards, >> Kees Bakker
