From patchwork Mon Jan 8 04:22:35 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Leo Yan X-Patchwork-Id: 10148963 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C1505601A1 for ; Mon, 8 Jan 2018 04:22:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A415A28797 for ; Mon, 8 Jan 2018 04:22:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 987D6287A3; Mon, 8 Jan 2018 04:22:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0179928797 for ; Mon, 8 Jan 2018 04:22:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755274AbeAHEWt (ORCPT ); Sun, 7 Jan 2018 23:22:49 -0500 Received: from mail-wr0-f194.google.com ([209.85.128.194]:34369 "EHLO mail-wr0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755266AbeAHEWs (ORCPT ); Sun, 7 Jan 2018 23:22:48 -0500 Received: by mail-wr0-f194.google.com with SMTP id 36so9379323wrh.1 for ; Sun, 07 Jan 2018 20:22:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=NQxV45RyJk/pni1bgJmFXfVHrmiSsfXwT7c6WwPm8Yc=; b=E90UuWUe756aP3FeUMT5tgpuHMF4r1bVL5yc9kJ52mTmf+ZVwEziMAPYI4E90nHjcw sMipYsq4V8QHVVPFWT6VCn6Tnrzlw75syBGoBDN6oZgOx+hW+9lAu/Hk+SsKpZgazxD5 QT/ASlkkpiqhTesNjStS3gCzTXT0D/nGCA+Og= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=NQxV45RyJk/pni1bgJmFXfVHrmiSsfXwT7c6WwPm8Yc=; b=ZIMRt2pQaDSlxE31ZxO3JzLug1qhhA4LQU71wsS5vYhVaHTawQPBNFRiTKppiNQOJt NZt+97Tv1bhSmNFHjsEIvhyfpoYlCCPIw5UWIt3IRKDKyEPXCq9mhA0ikqv6U3mcWdTt cBsBQew8BX6y4wsLyD+/adSRHgKnk/zpfhMLQJg2nF1sL9Fq1AZJcdeyNydxnw4mvYcM WfjfwrrXImcD1xQ/yQahVnHZqm12QLnKY3KHl0FFROYdwZ+z/IlmZOnyz7rsrUQc88g6 8A994pjylAnPXRi7LLCKfCs1WfcOPjZHybgFJ/z17wm83ti68Ei2CsnbJ778y7+QqChN wwiw== X-Gm-Message-State: AKGB3mJ4u/Yzs7xbwQV+Z5EICG9c7+DkDh7QS8DssUdHpVDKp1H+IIIW 5L4XFo9gy9YcuPXawZnJ/7J2EdEef68= X-Google-Smtp-Source: ACJfBoucs0z+ILeuFteDO93ZwDp0a1pANihIXF8IXObft74VBVxmbKrUs36dsWQZyD/BoHw92QKPyA== X-Received: by 10.223.177.207 with SMTP id r15mr9612772wra.259.1515385367256; Sun, 07 Jan 2018 20:22:47 -0800 (PST) Received: from leoy-ThinkPad-T440 (li560-169.members.linode.com. [151.236.216.169]) by smtp.gmail.com with ESMTPSA id y137sm13333339wme.0.2018.01.07.20.22.41 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Sun, 07 Jan 2018 20:22:45 -0800 (PST) Date: Mon, 8 Jan 2018 12:22:35 +0800 From: Leo Yan To: Masahiro Yamada Cc: "Wangtao (Kevin, Kirin)" , Wei Yongjun , Michael Turquette , Stephen Boyd , Kaihua Zhong , Ruyi Wang , Kai Zhao , linux-clk Subject: Re: [PATCH -next] clk: =?utf-8?Q?hisilicon?= =?utf-8?B?OiBoaTM2NjDvvJpGaQ==?= =?utf-8?Q?x?= potential NULL dereference in hi3660_stub_clk_probe() Message-ID: <20180108042235.GA21618@leoy-ThinkPad-T440> References: <1515047794-170174-1-git-send-email-weiyongjun1@huawei.com> <20180105084004.GC26855@leoy-linaro> <7454cc50-7b92-2531-4261-45bd1703f1fb@hisilicon.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-clk-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-clk@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Jan 05, 2018 at 06:50:21PM +0900, Masahiro Yamada wrote: > 2018-01-05 18:47 GMT+09:00 Wangtao (Kevin, Kirin) : > > > > > > 在 2018/1/5 16:40, Leo Yan 写道: > >> > >> On Fri, Jan 05, 2018 at 12:59:38PM +0900, Masahiro Yamada wrote: > >>> > >>> 2018-01-04 15:36 GMT+09:00 Wei Yongjun : > >>>> > >>>> platform_get_resource() may return NULL, add proper check to > >>>> avoid potential NULL dereferencing. > >>>> > >>>> This is detected by Coccinelle semantic patch. > >>>> > >>>> @@ > >>>> expression pdev, res, n, t, e, e1, e2; > >>>> @@ > >>>> > >>>> res = platform_get_resource(pdev, t, n); > >>>> + if (!res) > >>>> + return -EINVAL; > >>>> ... when != res == NULL > >>>> e = devm_ioremap(e1, res->start, e2); > >>>> > >>>> Fixes: 4f16f7ff3bc0 ("clk: hisilicon: Add support for Hi3660 stub > >>>> clocks") > >>>> Signed-off-by: Wei Yongjun > >>>> --- > >>>> drivers/clk/hisilicon/clk-hi3660-stub.c | 2 ++ > >>>> 1 file changed, 2 insertions(+) > >>>> > >>>> diff --git a/drivers/clk/hisilicon/clk-hi3660-stub.c > >>>> b/drivers/clk/hisilicon/clk-hi3660-stub.c > >>>> index 9b6c72b..e8b2c43 100644 > >>>> --- a/drivers/clk/hisilicon/clk-hi3660-stub.c > >>>> +++ b/drivers/clk/hisilicon/clk-hi3660-stub.c > >>>> @@ -149,6 +149,8 @@ static int hi3660_stub_clk_probe(struct > >>>> platform_device *pdev) > >>>> return PTR_ERR(stub_clk_chan.mbox); > >>>> > >>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > >>>> + if (!res) > >>>> + return -EINVAL; > >>>> freq_reg = devm_ioremap(dev, res->start, resource_size(res)); > >>>> if (!freq_reg) > >>>> return -ENOMEM; > >>> > >>> > >>> Probably, it is better to use devm_ioremap_resource(). > >>> This checks NULL input. > >> > >> > >> Thanks for suggestion, Masahiro. This is good suggestion. > >> > >> Yongjun, could you spin new patch by using devm_ioremap_resource() > >> to replace devm_ioremap()? > > > > clk-stub's memory region has intersection with mailbox, so we can not use > > devm_ioremap_resource here, it will cause problem when mailbox driver be > > accepted by mainline. > > Hmm, that sounds odd. > > syscon in this case? I verified Yongjun patch v2 for using devm_ioremap_resource(), just as Kevin mentioned after using devm_ioremap_resource() the kernel has memory mapping conflict between stub clock driver and mailbox driver. Following Masahiro suggestion, I changed the stub clock driver to use syscon & regmap to access the memory, I can confirm this can work well, you could review in below change. But this change should note two things, one thing is it is dependent on mailbox driver upstreaming, so I prefer we can commit one patch for this change along with mailbox driver patch set; the second thing is we need change DT binding document. If you have any better solution for this, also very welcome. Thanks, Leo Yan ---8<--- --- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/clk/hisilicon/clk-hi3660-stub.c b/drivers/clk/hisilicon/clk-hi3660-stub.c index e8dde4f..40ee63b 100644 --- a/drivers/clk/hisilicon/clk-hi3660-stub.c +++ b/drivers/clk/hisilicon/clk-hi3660-stub.c @@ -25,12 +25,14 @@ #include #include #include +#include #include #include #include +#include #include -#define HI3660_STUB_CLOCK_DATA (0x70) +#define HI3660_STUB_CLOCK_DATA (0x570) #define MHZ (1000 * 1000) #define DEFINE_CLK_STUB(_id, _cmd, _name) \ @@ -60,7 +62,7 @@ struct hi3660_stub_clk { unsigned int rate; }; -static void __iomem *freq_reg; +static struct regmap *freq_reg; static struct hi3660_stub_clk_chan stub_clk_chan; static unsigned long hi3660_stub_clk_recalc_rate(struct clk_hw *hw, @@ -72,7 +74,9 @@ static unsigned long hi3660_stub_clk_recalc_rate(struct clk_hw *hw, * LPM3 writes back the CPU frequency in shared SRAM so read * back the frequency. */ - stub_clk->rate = readl(freq_reg + (stub_clk->id << 2)) * MHZ; + regmap_read(freq_reg, HI3660_STUB_CLOCK_DATA + (stub_clk->id << 2), + &stub_clk->rate); + stub_clk->rate *= MHZ; return stub_clk->rate; } @@ -133,7 +137,7 @@ static struct clk_hw *hi3660_stub_clk_hw_get(struct of_phandle_args *clkspec, static int hi3660_stub_clk_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; - struct resource *res; + struct device_node *np = pdev->dev.of_node; unsigned int i; int ret; @@ -148,12 +152,12 @@ static int hi3660_stub_clk_probe(struct platform_device *pdev) if (IS_ERR(stub_clk_chan.mbox)) return PTR_ERR(stub_clk_chan.mbox); - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - freq_reg = devm_ioremap_resource(dev, res); - if (IS_ERR(freq_reg)) + freq_reg = syscon_regmap_lookup_by_phandle(np, + "hisilicon,hi3660-clk-sram"); + if (IS_ERR(freq_reg)) { + dev_err(dev, "failed to get sram regmap\n"); return PTR_ERR(freq_reg); - - freq_reg += HI3660_STUB_CLOCK_DATA; + } for (i = 0; i < HI3660_CLK_STUB_NUM; i++) { ret = devm_clk_hw_register(&pdev->dev, &hi3660_stub_clks[i].hw);