linux-kernel - [PATCH 3.16 218/254] net: fs_enet: do not call phy

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]
Date:   Wed, 28 Feb 2018 15:20:18 +0000
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, "David S. Miller" <davem@...emloft.net>,
        "Christophe Leroy" <christophe.leroy@....fr>
Subject: [PATCH 3.16 218/254] net: fs_enet: do not call phy_stop() in
 interrupts

3.16.55-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Christophe Leroy <christophe.leroy@....fr>

commit f8b39039cbf2a15f2b8c9f081e1cbd5dee00aaf5 upstream.

In case of TX timeout, fs_timeout() calls phy_stop(), which
triggers the following BUG_ON() as we are in interrupt.

[92708.199889] kernel BUG at drivers/net/phy/mdio_bus.c:482!
[92708.204985] Oops: Exception in kernel mode, sig: 5 [#1]
[92708.210119] PREEMPT
[92708.212107] CMPC885
[92708.214216] CPU: 0 PID: 3 Comm: ksoftirqd/0 Tainted: G        W       4.9.61 #39
[92708.223227] task: c60f0a40 task.stack: c6104000
[92708.227697] NIP: c02a84bc LR: c02a947c CTR: c02a93d8
[92708.232614] REGS: c6105c70 TRAP: 0700   Tainted: G        W        (4.9.61)
[92708.241193] MSR: 00021032 <ME,IR,DR,RI>[92708.244818]   CR: 24000822  XER: 20000000
[92708.248767]
GPR00: c02a947c c6105d20 c60f0a40 c62b4c00 00000005 0000001f c069aad8 0001a688
GPR08: 00000007 00000100 c02a93d8 00000000 000005fc 00000000 c6213240 c06338e4
GPR16: 00000001 c06330d4 c0633094 00000000 c0680000 c6104000 c6104000 00000000
GPR24: 00000200 00000000 ffffffff 00000004 00000078 00009032 00000000 c62b4c00
NIP [c02a84bc] mdiobus_read+0x20/0x74
[92708.281517] LR [c02a947c] kszphy_config_intr+0xa4/0xc4
[92708.286547] Call Trace:
[92708.288980] [c6105d20] [c6104000] 0xc6104000 (unreliable)
[92708.294339] [c6105d40] [c02a947c] kszphy_config_intr+0xa4/0xc4
[92708.300098] [c6105d50] [c02a5330] phy_stop+0x60/0x9c
[92708.305007] [c6105d60] [c02c84d0] fs_timeout+0xdc/0x110
[92708.310197] [c6105d80] [c035cd48] dev_watchdog+0x268/0x2a0
[92708.315593] [c6105db0] [c0060288] call_timer_fn+0x34/0x17c
[92708.321014] [c6105dd0] [c00605f0] run_timer_softirq+0x21c/0x2e4
[92708.326887] [c6105e50] [c001e19c] __do_softirq+0xf4/0x2f4
[92708.332207] [c6105eb0] [c001e3c8] run_ksoftirqd+0x2c/0x40
[92708.337560] [c6105ec0] [c003b420] smpboot_thread_fn+0x1f0/0x258
[92708.343405] [c6105ef0] [c003745c] kthread+0xbc/0xd0
[92708.348217] [c6105f40] [c000c400] ret_from_kernel_thread+0x5c/0x64
[92708.354275] Instruction dump:
[92708.357207] 7c0803a6 bbc10018 38210020 4e800020 7c0802a6 9421ffe0 54290024 bfc10018
[92708.364865] 90010024 7c7f1b78 81290008 552902ee <0f090000> 3bc3002c 7fc3f378 90810008
[92708.372711] ---[ end trace 42b05441616fafd7 ]---

This patch moves fs_timeout() actions into an async worker.

Fixes: commit 48257c4f168e5 ("Add fs_enet ethernet network driver, for several embedded platforms")
Signed-off-by: Christophe Leroy <christophe.leroy@....fr>
Signed-off-by: David S. Miller <davem@...emloft.net>
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 16 +++++++++++++---
 drivers/net/ethernet/freescale/fs_enet/fs_enet.h      |  1 +
 2 files changed, 14 insertions(+), 3 deletions(-)

--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -696,9 +696,11 @@ static int fs_enet_start_xmit(struct sk_
 	return NETDEV_TX_OK;
 }
 
-static void fs_timeout(struct net_device *dev)
+static void fs_timeout_work(struct work_struct *work)
 {
-	struct fs_enet_private *fep = netdev_priv(dev);
+	struct fs_enet_private *fep = container_of(work, struct fs_enet_private,
+						   timeout_work);
+	struct net_device *dev = fep->ndev;
 	unsigned long flags;
 	int wake = 0;
 
@@ -710,7 +712,6 @@ static void fs_timeout(struct net_device
 		phy_stop(fep->phydev);
 		(*fep->ops->stop)(dev);
 		(*fep->ops->restart)(dev);
-		phy_start(fep->phydev);
 	}
 
 	phy_start(fep->phydev);
@@ -721,6 +722,13 @@ static void fs_timeout(struct net_device
 		netif_wake_queue(dev);
 }
 
+static void fs_timeout(struct net_device *dev)
+{
+	struct fs_enet_private *fep = netdev_priv(dev);
+
+	schedule_work(&fep->timeout_work);
+}
+
 /*-----------------------------------------------------------------------------
  *  generic link-change handler - should be sufficient for most cases
  *-----------------------------------------------------------------------------*/
@@ -847,6 +855,7 @@ static int fs_enet_close(struct net_devi
 	netif_carrier_off(dev);
 	if (fep->fpi->use_napi)
 		napi_disable(&fep->napi);
+	cancel_work_sync(&fep->timeout_work);
 	phy_stop(fep->phydev);
 
 	spin_lock_irqsave(&fep->lock, flags);
@@ -1102,6 +1111,7 @@ static int fs_enet_probe(struct platform
 
 	ndev->netdev_ops = &fs_enet_netdev_ops;
 	ndev->watchdog_timeo = 2 * HZ;
+	INIT_WORK(&fep->timeout_work, fs_timeout_work);
 	if (fpi->use_napi)
 		netif_napi_add(ndev, &fep->napi, fs_enet_rx_napi,
 		               fpi->napi_weight);
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet.h
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet.h
@@ -124,6 +124,7 @@ struct fs_enet_private {
 	spinlock_t lock;	/* during all ops except TX pckt processing */
 	spinlock_t tx_lock;	/* during fs_start_xmit and fs_tx         */
 	struct fs_platform_info *fpi;
+	struct work_struct timeout_work;
 	const struct fs_ops *ops;
 	int rx_ring, tx_ring;
 	dma_addr_t ring_mem_addr;