public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] net, phy, cpsw: fix unaligned access if no phy found
@ 2013-09-04 12:16 Heiko Schocher
  2013-09-04 13:14 ` Tom Rini
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Heiko Schocher @ 2013-09-04 12:16 UTC (permalink / raw)
  To: u-boot

if phy_connect() did not find a phy, phydev is not initialized
and following code in cpsw_phy_init() maybe crashes. Fix this.

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Joe Hershberger <joe.hershberger@gmail.com>
Cc: Mugunthan V N <mugunthanvnm@ti.com>
Cc: Tom Rini <trini@ti.com>

---
Found on the dxr2 board with no phy connected to the board,
U-Boot crashes with:

U-Boot 2013.07-12701-gea98378-dirty (Sep 04 2013 - 06:58:16)

I2C:   ready
DRAM:  128 MiB
Enable d-cache
FactorySet is not right in eeprom.
NAND:  256 MiB
MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1
8-bit BCH HW ECC selected
Net:   Could not get PHY for cpsw: addr 0
data abort

    MAYBE you should read doc/README.arm-unaligned-accesses

pc : [<87f80574>]          lr : [<87f80fcc>]
sp : 86f5aee0  ip : 00000034     fp : 80100020
r10: 00000014  r9 : 07e5d000     r8 : 86f5af30
r7 : 86f5f750  r6 : 86f5f804     r5 : 86f5f708  r4 : 86f5f750
r3 : 00000000  r2 : 00000000     r1 : 87fa4d08  r0 : 00000000
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32
Resetting CPU ...

resetting ...
---
 drivers/net/cpsw.c | 3 +++
 1 Datei ge?ndert, 3 Zeilen hinzugef?gt(+)

diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
index 9bab71a..b18d528 100644
--- a/drivers/net/cpsw.c
+++ b/drivers/net/cpsw.c
@@ -947,6 +947,9 @@ static int cpsw_phy_init(struct eth_device *dev, struct cpsw_slave *slave)
 			dev,
 			slave->data->phy_if);
 
+	if (!phydev)
+		return -1;
+
 	phydev->supported &= supported;
 	phydev->advertising = phydev->supported;
 
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net, phy, cpsw: fix unaligned access if no phy found
  2013-09-04 12:16 [U-Boot] [PATCH] net, phy, cpsw: fix unaligned access if no phy found Heiko Schocher
@ 2013-09-04 13:14 ` Tom Rini
  2013-09-05  5:56   ` Heiko Schocher
  2013-09-04 19:11 ` Joe Hershberger
  2013-09-05  6:05 ` [U-Boot] [PATCH v2] net, phy, cpsw: fix NULL pointer deference Heiko Schocher
  2 siblings, 1 reply; 9+ messages in thread
From: Tom Rini @ 2013-09-04 13:14 UTC (permalink / raw)
  To: u-boot

On Wed, Sep 04, 2013 at 02:16:01PM +0200, Heiko Schocher wrote:

> if phy_connect() did not find a phy, phydev is not initialized
> and following code in cpsw_phy_init() maybe crashes. Fix this.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Joe Hershberger <joe.hershberger@gmail.com>
> Cc: Mugunthan V N <mugunthanvnm@ti.com>
> Cc: Tom Rini <trini@ti.com>
> 
> ---
> Found on the dxr2 board with no phy connected to the board,
> U-Boot crashes with:
> 
> U-Boot 2013.07-12701-gea98378-dirty (Sep 04 2013 - 06:58:16)
> 
> I2C:   ready
> DRAM:  128 MiB
> Enable d-cache
> FactorySet is not right in eeprom.
> NAND:  256 MiB
> MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1
> 8-bit BCH HW ECC selected
> Net:   Could not get PHY for cpsw: addr 0
> data abort
> 
>     MAYBE you should read doc/README.arm-unaligned-accesses
> 
> pc : [<87f80574>]          lr : [<87f80fcc>]
> sp : 86f5aee0  ip : 00000034     fp : 80100020
> r10: 00000014  r9 : 07e5d000     r8 : 86f5af30
> r7 : 86f5f750  r6 : 86f5f804     r5 : 86f5f708  r4 : 86f5f750
> r3 : 00000000  r2 : 00000000     r1 : 87fa4d08  r0 : 00000000
> Flags: nZCv  IRQs off  FIQs on  Mode SVC_32
> Resetting CPU ...
> 
> resetting ...
> ---
>  drivers/net/cpsw.c | 3 +++
>  1 Datei ge??ndert, 3 Zeilen hinzugef??gt(+)
> 
> diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
> index 9bab71a..b18d528 100644
> --- a/drivers/net/cpsw.c
> +++ b/drivers/net/cpsw.c
> @@ -947,6 +947,9 @@ static int cpsw_phy_init(struct eth_device *dev, struct cpsw_slave *slave)
>  			dev,
>  			slave->data->phy_if);
>  
> +	if (!phydev)
> +		return -1;
> +
>  	phydev->supported &= supported;
>  	phydev->advertising = phydev->supported;

This isn't really an unaligned access problem it's a NULL pointer
dereference, so I'll re-word the commit message when I grab this for
u-boot-ti soon.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130904/ba721599/attachment.pgp>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net, phy, cpsw: fix unaligned access if no phy found
  2013-09-04 12:16 [U-Boot] [PATCH] net, phy, cpsw: fix unaligned access if no phy found Heiko Schocher
  2013-09-04 13:14 ` Tom Rini
@ 2013-09-04 19:11 ` Joe Hershberger
  2013-09-05  6:05 ` [U-Boot] [PATCH v2] net, phy, cpsw: fix NULL pointer deference Heiko Schocher
  2 siblings, 0 replies; 9+ messages in thread
From: Joe Hershberger @ 2013-09-04 19:11 UTC (permalink / raw)
  To: u-boot

On Wed, Sep 4, 2013 at 7:16 AM, Heiko Schocher <hs@denx.de> wrote:
> if phy_connect() did not find a phy, phydev is not initialized
> and following code in cpsw_phy_init() maybe crashes. Fix this.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Joe Hershberger <joe.hershberger@gmail.com>
> Cc: Mugunthan V N <mugunthanvnm@ti.com>
> Cc: Tom Rini <trini@ti.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net, phy, cpsw: fix unaligned access if no phy found
  2013-09-04 13:14 ` Tom Rini
@ 2013-09-05  5:56   ` Heiko Schocher
  0 siblings, 0 replies; 9+ messages in thread
From: Heiko Schocher @ 2013-09-05  5:56 UTC (permalink / raw)
  To: u-boot

Hello Tom,

Am 04.09.2013 15:14, schrieb Tom Rini:
> On Wed, Sep 04, 2013 at 02:16:01PM +0200, Heiko Schocher wrote:
>
>> if phy_connect() did not find a phy, phydev is not initialized
>> and following code in cpsw_phy_init() maybe crashes. Fix this.
>>
>> Signed-off-by: Heiko Schocher<hs@denx.de>
>> Cc: Joe Hershberger<joe.hershberger@gmail.com>
>> Cc: Mugunthan V N<mugunthanvnm@ti.com>
>> Cc: Tom Rini<trini@ti.com>
>>
>> ---
>> Found on the dxr2 board with no phy connected to the board,
>> U-Boot crashes with:
>>
>> U-Boot 2013.07-12701-gea98378-dirty (Sep 04 2013 - 06:58:16)
>>
>> I2C:   ready
>> DRAM:  128 MiB
>> Enable d-cache
>> FactorySet is not right in eeprom.
>> NAND:  256 MiB
>> MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1
>> 8-bit BCH HW ECC selected
>> Net:   Could not get PHY for cpsw: addr 0
>> data abort
>>
>>      MAYBE you should read doc/README.arm-unaligned-accesses
>>
>> pc : [<87f80574>]          lr : [<87f80fcc>]
>> sp : 86f5aee0  ip : 00000034     fp : 80100020
>> r10: 00000014  r9 : 07e5d000     r8 : 86f5af30
>> r7 : 86f5f750  r6 : 86f5f804     r5 : 86f5f708  r4 : 86f5f750
>> r3 : 00000000  r2 : 00000000     r1 : 87fa4d08  r0 : 00000000
>> Flags: nZCv  IRQs off  FIQs on  Mode SVC_32
>> Resetting CPU ...
>>
>> resetting ...
>> ---
>>   drivers/net/cpsw.c | 3 +++
>>   1 Datei ge??ndert, 3 Zeilen hinzugef??gt(+)
>>
>> diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
>> index 9bab71a..b18d528 100644
>> --- a/drivers/net/cpsw.c
>> +++ b/drivers/net/cpsw.c
>> @@ -947,6 +947,9 @@ static int cpsw_phy_init(struct eth_device *dev, struct cpsw_slave *slave)
>>   			dev,
>>   			slave->data->phy_if);
>>
>> +	if (!phydev)
>> +		return -1;
>> +
>>   	phydev->supported&= supported;
>>   	phydev->advertising = phydev->supported;
>
> This isn't really an unaligned access problem it's a NULL pointer
> dereference, so I'll re-word the commit message when I grab this for
> u-boot-ti soon.

Yes, thanks ... Hmm.. there are also problems if starting tftp on such
a hardware ... I found more issues in this driver ... I repost a v2
soon ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2] net, phy, cpsw: fix NULL pointer deference
  2013-09-04 12:16 [U-Boot] [PATCH] net, phy, cpsw: fix unaligned access if no phy found Heiko Schocher
  2013-09-04 13:14 ` Tom Rini
  2013-09-04 19:11 ` Joe Hershberger
@ 2013-09-05  6:05 ` Heiko Schocher
  2013-09-05  8:27   ` Mugunthan V N
  2013-09-05  9:50   ` [U-Boot] [PATCH v3] " Heiko Schocher
  2 siblings, 2 replies; 9+ messages in thread
From: Heiko Schocher @ 2013-09-05  6:05 UTC (permalink / raw)
  To: u-boot

if phy_connect() did not find a phy, phydev is NULL and
following code in cpsw_phy_init() crashes. Fix this.

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Joe Hershberger <joe.hershberger@gmail.com>
Cc: Mugunthan V N <mugunthanvnm@ti.com>
Cc: Tom Rini <trini@ti.com>

---
- changes for v2:
  - change commit message as it is a NULL pointer deference error
    not an unaligned access problem, as Tom Rini mentioned
  - fix more places in the driver with NULL pointer problem

Found on the dxr2 board with no phy connected to the board,
U-Boot crashes with:

U-Boot 2013.07-12701-gea98378-dirty (Sep 04 2013 - 06:58:16)

I2C:   ready
DRAM:  128 MiB
Enable d-cache
FactorySet is not right in eeprom.
NAND:  256 MiB
MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1
8-bit BCH HW ECC selected
Net:   Could not get PHY for cpsw: addr 0
data abort

    MAYBE you should read doc/README.arm-unaligned-accesses

pc : [<87f80574>]          lr : [<87f80fcc>]
sp : 86f5aee0  ip : 00000034     fp : 80100020
r10: 00000014  r9 : 07e5d000     r8 : 86f5af30
r7 : 86f5f750  r6 : 86f5f804     r5 : 86f5f708  r4 : 86f5f750
r3 : 00000000  r2 : 00000000     r1 : 87fa4d08  r0 : 00000000
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32
Resetting CPU ...
---
 drivers/net/cpsw.c | 26 +++++++++++++++++++++++++-
 1 Datei ge?ndert, 25 Zeilen hinzugef?gt(+), 1 Zeile entfernt(-)

diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
index 9bab71a..186665b 100644
--- a/drivers/net/cpsw.c
+++ b/drivers/net/cpsw.c
@@ -561,6 +561,9 @@ static inline void setbit_and_wait_for_clear32(void *addr)
 static void cpsw_set_slave_mac(struct cpsw_slave *slave,
 			       struct cpsw_priv *priv)
 {
+	if (!priv)
+		return;
+
 	__raw_writel(mac_hi(priv->dev->enetaddr), &slave->regs->sa_hi);
 	__raw_writel(mac_lo(priv->dev->enetaddr), &slave->regs->sa_lo);
 }
@@ -568,9 +571,17 @@ static void cpsw_set_slave_mac(struct cpsw_slave *slave,
 static void cpsw_slave_update_link(struct cpsw_slave *slave,
 				   struct cpsw_priv *priv, int *link)
 {
-	struct phy_device *phy = priv->phydev;
+	struct phy_device *phy;
 	u32 mac_control = 0;
 
+	if (!priv)
+		return;
+
+	phy = priv->phydev;
+
+	if (!phy)
+		return;
+
 	phy_startup(phy);
 	*link = phy->link;
 
@@ -604,8 +615,12 @@ static int cpsw_update_link(struct cpsw_priv *priv)
 	int link = 0;
 	struct cpsw_slave *slave;
 
+	if (!priv)
+		return -1;
+
 	for_each_slave(slave, priv)
 		cpsw_slave_update_link(slave, priv, &link);
+
 	priv->mdio_link = readl(&mdio_regs->link);
 	return link;
 }
@@ -614,6 +629,9 @@ static int cpsw_check_link(struct cpsw_priv *priv)
 {
 	u32 link = 0;
 
+	if (!priv)
+		return -1;
+
 	link = __raw_readl(&mdio_regs->link) & priv->phy_mask;
 	if ((link) && (link == priv->mdio_link))
 		return 1;
@@ -623,6 +641,9 @@ static int cpsw_check_link(struct cpsw_priv *priv)
 
 static inline u32  cpsw_get_slave_port(struct cpsw_priv *priv, u32 slave_num)
 {
+	if (!priv)
+		return -1;
+
 	if (priv->host_port == 0)
 		return slave_num + 1;
 	else
@@ -947,6 +968,9 @@ static int cpsw_phy_init(struct eth_device *dev, struct cpsw_slave *slave)
 			dev,
 			slave->data->phy_if);
 
+	if (!phydev)
+		return -1;
+
 	phydev->supported &= supported;
 	phydev->advertising = phydev->supported;
 
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2] net, phy, cpsw: fix NULL pointer deference
  2013-09-05  6:05 ` [U-Boot] [PATCH v2] net, phy, cpsw: fix NULL pointer deference Heiko Schocher
@ 2013-09-05  8:27   ` Mugunthan V N
  2013-09-05  9:40     ` Heiko Schocher
  2013-09-05  9:50   ` [U-Boot] [PATCH v3] " Heiko Schocher
  1 sibling, 1 reply; 9+ messages in thread
From: Mugunthan V N @ 2013-09-05  8:27 UTC (permalink / raw)
  To: u-boot

On Thursday 05 September 2013 11:35 AM, Heiko Schocher wrote:
> if phy_connect() did not find a phy, phydev is NULL and
> following code in cpsw_phy_init() crashes. Fix this.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Joe Hershberger <joe.hershberger@gmail.com>
> Cc: Mugunthan V N <mugunthanvnm@ti.com>
> Cc: Tom Rini <trini@ti.com>
>
> ---
> - changes for v2:
>   - change commit message as it is a NULL pointer deference error
>     not an unaligned access problem, as Tom Rini mentioned
>   - fix more places in the driver with NULL pointer problem
>
> Found on the dxr2 board with no phy connected to the board,
> U-Boot crashes with:
>
> U-Boot 2013.07-12701-gea98378-dirty (Sep 04 2013 - 06:58:16)
>
> I2C:   ready
> DRAM:  128 MiB
> Enable d-cache
> FactorySet is not right in eeprom.
> NAND:  256 MiB
> MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1
> 8-bit BCH HW ECC selected
> Net:   Could not get PHY for cpsw: addr 0
> data abort
>
>     MAYBE you should read doc/README.arm-unaligned-accesses
>
> pc : [<87f80574>]          lr : [<87f80fcc>]
> sp : 86f5aee0  ip : 00000034     fp : 80100020
> r10: 00000014  r9 : 07e5d000     r8 : 86f5af30
> r7 : 86f5f750  r6 : 86f5f804     r5 : 86f5f708  r4 : 86f5f750
> r3 : 00000000  r2 : 00000000     r1 : 87fa4d08  r0 : 00000000
> Flags: nZCv  IRQs off  FIQs on  Mode SVC_32
> Resetting CPU ...
> ---
>  drivers/net/cpsw.c | 26 +++++++++++++++++++++++++-
>  1 Datei ge?ndert, 25 Zeilen hinzugef?gt(+), 1 Zeile entfernt(-)
>
> diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
> index 9bab71a..186665b 100644
> --- a/drivers/net/cpsw.c
> +++ b/drivers/net/cpsw.c
> @@ -561,6 +561,9 @@ static inline void setbit_and_wait_for_clear32(void *addr)
>  static void cpsw_set_slave_mac(struct cpsw_slave *slave,
>  			       struct cpsw_priv *priv)
>  {
> +	if (!priv)
> +		return;
> +
>  	__raw_writel(mac_hi(priv->dev->enetaddr), &slave->regs->sa_hi);
>  	__raw_writel(mac_lo(priv->dev->enetaddr), &slave->regs->sa_lo);
>  }
> @@ -568,9 +571,17 @@ static void cpsw_set_slave_mac(struct cpsw_slave *slave,
>  static void cpsw_slave_update_link(struct cpsw_slave *slave,
>  				   struct cpsw_priv *priv, int *link)
>  {
> -	struct phy_device *phy = priv->phydev;
> +	struct phy_device *phy;
>  	u32 mac_control = 0;
>  
> +	if (!priv)
> +		return;
> +
> +	phy = priv->phydev;
> +
> +	if (!phy)
> +		return;
> +
>  	phy_startup(phy);
>  	*link = phy->link;
>  
> @@ -604,8 +615,12 @@ static int cpsw_update_link(struct cpsw_priv *priv)
>  	int link = 0;
>  	struct cpsw_slave *slave;
>  
> +	if (!priv)
> +		return -1;
> +
>  	for_each_slave(slave, priv)
>  		cpsw_slave_update_link(slave, priv, &link);
> +
>  	priv->mdio_link = readl(&mdio_regs->link);
>  	return link;
>  }
> @@ -614,6 +629,9 @@ static int cpsw_check_link(struct cpsw_priv *priv)
>  {
>  	u32 link = 0;

All the above *priv* check can be remove as priv is already validated in
this function and all the above functions are called only from this
function.

>  
> +	if (!priv)
> +		return -1;
> +
>  	link = __raw_readl(&mdio_regs->link) & priv->phy_mask;
>  	if ((link) && (link == priv->mdio_link))
>  		return 1;
> @@ -623,6 +641,9 @@ static int cpsw_check_link(struct cpsw_priv *priv)
>  
>  static inline u32  cpsw_get_slave_port(struct cpsw_priv *priv, u32 slave_num)
>  {
> +	if (!priv)
> +		return -1;
> +
>  	if (priv->host_port == 0)
>  		return slave_num + 1;
>  	else
> @@ -947,6 +968,9 @@ static int cpsw_phy_init(struct eth_device *dev, struct cpsw_slave *slave)
>  			dev,
>  			slave->data->phy_if);
>  
> +	if (!phydev)
> +		return -1;
> +
>  	phydev->supported &= supported;
>  	phydev->advertising = phydev->supported;
>  
I don't think you need to validate *priv* as it is already validated in
driver register (cpsw_register()). In any case priv is NULL, then there
is some corruption in dev structure as without priv, dev should not
exist and stack is not supposed to call any cpsw apis.

Regards
Mugunthan V N

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2] net, phy, cpsw: fix NULL pointer deference
  2013-09-05  8:27   ` Mugunthan V N
@ 2013-09-05  9:40     ` Heiko Schocher
  0 siblings, 0 replies; 9+ messages in thread
From: Heiko Schocher @ 2013-09-05  9:40 UTC (permalink / raw)
  To: u-boot

Hello Mugunthan,

Am 05.09.2013 10:27, schrieb Mugunthan V N:
> On Thursday 05 September 2013 11:35 AM, Heiko Schocher wrote:
>> if phy_connect() did not find a phy, phydev is NULL and
>> following code in cpsw_phy_init() crashes. Fix this.
>>
>> Signed-off-by: Heiko Schocher<hs@denx.de>
>> Cc: Joe Hershberger<joe.hershberger@gmail.com>
>> Cc: Mugunthan V N<mugunthanvnm@ti.com>
>> Cc: Tom Rini<trini@ti.com>
>>
>> ---
[...]
>> diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
>> index 9bab71a..186665b 100644
>> --- a/drivers/net/cpsw.c
>> +++ b/drivers/net/cpsw.c
>> @@ -561,6 +561,9 @@ static inline void setbit_and_wait_for_clear32(void *addr)
>>   static void cpsw_set_slave_mac(struct cpsw_slave *slave,
>>   			       struct cpsw_priv *priv)
>>   {
>> +	if (!priv)
>> +		return;
>> +
>>   	__raw_writel(mac_hi(priv->dev->enetaddr),&slave->regs->sa_hi);
>>   	__raw_writel(mac_lo(priv->dev->enetaddr),&slave->regs->sa_lo);
>>   }
>> @@ -568,9 +571,17 @@ static void cpsw_set_slave_mac(struct cpsw_slave *slave,
>>   static void cpsw_slave_update_link(struct cpsw_slave *slave,
>>   				   struct cpsw_priv *priv, int *link)
>>   {
>> -	struct phy_device *phy = priv->phydev;
>> +	struct phy_device *phy;
>>   	u32 mac_control = 0;
>>
>> +	if (!priv)
>> +		return;
>> +
>> +	phy = priv->phydev;
>> +
>> +	if (!phy)
>> +		return;
>> +
>>   	phy_startup(phy);
>>   	*link = phy->link;
>>
>> @@ -604,8 +615,12 @@ static int cpsw_update_link(struct cpsw_priv *priv)
>>   	int link = 0;
>>   	struct cpsw_slave *slave;
>>
>> +	if (!priv)
>> +		return -1;
>> +
>>   	for_each_slave(slave, priv)
>>   		cpsw_slave_update_link(slave, priv,&link);
>> +
>>   	priv->mdio_link = readl(&mdio_regs->link);
>>   	return link;
>>   }
>> @@ -614,6 +629,9 @@ static int cpsw_check_link(struct cpsw_priv *priv)
>>   {
>>   	u32 link = 0;
>
> All the above *priv* check can be remove as priv is already validated in
> this function and all the above functions are called only from this
> function.

No, cpsw_update_link() is called for example directly from cpsw_init()
cpsw_set_slave_mac() from cpsw_slave_init()

>> +	if (!priv)
>> +		return -1;
>> +
>>   	link = __raw_readl(&mdio_regs->link)&  priv->phy_mask;
>>   	if ((link)&&  (link == priv->mdio_link))
>>   		return 1;
>> @@ -623,6 +641,9 @@ static int cpsw_check_link(struct cpsw_priv *priv)
>>
>>   static inline u32  cpsw_get_slave_port(struct cpsw_priv *priv, u32 slave_num)
>>   {
>> +	if (!priv)
>> +		return -1;
>> +
>>   	if (priv->host_port == 0)
>>   		return slave_num + 1;
>>   	else
>> @@ -947,6 +968,9 @@ static int cpsw_phy_init(struct eth_device *dev, struct cpsw_slave *slave)
>>   			dev,
>>   			slave->data->phy_if);
>>
>> +	if (!phydev)
>> +		return -1;
>> +
>>   	phydev->supported&= supported;
>>   	phydev->advertising = phydev->supported;
>>
> I don't think you need to validate *priv* as it is already validated in
> driver register (cpsw_register()). In any case priv is NULL, then there
> is some corruption in dev structure as without priv, dev should not
> exist and stack is not supposed to call any cpsw apis.

Yes, you are right. Checked it on the dxr2 board. We do not need
the "priv check" ... Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v3] net, phy, cpsw: fix NULL pointer deference
  2013-09-05  6:05 ` [U-Boot] [PATCH v2] net, phy, cpsw: fix NULL pointer deference Heiko Schocher
  2013-09-05  8:27   ` Mugunthan V N
@ 2013-09-05  9:50   ` Heiko Schocher
  2013-09-05 10:04     ` Mugunthan V N
  1 sibling, 1 reply; 9+ messages in thread
From: Heiko Schocher @ 2013-09-05  9:50 UTC (permalink / raw)
  To: u-boot

if phy_connect() did not find a phy, phydev is NULL and
following code in cpsw_phy_init() crashes. Fix this.

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Joe Hershberger <joe.hershberger@gmail.com>
Cc: Mugunthan V N <mugunthanvnm@ti.com>
Cc: Tom Rini <trini@ti.com>

---
- changes for v2:
  - change commit message as it is a NULL pointer deference error
    not a unaligned access problem, as Tom Rini mentioned
  - fix more places in the driver with NULL pointer problem
- changes for v3:
  - add comment from Mugunthan V N:
    - priv NULL pointer checks not needed

Found on the dxr2 board with no phy connected to the board,
U-Boot crashes with:

U-Boot 2013.07-12701-gea98378-dirty (Sep 04 2013 - 06:58:16)

I2C:   ready
DRAM:  128 MiB
Enable d-cache
FactorySet is not right in eeprom.
NAND:  256 MiB
MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1
8-bit BCH HW ECC selected
Net:   Could not get PHY for cpsw: addr 0
data abort

    MAYBE you should read doc/README.arm-unaligned-accesses

pc : [<87f80574>]          lr : [<87f80fcc>]
sp : 86f5aee0  ip : 00000034     fp : 80100020
r10: 00000014  r9 : 07e5d000     r8 : 86f5af30
r7 : 86f5f750  r6 : 86f5f804     r5 : 86f5f708  r4 : 86f5f750
r3 : 00000000  r2 : 00000000     r1 : 87fa4d08  r0 : 00000000
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32
Resetting CPU ...
---
 drivers/net/cpsw.c | 10 +++++++++-
 1 Datei ge?ndert, 9 Zeilen hinzugef?gt(+), 1 Zeile entfernt(-)

diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
index 9bab71a..39240d9 100644
--- a/drivers/net/cpsw.c
+++ b/drivers/net/cpsw.c
@@ -568,9 +568,14 @@ static void cpsw_set_slave_mac(struct cpsw_slave *slave,
 static void cpsw_slave_update_link(struct cpsw_slave *slave,
 				   struct cpsw_priv *priv, int *link)
 {
-	struct phy_device *phy = priv->phydev;
+	struct phy_device *phy;
 	u32 mac_control = 0;
 
+	phy = priv->phydev;
+
+	if (!phy)
+		return;
+
 	phy_startup(phy);
 	*link = phy->link;
 
@@ -947,6 +952,9 @@ static int cpsw_phy_init(struct eth_device *dev, struct cpsw_slave *slave)
 			dev,
 			slave->data->phy_if);
 
+	if (!phydev)
+		return -1;
+
 	phydev->supported &= supported;
 	phydev->advertising = phydev->supported;
 
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v3] net, phy, cpsw: fix NULL pointer deference
  2013-09-05  9:50   ` [U-Boot] [PATCH v3] " Heiko Schocher
@ 2013-09-05 10:04     ` Mugunthan V N
  0 siblings, 0 replies; 9+ messages in thread
From: Mugunthan V N @ 2013-09-05 10:04 UTC (permalink / raw)
  To: u-boot

On Thursday 05 September 2013 03:20 PM, Heiko Schocher wrote:
> if phy_connect() did not find a phy, phydev is NULL and
> following code in cpsw_phy_init() crashes. Fix this.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Joe Hershberger <joe.hershberger@gmail.com>
> Cc: Mugunthan V N <mugunthanvnm@ti.com>
> Cc: Tom Rini <trini@ti.com>
Acked-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-09-05 10:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-04 12:16 [U-Boot] [PATCH] net, phy, cpsw: fix unaligned access if no phy found Heiko Schocher
2013-09-04 13:14 ` Tom Rini
2013-09-05  5:56   ` Heiko Schocher
2013-09-04 19:11 ` Joe Hershberger
2013-09-05  6:05 ` [U-Boot] [PATCH v2] net, phy, cpsw: fix NULL pointer deference Heiko Schocher
2013-09-05  8:27   ` Mugunthan V N
2013-09-05  9:40     ` Heiko Schocher
2013-09-05  9:50   ` [U-Boot] [PATCH v3] " Heiko Schocher
2013-09-05 10:04     ` Mugunthan V N

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox