net: sfp: add mutex to prevent concurrent state checks
authorRobert Hancock <hancock@sedsystems.ca>
Fri, 7 Jun 2019 16:42:36 +0000 (10:42 -0600)
committerDavid S. Miller <davem@davemloft.net>
Mon, 10 Jun 2019 02:25:59 +0000 (19:25 -0700)
sfp_check_state can potentially be called by both a threaded IRQ handler
and delayed work. If it is concurrently called, it could result in
incorrect state management. Add a st_mutex to protect the state - this
lock gets taken outside of code that checks and handle state changes, and
the existing sm_mutex nests inside of it.

Suggested-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/phy/sfp.c

index 44d787d..97960d7 100644 (file)
@@ -188,10 +188,11 @@ struct sfp {
        int gpio_irq[GPIO_MAX];
 
        bool attached;
+       struct mutex st_mutex;                  /* Protects state */
        unsigned int state;
        struct delayed_work poll;
        struct delayed_work timeout;
-       struct mutex sm_mutex;
+       struct mutex sm_mutex;                  /* Protects state machine */
        unsigned char sm_mod_state;
        unsigned char sm_dev_state;
        unsigned short sm_state;
@@ -1721,6 +1722,7 @@ static void sfp_check_state(struct sfp *sfp)
 {
        unsigned int state, i, changed;
 
+       mutex_lock(&sfp->st_mutex);
        state = sfp_get_state(sfp);
        changed = state ^ sfp->state;
        changed &= SFP_F_PRESENT | SFP_F_LOS | SFP_F_TX_FAULT;
@@ -1746,6 +1748,7 @@ static void sfp_check_state(struct sfp *sfp)
                sfp_sm_event(sfp, state & SFP_F_LOS ?
                                SFP_E_LOS_HIGH : SFP_E_LOS_LOW);
        rtnl_unlock();
+       mutex_unlock(&sfp->st_mutex);
 }
 
 static irqreturn_t sfp_irq(int irq, void *data)
@@ -1776,6 +1779,7 @@ static struct sfp *sfp_alloc(struct device *dev)
        sfp->dev = dev;
 
        mutex_init(&sfp->sm_mutex);
+       mutex_init(&sfp->st_mutex);
        INIT_DELAYED_WORK(&sfp->poll, sfp_poll);
        INIT_DELAYED_WORK(&sfp->timeout, sfp_timeout);