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;
IronPort-SDR: 7yFG385H+NjQx2c4MMYsQvpX9JygRFPnAVIFczh9dl91etTnNR10q2cKXSsOErM7p1ohJJftM1
 xzQxL7db6q8wxRZJ0x6RuD9LSEpIRSJTpyd0pr+UNYc34K06K8YLuVqaY7BK9hC7k50VtyglKg
 rPIWy1089fq/7ZzCmon7m6h4k1O99siPBzHEHdr6PCqC7ZYYb3zcUs26Y2vdsPdhl8DgAsFi9s
 ZPrcQSWVdGxhgKilpGR9vzQNZKrqr/yxfysxUlO/UxHLxRVp4ChFCtXClS4TJCA3Zy9ivAuJzj
 md4=
IronPort-PHdr: =?us-ascii?q?9a23=3ARqJfGRNN92krWgkYiTMl6mtUPXoX/o7sNwtQ0K?=
 =?us-ascii?q?IMzox0LfT5rarrMEGX3/hxlliBBdydt6sfzbaG+Pm5AiQp2tWoiDg6aptCVh?=
 =?us-ascii?q?sI2409vjcLJ4q7M3D9N+PgdCcgHc5PBxdP9nC/NlVJSo6lPwWB6nK94iQPFR?=
 =?us-ascii?q?rhKAF7Ovr6GpLIj8Swyuu+54Dfbx9HiTagb75+Nhq7oAbeusULnIdvJLs6xw?=
 =?us-ascii?q?fUrHdPZ+lY335jK0iJnxb76Mew/Zpj/DpVtvk86cNOUrj0crohQ7BAAzsoL2?=
 =?us-ascii?q?465MvwtRneVgSP/WcTUn8XkhVTHQfI6gzxU4rrvSv7sup93zSaPdHzQLspVz?=
 =?us-ascii?q?mu87tnRRn1gyocKTU37H/YhdBxjKJDoRKuuRp/w5LPYIqIMPZyZ77Rcc8GSW?=
 =?us-ascii?q?ZEWMtaSi5PDZ6mb4YXAOUBM+RXoYnzqVUNsBWwGxWjCfjzyjNUnHL6wbE23/?=
 =?us-ascii?q?gjHAzAwQcuH8gOsHPRrNjtNqgSUOG0zKnVzTXEcvhZ2jf955LJchs8pvyNXb?=
 =?us-ascii?q?NxccrLxkkuCw/JkludpJf4PzyJzOQBqXaU4Pd9Ve+2jWMstgJ/oiC3y8sylo?=
 =?us-ascii?q?XEgpgZx1PE+Clj3oo5Od61RFRlbdK4DJddsTyROZFsTcM4WW5ovT43yrgBuZ?=
 =?us-ascii?q?GmYicH0I8nxxvDa/yfdIiI/w7jWP6RIThmgHJlf6qyhxOo/kihzu3wT8200F?=
 =?us-ascii?q?RXoiZcnNnAqGwB2wDJ5siITft9+Uih2TKR2AzJ9u5EJkU0mbLaK54n3LEwio?=
 =?us-ascii?q?IevVrfEiLygkn7j6+bel869uS06OnreKvqqoOAO4NsjwHxKKUumsixAeQiNQ?=
 =?us-ascii?q?gOWnCW+OS91b3j50L5QalGguE4n6TCrZDVOd4bqrSnDABIz4Yv8wy/ACu+0N?=
 =?us-ascii?q?QEgXkHK0pIeBaGj4jvJlHPL+n0DfShjFS2ljdk2fTGM6b/ApXCMHfDiq3tfb?=
 =?us-ascii?q?Vj5E5Gzgo809Rf64hTCrEbL/KgEnP24fDREB41eym1x+LqEp0p2ooAVGenDq?=
 =?us-ascii?q?aHPabWtlGUoOQiP7/fSpUSvWPMKuol+vmmv38wmBdJbLup1JpPMCuQA/98ZU?=
 =?us-ascii?q?iVfCy/0Z86DW4Ws19mH6TRg1qYXGsWPi7qUg=3D=3D?=
MIME-Version: 1.0
References: <20191001202439.15766-1-yzhai003@ucr.edu> <20191002.142214.252882219207569928.davem@davemloft.net>
In-Reply-To: <20191002.142214.252882219207569928.davem@davemloft.net>
From:   Yizhuo Zhai <yzhai003@ucr.edu>
Date:   Thu, 3 Oct 2019 10:42:58 -0700
Message-ID: <CABvMjLTzcwN5h+Fn-nNc4VcSr71skh-813u+mCb+EW5wGOT0+g@mail.gmail.com>
Subject: Re: [PATCH] net: hisilicon: Fix usage of uninitialized variable in
 function mdio_sc_cfg_reg_write()
To:     David Miller <davem@davemloft.net>
Cc:     Chengyu Song <csong@cs.ucr.edu>, Zhiyun Qian <zhiyunq@cs.ucr.edu>,
        Yisen Zhuang <yisen.zhuang@huawei.com>,
        Salil Mehta <salil.mehta@huawei.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi David:

Thanks for your feedback. "regmap_write()" could also fail and cause
influence on the caller. If patches for "regmap_write()" are needed,
then the title could be changed from "uninitialized use" to "miss
return check".

On Wed, Oct 2, 2019 at 2:22 PM David Miller <davem@davemloft.net> wrote:
>
> 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.


-- 
Kind Regards,

Yizhuo Zhai

Computer Science, Graduate Student
University of California, Riverside