* [v2] net:ethernet:cavium:octeon:octeon_mgmt: Handle return NULL error from devm_ioremap
@ 2016-12-14 16:25 Arvind Yadav
2016-12-14 17:32 ` David Daney
0 siblings, 1 reply; 7+ messages in thread
From: Arvind Yadav @ 2016-12-14 16:25 UTC (permalink / raw)
To: peter.chen, fw, david.daney; +Cc: netdev, linux-kernel
Here, If devm_ioremap will fail. It will return NULL.
Kernel can run into a NULL-pointer dereference.
This error check will avoid NULL pointer dereference.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
index 4ab404f..33c2fec 100644
--- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
+++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
@@ -1479,6 +1479,12 @@ static int octeon_mgmt_probe(struct platform_device *pdev)
p->agl = (u64)devm_ioremap(&pdev->dev, p->agl_phys, p->agl_size);
p->agl_prt_ctl = (u64)devm_ioremap(&pdev->dev, p->agl_prt_ctl_phys,
p->agl_prt_ctl_size);
+ if (!p->mix || !p->agl || !p->agl_prt_ctl) {
+ dev_err(&pdev->dev, "failed to map I/O memory\n");
+ result = -ENOMEM;
+ goto err;
+ }
+
spin_lock_init(&p->lock);
skb_queue_head_init(&p->tx_list);
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [v2] net:ethernet:cavium:octeon:octeon_mgmt: Handle return NULL error from devm_ioremap
2016-12-14 16:25 [v2] net:ethernet:cavium:octeon:octeon_mgmt: Handle return NULL error from devm_ioremap Arvind Yadav
@ 2016-12-14 17:32 ` David Daney
2016-12-14 18:06 ` arvind Yadav
0 siblings, 1 reply; 7+ messages in thread
From: David Daney @ 2016-12-14 17:32 UTC (permalink / raw)
To: Arvind Yadav, peter.chen, fw, david.daney; +Cc: netdev, linux-kernel
On 12/14/2016 08:25 AM, Arvind Yadav wrote:
> Here, If devm_ioremap will fail. It will return NULL.
> Kernel can run into a NULL-pointer dereference.
> This error check will avoid NULL pointer dereference.
>
Have you ever seen this failure in the wild?
How was the patch tested?
Thanks,
David Daney
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
> index 4ab404f..33c2fec 100644
> --- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
> +++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
> @@ -1479,6 +1479,12 @@ static int octeon_mgmt_probe(struct platform_device *pdev)
> p->agl = (u64)devm_ioremap(&pdev->dev, p->agl_phys, p->agl_size);
> p->agl_prt_ctl = (u64)devm_ioremap(&pdev->dev, p->agl_prt_ctl_phys,
> p->agl_prt_ctl_size);
> + if (!p->mix || !p->agl || !p->agl_prt_ctl) {
> + dev_err(&pdev->dev, "failed to map I/O memory\n");
> + result = -ENOMEM;
> + goto err;
> + }
> +
> spin_lock_init(&p->lock);
>
> skb_queue_head_init(&p->tx_list);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2] net:ethernet:cavium:octeon:octeon_mgmt: Handle return NULL error from devm_ioremap
2016-12-14 17:32 ` David Daney
@ 2016-12-14 18:06 ` arvind Yadav
2016-12-14 18:14 ` David Daney
0 siblings, 1 reply; 7+ messages in thread
From: arvind Yadav @ 2016-12-14 18:06 UTC (permalink / raw)
To: David Daney, peter.chen, fw, david.daney; +Cc: netdev, linux-kernel
Yes, I have seen this error. We have a device with very less memory.
Basically it's OMAP2 board. We have to port Android L on this.
It's has 3.10 kernel version. In this device, we were getting Page
allocation failure.
Vmalloc size was not enough to run all application. So we have decide to
increase vmalloc reserve space. once we increases Vmalloc space.
We start getting ioremap falilure. Kernel is getting NULL-pointer
dereference error.
Here, It's just check to avoid any kernel crash because of ioremap failure.
We can keep this check to avoid this kind of scenario.
Thanks
-Arvind
On Wednesday 14 December 2016 11:02 PM, David Daney wrote:
> On 12/14/2016 08:25 AM, Arvind Yadav wrote:
>> Here, If devm_ioremap will fail. It will return NULL.
>> Kernel can run into a NULL-pointer dereference.
>> This error check will avoid NULL pointer dereference.
>>
> i
> Have you ever seen this failure in the wild?
>
> How was the patch tested?
>
> Thanks,
> David Daney
>
>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>> ---
>> drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>> b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>> index 4ab404f..33c2fec 100644
>> --- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>> +++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>> @@ -1479,6 +1479,12 @@ static int octeon_mgmt_probe(struct
>> platform_device *pdev)
>> p->agl = (u64)devm_ioremap(&pdev->dev, p->agl_phys, p->agl_size);
>> p->agl_prt_ctl = (u64)devm_ioremap(&pdev->dev, p->agl_prt_ctl_phys,
>> p->agl_prt_ctl_size);
>> + if (!p->mix || !p->agl || !p->agl_prt_ctl) {
>> + dev_err(&pdev->dev, "failed to map I/O memory\n");
>> + result = -ENOMEM;
>> + goto err;
>> + }
>> +
>> spin_lock_init(&p->lock);
>>
>> skb_queue_head_init(&p->tx_list);
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2] net:ethernet:cavium:octeon:octeon_mgmt: Handle return NULL error from devm_ioremap
2016-12-14 18:06 ` arvind Yadav
@ 2016-12-14 18:14 ` David Daney
2016-12-14 18:39 ` arvind Yadav
0 siblings, 1 reply; 7+ messages in thread
From: David Daney @ 2016-12-14 18:14 UTC (permalink / raw)
To: arvind Yadav, peter.chen, fw, david.daney; +Cc: netdev, linux-kernel
On 12/14/2016 10:06 AM, arvind Yadav wrote:
> Yes, I have seen this error. We have a device with very less memory.
> Basically it's OMAP2 board. We have to port Android L on this.
> It's has 3.10 kernel version. In this device, we were getting Page
> allocation failure.
This makes absolutely no sense to me. OCTEON is a mips64 SoC with a ton
of memory where ioremap can never fail, and it doesn't run Android, and
you are talking about OMAP2.
Q1: Have you observed a failure on the device for which you are
modifying the driver?
Q2: Have you tested the patch on hardware that uses the driver you are
modifying by running network traffic through the Ethernet interface this
driver controls?
If you cannot answer yes to both of those questions, then you should
probably note in the changelog that the patch is untested.
David.
> Vmalloc size was not enough to run all application. So we have decide to
> increase vmalloc reserve space. once we increases Vmalloc space.
> We start getting ioremap falilure. Kernel is getting NULL-pointer
> dereference error.
>
> Here, It's just check to avoid any kernel crash because of ioremap failure.
> We can keep this check to avoid this kind of scenario.
>
> Thanks
> -Arvind
>
>
> On Wednesday 14 December 2016 11:02 PM, David Daney wrote:
>> On 12/14/2016 08:25 AM, Arvind Yadav wrote:
>>> Here, If devm_ioremap will fail. It will return NULL.
>>> Kernel can run into a NULL-pointer dereference.
>>> This error check will avoid NULL pointer dereference.
>>>
>> i
>> Have you ever seen this failure in the wild?
>>
>> How was the patch tested?
>>
>> Thanks,
>> David Daney
>>
>>
>>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>>> ---
>>> drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>>> b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>>> index 4ab404f..33c2fec 100644
>>> --- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>>> +++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>>> @@ -1479,6 +1479,12 @@ static int octeon_mgmt_probe(struct
>>> platform_device *pdev)
>>> p->agl = (u64)devm_ioremap(&pdev->dev, p->agl_phys, p->agl_size);
>>> p->agl_prt_ctl = (u64)devm_ioremap(&pdev->dev, p->agl_prt_ctl_phys,
>>> p->agl_prt_ctl_size);
>>> + if (!p->mix || !p->agl || !p->agl_prt_ctl) {
>>> + dev_err(&pdev->dev, "failed to map I/O memory\n");
>>> + result = -ENOMEM;
>>> + goto err;
>>> + }
>>> +
>>> spin_lock_init(&p->lock);
>>>
>>> skb_queue_head_init(&p->tx_list);
>>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2] net:ethernet:cavium:octeon:octeon_mgmt: Handle return NULL error from devm_ioremap
2016-12-14 18:14 ` David Daney
@ 2016-12-14 18:39 ` arvind Yadav
2016-12-14 18:54 ` Florian Fainelli
0 siblings, 1 reply; 7+ messages in thread
From: arvind Yadav @ 2016-12-14 18:39 UTC (permalink / raw)
To: David Daney, peter.chen, fw, david.daney; +Cc: netdev, linux-kernel
Hi David,
I have gave my comment.
Thanks
Arvind
On Wednesday 14 December 2016 11:44 PM, David Daney wrote:
> On 12/14/2016 10:06 AM, arvind Yadav wrote:
>> Yes, I have seen this error. We have a device with very less memory.
>> Basically it's OMAP2 board. We have to port Android L on this.
>> It's has 3.10 kernel version. In this device, we were getting Page
>> allocation failure.
>
> This makes absolutely no sense to me. OCTEON is a mips64 SoC with a
> ton of memory where ioremap can never fail, and it doesn't run
> Android, and you are talking about OMAP2.
-I just gave as example where i have seen ioremap issue.
Please don't relate. I know, Now it will not fail. ioremap will through
NULL on failure. We should catch this error. Even other driver of MIPS
soc is having same check. It's just check which will not impact any
functionality or performance of this driver. It will avoid NULL pointer
error. We know, if function is returning any error. we should catch.
>
> Q1: Have you observed a failure on the device for which you are
> modifying the driver?
-No, I did not observe this error.
>
> Q2: Have you tested the patch on hardware that uses the driver you are
> modifying by running network traffic through the Ethernet interface
> this driver controls?
-Right Now we can not tested these kind of failure,
>
> If you cannot answer yes to both of those questions, then you should
> probably note in the changelog that the patch is untested.
>
> David.
>
>
>> Vmalloc size was not enough to run all application. So we have decide to
>> increase vmalloc reserve space. once we increases Vmalloc space.
>> We start getting ioremap falilure. Kernel is getting NULL-pointer
>> dereference error.
>>
>> Here, It's just check to avoid any kernel crash because of ioremap
>> failure.
>> We can keep this check to avoid this kind of scenario.
>>
>> Thanks
>> -Arvind
>>
>>
>> On Wednesday 14 December 2016 11:02 PM, David Daney wrote:
>>> On 12/14/2016 08:25 AM, Arvind Yadav wrote:
>>>> Here, If devm_ioremap will fail. It will return NULL.
>>>> Kernel can run into a NULL-pointer dereference.
>>>> This error check will avoid NULL pointer dereference.
>>>>
>>> i
>>> Have you ever seen this failure in the wild?
>>>
>>> How was the patch tested?
>>>
>>> Thanks,
>>> David Daney
>>>
>>>
>>>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>>>> ---
>>>> drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 6 ++++++
>>>> 1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>>>> b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>>>> index 4ab404f..33c2fec 100644
>>>> --- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>>>> +++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>>>> @@ -1479,6 +1479,12 @@ static int octeon_mgmt_probe(struct
>>>> platform_device *pdev)
>>>> p->agl = (u64)devm_ioremap(&pdev->dev, p->agl_phys, p->agl_size);
>>>> p->agl_prt_ctl = (u64)devm_ioremap(&pdev->dev,
>>>> p->agl_prt_ctl_phys,
>>>> p->agl_prt_ctl_size);
>>>> + if (!p->mix || !p->agl || !p->agl_prt_ctl) {
>>>> + dev_err(&pdev->dev, "failed to map I/O memory\n");
>>>> + result = -ENOMEM;
>>>> + goto err;
>>>> + }
>>>> +
>>>> spin_lock_init(&p->lock);
>>>>
>>>> skb_queue_head_init(&p->tx_list);
>>>>
>>>
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2] net:ethernet:cavium:octeon:octeon_mgmt: Handle return NULL error from devm_ioremap
2016-12-14 18:39 ` arvind Yadav
@ 2016-12-14 18:54 ` Florian Fainelli
2016-12-14 19:05 ` arvind Yadav
0 siblings, 1 reply; 7+ messages in thread
From: Florian Fainelli @ 2016-12-14 18:54 UTC (permalink / raw)
To: arvind Yadav; +Cc: netdev, linux-kernel
On 12/14/2016 10:39 AM, arvind Yadav wrote:
> Hi David,
>
> I have gave my comment.
>
> Thanks
> Arvind
>
> On Wednesday 14 December 2016 11:44 PM, David Daney wrote:
>> On 12/14/2016 10:06 AM, arvind Yadav wrote:
>>> Yes, I have seen this error. We have a device with very less memory.
>>> Basically it's OMAP2 board. We have to port Android L on this.
>>> It's has 3.10 kernel version. In this device, we were getting Page
>>> allocation failure.
>>
>> This makes absolutely no sense to me. OCTEON is a mips64 SoC with a
>> ton of memory where ioremap can never fail, and it doesn't run
>> Android, and you are talking about OMAP2.
> -I just gave as example where i have seen ioremap issue.
> Please don't relate. I know, Now it will not fail. ioremap will through
> NULL on failure. We should catch this error. Even other driver of MIPS
> soc is having same check. It's just check which will not impact any
> functionality or performance of this driver. It will avoid NULL pointer
> error. We know, if function is returning any error. we should catch.
Your patch subject should also be changed to insert spaces between
semicolon, so this would be:
net: ethernet: cavium: octeon: octeon_mgmt:
--
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2] net:ethernet:cavium:octeon:octeon_mgmt: Handle return NULL error from devm_ioremap
2016-12-14 18:54 ` Florian Fainelli
@ 2016-12-14 19:05 ` arvind Yadav
0 siblings, 0 replies; 7+ messages in thread
From: arvind Yadav @ 2016-12-14 19:05 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, linux-kernel
Hi,
As per your suggestion, I have change the subject.
Thanks
On Thursday 15 December 2016 12:24 AM, Florian Fainelli wrote:
> On 12/14/2016 10:39 AM, arvind Yadav wrote:
>> Hi David,
>>
>> I have gave my comment.
>>
>> Thanks
>> Arvind
>>
>> On Wednesday 14 December 2016 11:44 PM, David Daney wrote:
>>> On 12/14/2016 10:06 AM, arvind Yadav wrote:
>>>> Yes, I have seen this error. We have a device with very less memory.
>>>> Basically it's OMAP2 board. We have to port Android L on this.
>>>> It's has 3.10 kernel version. In this device, we were getting Page
>>>> allocation failure.
>>> This makes absolutely no sense to me. OCTEON is a mips64 SoC with a
>>> ton of memory where ioremap can never fail, and it doesn't run
>>> Android, and you are talking about OMAP2.
>> -I just gave as example where i have seen ioremap issue.
>> Please don't relate. I know, Now it will not fail. ioremap will through
>> NULL on failure. We should catch this error. Even other driver of MIPS
>> soc is having same check. It's just check which will not impact any
>> functionality or performance of this driver. It will avoid NULL pointer
>> error. We know, if function is returning any error. we should catch.
> Your patch subject should also be changed to insert spaces between
> semicolon, so this would be:
>
> net: ethernet: cavium: octeon: octeon_mgmt:
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-12-14 19:05 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-14 16:25 [v2] net:ethernet:cavium:octeon:octeon_mgmt: Handle return NULL error from devm_ioremap Arvind Yadav
2016-12-14 17:32 ` David Daney
2016-12-14 18:06 ` arvind Yadav
2016-12-14 18:14 ` David Daney
2016-12-14 18:39 ` arvind Yadav
2016-12-14 18:54 ` Florian Fainelli
2016-12-14 19:05 ` arvind Yadav
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.