linux-kernel - [PATCH] net: phy: Tell caller result of 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]

Message-ID: <20180308203157.10855-1-brad.mouring@ni.com>
Date:   Thu, 8 Mar 2018 14:31:57 -0600
From:   Brad Mouring <brad.mouring@...com>
To:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>
CC:     <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
        Brad Mouring <brad.mouring@...com>
Subject: [PATCH] net: phy: Tell caller result of phy_change()

In 664fcf123a30e (net: phy: Threaded interrupts allow some simplification)
the phy_interrupt system was changed to use a traditional threaded
interrupt scheme instead of a workqueue approach.

With this change, the phy status check moved into phy_change, which
did not report back to the caller whether or not the interrupt was
handled. This means that, in the case of a shared phy interrupt,
only the first phydev's interrupt registers are checked (since
phy_interrupt() would always return IRQ_HANDLED). This leads to
interrupt storms when it is a secondary device that's actually the
interrupt source.

Signed-off-by: Brad Mouring <brad.mouring@...com>
---
 drivers/net/phy/phy.c | 13 +++++++------
 include/linux/phy.h   |  1 -
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index e3e29c2b028b..ebce9f2b2742 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -617,6 +617,8 @@ static void phy_error(struct phy_device *phydev)
 	phy_trigger_machine(phydev, false);
 }
 
+static irqreturn_t phy_change(struct phy_device *phydev);
+
 /**
  * phy_interrupt - PHY interrupt handler
  * @irq: interrupt line
@@ -632,9 +634,7 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
 	if (PHY_HALTED == phydev->state)
 		return IRQ_NONE;		/* It can't be ours.  */
 
-	phy_change(phydev);
-
-	return IRQ_HANDLED;
+	return phy_change(phydev);
 }
 
 /**
@@ -723,12 +723,12 @@ EXPORT_SYMBOL(phy_stop_interrupts);
  * phy_change - Called by the phy_interrupt to handle PHY changes
  * @phydev: phy_device struct that interrupted
  */
-void phy_change(struct phy_device *phydev)
+static irqreturn_t phy_change(struct phy_device *phydev)
 {
 	if (phy_interrupt_is_valid(phydev)) {
 		if (phydev->drv->did_interrupt &&
 		    !phydev->drv->did_interrupt(phydev))
-			return;
+			return IRQ_NONE;
 
 		if (phydev->state == PHY_HALTED)
 			if (phy_disable_interrupts(phydev))
@@ -745,10 +745,11 @@ void phy_change(struct phy_device *phydev)
 
 	if (phy_interrupt_is_valid(phydev) && phy_clear_interrupt(phydev))
 		goto phy_err;
-	return;
+	return IRQ_HANDLED;
 
 phy_err:
 	phy_error(phydev);
+	return IRQ_NONE;
 }
 
 /**
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5a0c3e53e7c2..2e55838655b9 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1011,7 +1011,6 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner);
 int phy_drivers_register(struct phy_driver *new_driver, int n,
 			 struct module *owner);
 void phy_state_machine(struct work_struct *work);
-void phy_change(struct phy_device *phydev);
 void phy_change_work(struct work_struct *work);
 void phy_mac_interrupt(struct phy_device *phydev);
 void phy_start_machine(struct phy_device *phydev);
-- 
2.16.2