netdev - Re: [PATCH / RFC] net: fix locking in ibm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1218685554.8041.172.camel@pasglop>
Date:	Thu, 14 Aug 2008 13:45:54 +1000
From:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:	Sebastian Siewior <bigeasy@...utronix.de>
Cc:	netdev@...r.kernel.org, linuxppc-dev@...abs.org,
	Valentine Barshak <vbarshak@...mvista.com>
Subject: Re: [PATCH / RFC] net: fix locking in ibm_newemac

On Wed, 2008-08-13 at 20:29 +0200, Sebastian Siewior wrote:
> |PPC 4xx OCP EMAC driver, version 3.54
> |MAL v2 /plb/mcmal, 2 TX channels, 2 RX channels
> |RGMII /plb/opb/emac-rgmii@...00b00 initialized with MDIO support
> |/plb/opb/emac-rgmii@...00b00: input 0 in RGMII mode
> |BUG: spinlock bad magic on CPU#0, swapper/1
> | lock: cf81632c, .magic: 00000000, .owner: <none>/-1, .owner_cpu: 0
> |Call Trace:
> |[cf82bc70] [c00071c4] show_stack+0x34/0x194 (unreliable)
> |[cf82bca0] [c0136d5c] spin_bug+0x8c/0xd0
> |[cf82bcc0] [c0136f64] _raw_spin_lock+0x94/0x16c
> |[cf82bcf0] [c023b528] _spin_lock_bh+0x20/0x34
> |[cf82bd10] [c01d04a8] dev_mc_add+0x2c/0x84
> |[cf82bd30] [c0166f80] emac_configure+0x2a8/0x55c
> |[cf82bd60] [c02418bc] emac_probe+0xc24/0x105c
> |[cf82be40] [c01bb504] of_platform_device_probe+0x58/0x80
> |[cf82be60] [c016057c] driver_probe_device+0xb8/0x1ec
> |[cf82be80] [c0160734] __driver_attach+0x84/0x88
> |[cf82bea0] [c015fa4c] bus_for_each_dev+0x5c/0x98
> |[cf82bed0] [c0160384] driver_attach+0x24/0x34
> |[cf82bee0] [c01600b4] bus_add_driver+0x1d8/0x24c
> |[cf82bf00] [c0160944] driver_register+0x5c/0x158
> |[cf82bf20] [c01bb3dc] of_register_driver+0x54/0x70
> |[cf82bf30] [c030ba8c] emac_init+0x1c8/0x208
> |[cf82bf60] [c02f4184] kernel_init+0x84/0x27c
> |[cf82bff0] [c000e594] kernel_thread+0x44/0x60
> 
> The fix is to defer phy init until netdevice is registered / initialized.

I think it's better instead to take the dev_mc_add() statement
out of emac_configure().

What about this patch instead, does it fix it for you ?

ibm_newemac: Fix uninitialized spinlock at probe time

We must not call dev_mc_add() from within our HW configure which happens
before we initialize and register the netdev. Do it in open() instead.

Signed-off-by: Benjamin Herrenschmidt <benh@...nel.crashing.org>
---

Index: linux-work/drivers/net/ibm_newemac/core.c
===================================================================
--- linux-work.orig/drivers/net/ibm_newemac/core.c	2008-08-14 13:36:10.000000000 +1000
+++ linux-work/drivers/net/ibm_newemac/core.c	2008-08-14 13:42:10.000000000 +1000
@@ -663,9 +663,6 @@ static int emac_configure(struct emac_in
 	if (emac_phy_gpcs(dev->phy.mode))
 		emac_mii_reset_phy(&dev->phy);
 
-	/* Required for Pause packet support in EMAC */
-	dev_mc_add(ndev, default_mcast_addr, sizeof(default_mcast_addr), 1);
-
 	return 0;
 }
 
@@ -1150,6 +1147,9 @@ static int emac_open(struct net_device *
 	} else
 		netif_carrier_on(dev->ndev);
 
+	/* Required for Pause packet support in EMAC */
+	dev_mc_add(ndev, default_mcast_addr, sizeof(default_mcast_addr), 1);
+
 	emac_configure(dev);
 	mal_poll_add(dev->mal, &dev->commac);
 	mal_enable_tx_channel(dev->mal, dev->mal_tx_chan);


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html