* [PATCH] RDS: IB: ensure an initialized ret is printed in pr_warn message
@ 2017-04-07 7:57 ` Colin King
0 siblings, 0 replies; 9+ messages in thread
From: Colin King @ 2017-04-07 7:57 UTC (permalink / raw)
To: Santosh Shilimkar, David S . Miller, netdev, linux-rdma,
rds-devel
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
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 <colin.king@canonical.com>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH] RDS: IB: ensure an initialized ret is printed in pr_warn message
@ 2017-04-07 7:57 ` Colin King
0 siblings, 0 replies; 9+ messages in thread
From: Colin King @ 2017-04-07 7:57 UTC (permalink / raw)
To: Santosh Shilimkar, David S . Miller,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
rds-devel-N0ozoZBvEnrZJqsBc5GL+g
Cc: kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
From: Colin Ian King <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
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 <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] RDS: IB: ensure an initialized ret is printed in pr_warn message
2017-04-07 7:57 ` Colin King
@ 2017-04-07 13:10 ` David Miller
-1 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2017-04-07 13:10 UTC (permalink / raw)
To: colin.king
Cc: santosh.shilimkar, netdev, linux-rdma, rds-devel, kernel-janitors,
linux-kernel
From: Colin King <colin.king@canonical.com>
Date: Fri, 7 Apr 2017 08:57:23 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> 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 <colin.king@canonical.com>
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.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] RDS: IB: ensure an initialized ret is printed in pr_warn message
@ 2017-04-07 13:10 ` David Miller
0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2017-04-07 13:10 UTC (permalink / raw)
To: colin.king
Cc: santosh.shilimkar, netdev, linux-rdma, rds-devel, kernel-janitors,
linux-kernel
From: Colin King <colin.king@canonical.com>
Date: Fri, 7 Apr 2017 08:57:23 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> 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 <colin.king@canonical.com>
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] RDS: IB: ensure an initialized ret is printed in pr_warn message
2017-04-07 7:57 ` Colin King
@ 2017-04-07 14:24 ` Dan Carpenter
-1 siblings, 0 replies; 9+ messages in thread
From: Dan Carpenter @ 2017-04-07 14:24 UTC (permalink / raw)
To: Colin King, Santosh Shilimkar
Cc: David S . Miller, netdev, linux-rdma, rds-devel, kernel-janitors,
linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] RDS: IB: ensure an initialized ret is printed in pr_warn message
@ 2017-04-07 14:24 ` Dan Carpenter
0 siblings, 0 replies; 9+ messages in thread
From: Dan Carpenter @ 2017-04-07 14:24 UTC (permalink / raw)
To: Colin King, Santosh Shilimkar
Cc: David S . Miller, netdev, linux-rdma, rds-devel, kernel-janitors,
linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] RDS: IB: ensure an initialized ret is printed in pr_warn message
2017-04-07 14:24 ` Dan Carpenter
@ 2017-04-07 15:59 ` Santosh Shilimkar
-1 siblings, 0 replies; 9+ messages in thread
From: Santosh Shilimkar @ 2017-04-07 15:59 UTC (permalink / raw)
To: Dan Carpenter, Colin King
Cc: David S . Miller, netdev, linux-rdma, rds-devel, kernel-janitors,
linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] RDS: IB: ensure an initialized ret is printed in pr_warn message
@ 2017-04-07 15:59 ` Santosh Shilimkar
0 siblings, 0 replies; 9+ messages in thread
From: Santosh Shilimkar @ 2017-04-07 15:59 UTC (permalink / raw)
To: Dan Carpenter, Colin King
Cc: David S . Miller, netdev, linux-rdma, rds-devel, kernel-janitors,
linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread