• Christian Eggers's avatar
    net: dsa: microchip: fix race condition · 8098bd69
    Christian Eggers authored
    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: 7c6ff470 ("net: dsa: microchip: add MIB counter reading support")
    Signed-off-by: default avatarChristian Eggers <ceggers@arri.de>
    Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
    Reviewed-by: default avatarVladimir Oltean <olteanv@gmail.com>
    Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
    8098bd69
ksz_common.c 11 KB