From: Jack Wang <jinpu.w...@cloud.ionos.com>

[ Upstream commit ed408529679737a9a7ad816c8de5d59ba104bb11 ]

smatch gives the warning:

  drivers/infiniband/ulp/rtrs/rtrs-srv.c:1805 rtrs_rdma_connect() warn: passing 
zero to 'PTR_ERR'

Which is trying to say smatch has shown that srv is not an error pointer
and thus cannot be passed to PTR_ERR.

The solution is to move the list_add() down after full initilization of
rtrs_srv. To avoid holding the srv_mutex too long, only hold it during the
list operation as suggested by Leon.

Fixes: 03e9b33a0fd6 ("RDMA/rtrs: Only allow addition of path to an already 
established session")
Link: 
https://lore.kernel.org/r/20210216143807.65923-1-jinpu.w...@cloud.ionos.com
Reported-by: kernel test robot <l...@intel.com>
Reported-by: Dan Carpenter <dan.carpen...@oracle.com>
Signed-off-by: Jack Wang <jinpu.w...@cloud.ionos.com>
Signed-off-by: Jason Gunthorpe <j...@nvidia.com>
Signed-off-by: Sasha Levin <sas...@kernel.org>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c 
b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 717304c49d0c3..f009a6907169c 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1364,21 +1364,18 @@ static struct rtrs_srv *get_or_create_srv(struct 
rtrs_srv_ctx *ctx,
                        return srv;
                }
        }
+       mutex_unlock(&ctx->srv_mutex);
        /*
         * If this request is not the first connection request from the
         * client for this session then fail and return error.
         */
-       if (!first_conn) {
-               mutex_unlock(&ctx->srv_mutex);
+       if (!first_conn)
                return ERR_PTR(-ENXIO);
-       }
 
        /* need to allocate a new srv */
        srv = kzalloc(sizeof(*srv), GFP_KERNEL);
-       if  (!srv) {
-               mutex_unlock(&ctx->srv_mutex);
+       if  (!srv)
                return ERR_PTR(-ENOMEM);
-       }
 
        INIT_LIST_HEAD(&srv->paths_list);
        mutex_init(&srv->paths_mutex);
@@ -1388,8 +1385,6 @@ static struct rtrs_srv *get_or_create_srv(struct 
rtrs_srv_ctx *ctx,
        srv->ctx = ctx;
        device_initialize(&srv->dev);
        srv->dev.release = rtrs_srv_dev_release;
-       list_add(&srv->ctx_list, &ctx->srv_list);
-       mutex_unlock(&ctx->srv_mutex);
 
        srv->chunks = kcalloc(srv->queue_depth, sizeof(*srv->chunks),
                              GFP_KERNEL);
@@ -1402,6 +1397,9 @@ static struct rtrs_srv *get_or_create_srv(struct 
rtrs_srv_ctx *ctx,
                        goto err_free_chunks;
        }
        refcount_set(&srv->refcount, 1);
+       mutex_lock(&ctx->srv_mutex);
+       list_add(&srv->ctx_list, &ctx->srv_list);
+       mutex_unlock(&ctx->srv_mutex);
 
        return srv;
 
@@ -1816,11 +1814,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
        }
        recon_cnt = le16_to_cpu(msg->recon_cnt);
        srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
-       /*
-        * "refcount == 0" happens if a previous thread calls get_or_create_srv
-        * allocate srv, but chunks of srv are not allocated yet.
-        */
-       if (IS_ERR(srv) || refcount_read(&srv->refcount) == 0) {
+       if (IS_ERR(srv)) {
                err = PTR_ERR(srv);
                goto reject_w_err;
        }
-- 
2.27.0



Reply via email to