Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1542125 > unrolled thread

[v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return NULL error from devm_ioremap

Started byArvind Yadav <arvind.yadav.cs@gmail.com>
First post2016-12-14 20:10 +0100
Last post2016-12-19 22:40 +0100
Articles 4 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return NULL error from devm_ioremap Arvind Yadav <arvind.yadav.cs@gmail.com> - 2016-12-14 20:10 +0100
    Re: [v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return  NULL error from devm_ioremap arvind Yadav <arvind.yadav.cs@gmail.com> - 2016-12-15 16:10 +0100
    Re: [v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return  NULL error from devm_ioremap David Miller <davem@davemloft.net> - 2016-12-19 17:10 +0100
      Re: [v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return  NULL error from devm_ioremap David Daney <ddaney.cavm@gmail.com> - 2016-12-19 22:40 +0100

#1542125 — [v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return NULL error from devm_ioremap

FromArvind Yadav <arvind.yadav.cs@gmail.com>
Date2016-12-14 20:10 +0100
Subject[v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return NULL error from devm_ioremap
Message-ID<sOn69-2mQ-1@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

#1542805 — Re: [v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return NULL error from devm_ioremap

Fromarvind Yadav <arvind.yadav.cs@gmail.com>
Date2016-12-15 16:10 +0100
SubjectRe: [v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return NULL error from devm_ioremap
Message-ID<sOFPs-7kP-37@gated-at.bofh.it>
In reply to#1542125
Hi David,

I did not tested this feature.  I have build it and flashed on hardware.
You can check below commit id. Which has similar check for ioremap.
1- Commit id - de9e397e40f56b9f34af4bf6a5bd7a75ea02456c
       In 'drivers/net/phy/mdio-octeon.c'

2- Commit id - 592569de4c247fe4f25db8369dc0c63860f9560b
       In 'drivers/gpio/gpio-octeon.c'

Thanks
Arvind

On Thursday 15 December 2016 12:58 AM, David Daney wrote:
> On 12/14/2016 11:03 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. t
>
> I have asked you twice already this question, but could not determine 
> from your response what the answer is:
>
> Q: Have you tested the patch on OCTEON based hardware that contains 
> the "octeon_mgmt" Ethernet ports?  Please answer either "yes" or "no".
>
>
> 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);
>>
>

[toc] | [prev] | [next] | [standalone]

#1544630 — Re: [v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return NULL error from devm_ioremap

FromDavid Miller <davem@davemloft.net>
Date2016-12-19 17:10 +0100
SubjectRe: [v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return NULL error from devm_ioremap
Message-ID<sQ8FH-1Hm-3@gated-at.bofh.it>
In reply to#1542125
From: Arvind Yadav <arvind.yadav.cs@gmail.com>
Date: Thu, 15 Dec 2016 00:33:30 +0530

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

Since ioremap() is in fact designed to possibly fail, we do
need to always check it's return value.  So this change is
correct and I have applied it to the 'net' tree.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1544780 — Re: [v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return NULL error from devm_ioremap

FromDavid Daney <ddaney.cavm@gmail.com>
Date2016-12-19 22:40 +0100
SubjectRe: [v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return NULL error from devm_ioremap
Message-ID<sQdP3-4P4-17@gated-at.bofh.it>
In reply to#1544630
On 12/19/2016 08:04 AM, David Miller wrote:
> From: Arvind Yadav <arvind.yadav.cs@gmail.com>
> Date: Thu, 15 Dec 2016 00:33:30 +0530
>
>> 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>
>
> Since ioremap() is in fact designed to possibly fail, we do
> need to always check it's return value.  So this change is
> correct and I have applied it to the 'net' tree.

Yes, I think that is fine, although I have not tested the patch.

In general I like to know if a patch fixes a problem that has occurred 
on a platform used by the patch author, or if the author just noticed an 
issue through code inspection or automated tool for a platform that they 
cannot test on.

This patch appears to fall into the second category, but attempts to 
determine this for sure were for the most part unsuccessful.

With respect to ioremap(), in general I agree that it is designed to 
possibly fail.  For mips64 however, I think the implementation can never 
fail.  Certainly testing for failure fits better with what we expect to 
see in Linux kernel code.


>
> Thanks.
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web