lkml.org 
[lkml]   [2020]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.9 008/757] net: dsa: microchip: fix race condition
    Date
    From: Christian Eggers <ceggers@arri.de>

    [ Upstream commit 8098bd69bc4e925070313b1b95d03510f4f24738 ]

    Between queuing the delayed work and finishing the setup of the dsa
    ports, the process may sleep in request_module() (via
    phy_device_create()) and the queued work may be executed prior to the
    switch net devices being registered. In ksz_mib_read_work(), a NULL
    dereference will happen within netof_carrier_ok(dp->slave).

    Not queuing the delayed work in ksz_init_mib_timer() makes things even
    worse because the work will now be queued for immediate execution
    (instead of 2000 ms) in ksz_mac_link_down() via
    dsa_port_link_register_of().

    Call tree:
    ksz9477_i2c_probe()
    \--ksz9477_switch_register()
    \--ksz_switch_register()
    +--dsa_register_switch()
    | \--dsa_switch_probe()
    | \--dsa_tree_setup()
    | \--dsa_tree_setup_switches()
    | +--dsa_switch_setup()
    | | +--ksz9477_setup()
    | | | \--ksz_init_mib_timer()
    | | | |--/* Start the timer 2 seconds later. */
    | | | \--schedule_delayed_work(&dev->mib_read, msecs_to_jiffies(2000));
    | | \--__mdiobus_register()
    | | \--mdiobus_scan()
    | | \--get_phy_device()
    | | +--get_phy_id()
    | | \--phy_device_create()
    | | |--/* sleeping, ksz_mib_read_work() can be called meanwhile */
    | | \--request_module()
    | |
    | \--dsa_port_setup()
    | +--/* Called for non-CPU ports */
    | +--dsa_slave_create()
    | | +--/* Too late, ksz_mib_read_work() may be called beforehand */
    | | \--port->slave = ...
    | ...
    | +--Called for CPU port */
    | \--dsa_port_link_register_of()
    | \--ksz_mac_link_down()
    | +--/* mib_read must be initialized here */
    | +--/* work is already scheduled, so it will be executed after 2000 ms */
    | \--schedule_delayed_work(&dev->mib_read, 0);
    \-- /* here port->slave is setup properly, scheduling the delayed work should be safe */

    Solution:
    1. Do not queue (only initialize) delayed work in ksz_init_mib_timer().
    2. Only queue delayed work in ksz_mac_link_down() if init is completed.
    3. Queue work once in ksz_switch_register(), after dsa_register_switch()
    has completed.

    Fixes: 7c6ff470aa86 ("net: dsa: microchip: add MIB counter reading support")
    Signed-off-by: Christian Eggers <ceggers@arri.de>
    Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
    Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
    Signed-off-by: Jakub Kicinski <kuba@kernel.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/net/dsa/microchip/ksz_common.c | 16 +++++++++-------
    1 file changed, 9 insertions(+), 7 deletions(-)

    --- a/drivers/net/dsa/microchip/ksz_common.c
    +++ b/drivers/net/dsa/microchip/ksz_common.c
    @@ -103,14 +103,8 @@ void ksz_init_mib_timer(struct ksz_devic

    INIT_DELAYED_WORK(&dev->mib_read, ksz_mib_read_work);

    - /* Read MIB counters every 30 seconds to avoid overflow. */
    - dev->mib_read_interval = msecs_to_jiffies(30000);
    -
    for (i = 0; i < dev->mib_port_cnt; i++)
    dev->dev_ops->port_init_cnt(dev, i);
    -
    - /* Start the timer 2 seconds later. */
    - schedule_delayed_work(&dev->mib_read, msecs_to_jiffies(2000));
    }
    EXPORT_SYMBOL_GPL(ksz_init_mib_timer);

    @@ -143,7 +137,9 @@ void ksz_mac_link_down(struct dsa_switch

    /* Read all MIB counters when the link is going down. */
    p->read = true;
    - schedule_delayed_work(&dev->mib_read, 0);
    + /* timer started */
    + if (dev->mib_read_interval)
    + schedule_delayed_work(&dev->mib_read, 0);
    }
    EXPORT_SYMBOL_GPL(ksz_mac_link_down);

    @@ -450,6 +446,12 @@ int ksz_switch_register(struct ksz_devic
    return ret;
    }

    + /* Read MIB counters every 30 seconds to avoid overflow. */
    + dev->mib_read_interval = msecs_to_jiffies(30000);
    +
    + /* Start the MIB timer. */
    + schedule_delayed_work(&dev->mib_read, 0);
    +
    return 0;
    }
    EXPORT_SYMBOL(ksz_switch_register);

    \
     
     \ /
      Last update: 2020-10-27 17:11    [W:2.506 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site