> On Apr 30, 2021, at 2:26 PM, coverity-bot <keescook@xxxxxxxxxxxx> wrote: > > Hello! > > This is an experimental semi-automated report about issues detected by > Coverity from a scan of next-20210430 as part of the linux-next scan project: > https://scan.coverity.com/projects/linux-next-weekly-scan > > You're getting this email because you were associated with the identified > lines of code (noted below) that were touched by commits: > > Mon Apr 26 09:27:06 2021 -0400 > 9a301cafc861 ("xprtrdma: Move fr_linv_done field to struct rpcrdma_mr") > > Coverity reported the following: > > *** CID 1504556: Null pointer dereferences (FORWARD_NULL) > /net/sunrpc/xprtrdma/frwr_ops.c: 539 in frwr_unmap_sync() > 533 > 534 /* Strong send queue ordering guarantees that when the > 535 * last WR in the chain completes, all WRs in the chain > 536 * are complete. > 537 */ > 538 last->wr_cqe->done = frwr_wc_localinv_wake; > vvv CID 1504556: Null pointer dereferences (FORWARD_NULL) > vvv Passing null pointer "&mr->mr_linv_done" to "reinit_completion", which dereferences it. > 539 reinit_completion(&mr->mr_linv_done); > 540 > 541 /* Transport disconnect drains the receive CQ before it > 542 * replaces the QP. The RPC reply handler won't call us > 543 * unless re_id->qp is a valid pointer. > 544 */ > > If this is a false positive, please let us know so we can mark it as > such, or teach the Coverity rules to be smarter. Sure, not my proudest moment here. The sole call site for frwr_unmap_sync() is this one: net/sunrpc/xprtrdma/transport.c: 606 if (unlikely(!list_empty(&req->rl_registered))) { 607 trace_xprtrdma_mrs_zap(task); 608 frwr_unmap_sync(rpcx_to_rdmax(rqst->rq_xprt), req); 609 } Thus, in the current code base, the while() loop: net/sunrpc/xprtrdma/frwr_ops.c: 514 while ((mr = rpcrdma_mr_pop(&req->rl_registered))) { Should always terminate with mr containing a non-NULL address. Seems to me the frwr_unmap_sync() code before fdf5ecb1934b ("xprtrdma: Move fr_linv_done field to struct rpcrdma_mr") has the same risk -- frwr can be NULL if rl_registered is empty. I'm open to suggestions for improvement, but I'm not seeing this rise to the level of a pervasive and high impact issue. > If not, please make > sure fixes get into linux-next. :) For patches fixing this, please > include these lines (but double-check the "Fixes" first): > > Reported-by: coverity-bot <keescook+coverity-bot@xxxxxxxxxxxx> > Addresses-Coverity-ID: 1504556 ("Null pointer dereferences") > Fixes: 9a301cafc861 ("xprtrdma: Move fr_linv_done field to struct rpcrdma_mr") > > Thanks for your attention! > > -- > Coverity-bot -- Chuck Lever
