Re: [PATCH] RDS: IB: ensure an initialized ret is printed in pr_warn message

[Santosh Shilimkar]

On 4/7/2017 7:24 AM, Dan Carpenter wrote:

Setting it to zero doesn't seem like the right thing, it should be an
error code.  Oh, heh...  Smatch parses this one correctly.  "ret" is
always initialized but the code is probably buggy in a different way:



[...]


   561  ibmr = rds_ib_reg_fmr(rds_ibdev, sg, nents, key_ret);
   562  if (ibmr)

This condition is always true because those functions return ERR_PTRs
not NULLs.


Thanks Dan. I will fix that up.

Regards,
Santosh

Re: [PATCH] RDS: IB: ensure an initialized ret is printed in pr_warn message

[Dan Carpenter]

Setting it to zero doesn't seem like the right thing, it should be an
error code.  Oh, heh...  Smatch parses this one correctly.  "ret" is
always initialized but the code is probably buggy in a different way:

net/rds/ib_rdma.c
   539  void *rds_ib_get_mr(struct scatterlist *sg, unsigned long nents,
   540  struct rds_sock *rs, u32 *key_ret)
   541  {
   542  struct rds_ib_device *rds_ibdev;
   543  struct rds_ib_mr *ibmr = NULL;
   544  struct rds_ib_connection *ic = rs->rs_conn->c_transport_data;
   545  int ret;
   546  
   547  rds_ibdev = rds_ib_get_device(rs->rs_bound_addr);
   548  if (!rds_ibdev) {
   549  ret = -ENODEV;
   550  goto out;
   551  }
   552  
   553  if (!rds_ibdev->mr_8k_pool || !rds_ibdev->mr_1m_pool) {
   554  ret = -ENODEV;
   555  goto out;
   556  }
   557  
   558  if (rds_ibdev->use_fastreg)
   559  ibmr = rds_ib_reg_frmr(rds_ibdev, ic, sg, nents, 
key_ret);
   560  else
   561  ibmr = rds_ib_reg_fmr(rds_ibdev, sg, nents, key_ret);
   562  if (ibmr)

This condition is always true because those functions return ERR_PTRs
not NULLs.

   563  rds_ibdev = NULL;
   564  
   565   out:
   566  if (!ibmr)
^
This condition implies that "ret" is set to an error code.

   567  pr_warn("RDS/IB: rds_ib_get_mr failed (errno=%d)\n", 
ret);
   568  
   569  if (rds_ibdev)
   570  rds_ib_dev_put(rds_ibdev);
   571  
   572  return ibmr;
   573  }

regards,
dan carpenter

Re: [PATCH] RDS: IB: ensure an initialized ret is printed in pr_warn message

[David Miller]

From: Colin King 
Date: Fri,  7 Apr 2017 08:57:23 +0100

> From: Colin Ian King 
> 
> There is a path where ibmr is null and ret has not been initialized
> and hence a pr_warn message is printing an uninitialized value in
> ret.  Fix this by initializing ret to zero.
> 
> Detected by CoverityScan, CID#1357946 ("Uninitialized scalar variable")
> 
> Signed-off-by: Colin Ian King 

These are exactly the kinds of CoverityScan fixes I really do not want
to see.

Initializing ret to zero is not going to fix the problem.

This function gets error pointers back from the functions that are
used to obtain the ibmr pointer.  Therefore if there is a problem
ibmr won't be NULL, it will be an error pointer.

Therefore, the real problem is that the code isn't checking if
ibmr encodes error value.

[PATCH] RDS: IB: ensure an initialized ret is printed in pr_warn message

[Colin King]

From: Colin Ian King 

There is a path where ibmr is null and ret has not been initialized
and hence a pr_warn message is printing an uninitialized value in
ret.  Fix this by initializing ret to zero.

Detected by CoverityScan, CID#1357946 ("Uninitialized scalar variable")

Signed-off-by: Colin Ian King 
---
 net/rds/ib_rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rds/ib_rdma.c b/net/rds/ib_rdma.c
index 977f69886c00..4fd637491dd5 100644
--- a/net/rds/ib_rdma.c
+++ b/net/rds/ib_rdma.c
@@ -542,7 +542,7 @@ void *rds_ib_get_mr(struct scatterlist *sg, unsigned long 
nents,
struct rds_ib_device *rds_ibdev;
struct rds_ib_mr *ibmr = NULL;
struct rds_ib_connection *ic = rs->rs_conn->c_transport_data;
-   int ret;
+   int ret = 0;
 
rds_ibdev = rds_ib_get_device(rs->rs_bound_addr);
if (!rds_ibdev) {
-- 
2.11.0