linux-kernel - Re: [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 17 Aug 2020 22:29:57 -0700
From:   Nathan Chancellor <natechancellor@...il.com>
To:     Brooke Basile <brookebasile@...il.com>
Cc:     danil.kipnis@...ud.ionos.com, jinpu.wang@...ud.ionos.com,
        axboe@...nel.dk, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, clang-built-linux@...glegroups.com
Subject: Re: [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable
 warning

On Tue, Aug 18, 2020 at 12:03:18AM -0400, Brooke Basile wrote:
> Clang warns:
> 	drivers/block/rnbd/rnbd-srv.c:150:6: warning: variable 'err' is used
> 	uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>         	if (IS_ERR(bio)) {
>             	^~~~~~~~~~~
> 	drivers/block/rnbd/rnbd-srv.c:177:9: note: uninitialized use occurs here
> 		return err;
> 		^~~
> 	drivers/block/rnbd/rnbd-srv.c:126:9: note: initialize the variable 'err'
> 	to silence this warning
>         	int err;
>                	^
>                 	= 0
> 
> Silence this by replacing `err` with `ret`, returning ret = 0 upon
> success.
> 
> Signed-off-by: Brooke Basile <brookebasile@...il.com>
> ---
>  drivers/block/rnbd/rnbd-srv.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index 0fb94843a495..f515d1a048a9 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -123,10 +123,10 @@ static int process_rdma(struct rtrs_srv *sess,
>  	struct rnbd_io_private *priv;
>  	struct rnbd_srv_sess_dev *sess_dev;
>  	u32 dev_id;
> -	int err;
>  	struct rnbd_dev_blk_io *io;
>  	struct bio *bio;
>  	short prio;
> +	int ret = 0;
>  
>  	priv = kmalloc(sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
> @@ -138,7 +138,7 @@ static int process_rdma(struct rtrs_srv *sess,
>  	if (IS_ERR(sess_dev)) {
>  		pr_err_ratelimited("Got I/O request on session %s for unknown device id %d\n",
>  				   srv_sess->sessname, dev_id);
> -		err = -ENOTCONN;
> +		ret = -ENOTCONN;
>  		goto err;
>  	}
>  
> @@ -168,13 +168,13 @@ static int process_rdma(struct rtrs_srv *sess,
>  
>  	submit_bio(bio);
>  
> -	return 0;
> +	return ret;
>  
>  sess_dev_put:
>  	rnbd_put_sess_dev(sess_dev);
>  err:
>  	kfree(priv);
> -	return err;
> +	return ret;
>  }
>  
>  static void destroy_device(struct rnbd_srv_dev *dev)
> -- 
> 2.28.0
> 

I don't think this is a proper fix since the root cause of the warning
appears to be that we are ignoring the return value of
rnbd_bio_map_kern. Should we not set err to that value like this
(completely untested)?

Cheers,
Nathan

diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index 0fb94843a495..1b71cb2a885d 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -148,7 +148,8 @@ static int process_rdma(struct rtrs_srv *sess,
 	/* Generate bio with pages pointing to the rdma buffer */
 	bio = rnbd_bio_map_kern(data, sess_dev->rnbd_dev->ibd_bio_set, datalen, GFP_KERNEL);
 	if (IS_ERR(bio)) {
-		rnbd_srv_err(sess_dev, "Failed to generate bio, err: %ld\n", PTR_ERR(bio));
+		err = PTR_ERR(bio);
+		rnbd_srv_err(sess_dev, "Failed to generate bio, err: %ld\n", err);
 		goto sess_dev_put;
 	}