Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 02 Oct 2019 14:22:14 -0700 (PDT)
Message-Id: <20191002.142214.252882219207569928.davem@davemloft.net>
To:     yzhai003@ucr.edu
Cc:     csong@cs.ucr.edu, zhiyunq@cs.ucr.edu, yisen.zhuang@huawei.com,
        salil.mehta@huawei.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: hisilicon: Fix usage of uninitialized variable in
 function mdio_sc_cfg_reg_write()
From:   David Miller <davem@davemloft.net>
In-Reply-To: <20191001202439.15766-1-yzhai003@ucr.edu>
References: <20191001202439.15766-1-yzhai003@ucr.edu>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

From: Yizhuo <yzhai003@ucr.edu>
Date: Tue,  1 Oct 2019 13:24:39 -0700

> In function mdio_sc_cfg_reg_write(), variable "reg_value" could be
> uninitialized if regmap_read() fails. However, "reg_value" is used
> to decide the control flow later in the if statement, which is
> potentially unsafe.
> 
> Signed-off-by: Yizhuo <yzhai003@ucr.edu>

Applied, but really this is such a pervasive problem.

So much code doesn't check the return value from either regmap_read
or regmap_write.

_EVEN_ in the code you are editing, the patch context shows an unchecked
regmap_write() call.

> @@ -148,11 +148,15 @@ static int mdio_sc_cfg_reg_write(struct hns_mdio_device *mdio_dev,
>  {
>  	u32 time_cnt;
>  	u32 reg_value;
> +	int ret;
>  
>  	regmap_write(mdio_dev->subctrl_vbase, cfg_reg, set_val);
        ^^^^^^^^^^^^

Grepping for regmap_{read,write}() shows how big an issue this is.

I don't know what to do, maybe we can work over time to add checks to
all calls and then force warnings on unchecked return values so that
the problem is not introduced in the future.