diff mbox

[-next] clk: hisilicon: hi3660：Fix potential NULL dereference in hi3660_stub_clk_probe()

Message ID 1515047794-170174-1-git-send-email-weiyongjun1@huawei.com (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Wei Yongjun Jan. 4, 2018, 6:36 a.m. UTC 
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 <weiyongjun1@huawei.com>
---
 drivers/clk/hisilicon/clk-hi3660-stub.c | 2 ++
 1 file changed, 2 insertions(+)


--
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

Comments

Masahiro Yamada Jan. 5, 2018, 3:59 a.m. UTC | #1 
2018-01-04 15:36 GMT+09:00 Wei Yongjun <weiyongjun1@huawei.com>:
> 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 <weiyongjun1@huawei.com>
> ---
>  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.
Leo Yan Jan. 5, 2018, 8:40 a.m. UTC | #2 
On Fri, Jan 05, 2018 at 12:59:38PM +0900, Masahiro Yamada wrote:
> 2018-01-04 15:36 GMT+09:00 Wei Yongjun <weiyongjun1@huawei.com>:
> > 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 <weiyongjun1@huawei.com>
> > ---
> >  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()?

Thanks,
Leo Yan
--
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
Tao Wang Jan. 5, 2018, 9:47 a.m. UTC | #3 
在 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 <weiyongjun1@huawei.com>:
>>> 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 <weiyongjun1@huawei.com>
>>> ---
>>>   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.
> 
> Thanks,
> Leo Yan
> 
> .
> 

--
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
Masahiro Yamada Jan. 5, 2018, 9:50 a.m. UTC | #4 
2018-01-05 18:47 GMT+09:00 Wangtao (Kevin, Kirin) <kevin.wangtao@hisilicon.com>:
>
>
> 在 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 <weiyongjun1@huawei.com>:
>>>>
>>>> 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 <weiyongjun1@huawei.com>
>>>> ---
>>>>   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?
Tao Wang Jan. 5, 2018, 10:47 a.m. UTC | #5 
在 2018/1/5 17:50, Masahiro Yamada 写道:
> 2018-01-05 18:47 GMT+09:00 Wangtao (Kevin, Kirin) <kevin.wangtao@hisilicon.com>:
>>
>>
>> 在 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 <weiyongjun1@huawei.com>:
>>>>>
>>>>> 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 <weiyongjun1@huawei.com>
>>>>> ---
>>>>>    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?
actually it belongs to mailbox, but we use part of it as shared memory
> 
> 
> 
> 

--
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
Stephen Boyd March 12, 2018, 10:13 p.m. UTC | #6 
Quoting Wei Yongjun (2018-01-03 22:36:34)
> 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 <weiyongjun1@huawei.com>

Applied to clk-fixes
--
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 mbox

Patch

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;