Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

RDMA/rtrs-srv: Do not pass a valid pointer to PTR_ERR()

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.wang@cloud.ionos.com
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

authored by

Jack Wang and committed by
Jason Gunthorpe
ed408529 2b5715fc

+7 -13
+7 -13
drivers/infiniband/ulp/rtrs/rtrs-srv.c
··· 1347 1347 return srv; 1348 1348 } 1349 1349 } 1350 + mutex_unlock(&ctx->srv_mutex); 1350 1351 /* 1351 1352 * If this request is not the first connection request from the 1352 1353 * client for this session then fail and return error. 1353 1354 */ 1354 - if (!first_conn) { 1355 - mutex_unlock(&ctx->srv_mutex); 1355 + if (!first_conn) 1356 1356 return ERR_PTR(-ENXIO); 1357 - } 1358 1357 1359 1358 /* need to allocate a new srv */ 1360 1359 srv = kzalloc(sizeof(*srv), GFP_KERNEL); 1361 - if (!srv) { 1362 - mutex_unlock(&ctx->srv_mutex); 1360 + if (!srv) 1363 1361 return ERR_PTR(-ENOMEM); 1364 - } 1365 1362 1366 1363 INIT_LIST_HEAD(&srv->paths_list); 1367 1364 mutex_init(&srv->paths_mutex); ··· 1368 1371 srv->ctx = ctx; 1369 1372 device_initialize(&srv->dev); 1370 1373 srv->dev.release = rtrs_srv_dev_release; 1371 - list_add(&srv->ctx_list, &ctx->srv_list); 1372 - mutex_unlock(&ctx->srv_mutex); 1373 1374 1374 1375 srv->chunks = kcalloc(srv->queue_depth, sizeof(*srv->chunks), 1375 1376 GFP_KERNEL); ··· 1380 1385 goto err_free_chunks; 1381 1386 } 1382 1387 refcount_set(&srv->refcount, 1); 1388 + mutex_lock(&ctx->srv_mutex); 1389 + list_add(&srv->ctx_list, &ctx->srv_list); 1390 + mutex_unlock(&ctx->srv_mutex); 1383 1391 1384 1392 return srv; 1385 1393 ··· 1797 1799 } 1798 1800 recon_cnt = le16_to_cpu(msg->recon_cnt); 1799 1801 srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn); 1800 - /* 1801 - * "refcount == 0" happens if a previous thread calls get_or_create_srv 1802 - * allocate srv, but chunks of srv are not allocated yet. 1803 - */ 1804 - if (IS_ERR(srv) || refcount_read(&srv->refcount) == 0) { 1802 + if (IS_ERR(srv)) { 1805 1803 err = PTR_ERR(srv); 1806 1804 goto reject_w_err; 1807 1805 }