• Vladimir Oltean's avatar
    net: dsa: Fix use-after-free in probing of DSA switch tree · 6dc43cd3
    Vladimir Oltean authored
    DSA sets up a switch tree little by little. Every switch of the N
    members of the tree calls dsa_register_switch, and (N - 1) will just
    touch the dst->ports list with their ports and quickly exit. Only the
    last switch that calls dsa_register_switch will find all DSA links
    complete in dsa_tree_setup_routing_table, and not return zero as a
    result but instead go ahead and set up the entire DSA switch tree
    (practically on behalf of the other switches too).
    
    The trouble is that the (N - 1) switches don't clean up after themselves
    after they get an error such as EPROBE_DEFER. Their footprint left in
    dst->ports by dsa_switch_touch_ports is still there. And switch N, the
    one responsible with actually setting up the tree, is going to work with
    those stale dp, dp->ds and dp->ds->dev pointers. In particular ds and
    ds->dev might get freed by the device driver.
    
    Be there a 2-switch tree and the following calling order:
    - Switch 1 calls dsa_register_switch
      - Calls dsa_switch_touch_ports, populates dst->ports
      - Calls dsa_port_parse_cpu, gets -EPROBE_DEFER, exits.
    - Switch 2 calls dsa_register_switch
      - Calls dsa_switch_touch_ports, populates dst->ports
      - Probe doesn't get deferred, so it goes ahead.
      - Calls dsa_tree_setup_routing_table, which returns "complete == true"
        due to Switch 1 having called dsa_switch_touch_ports before.
      - Because the DSA links are complete, it calls dsa_tree_setup_switches
        now.
      - dsa_tree_setup_switches iterates through dst->ports, initializing
        the Switch 1 ds structure (invalid) and the Switch 2 ds structure
        (valid).
      - Undefined behavior (use after free, sometimes NULL pointers, etc).
    
    Real example below (debugging prints added by me, as well as guards
    against NULL pointers):
    
    [    5.477947] dsa_tree_setup_switches: Setting up port 0 of switch ffffff803df0b980 (dev ffffff803f775c00)
    [    6.313002] dsa_tree_setup_switches: Setting up port 1 of switch ffffff803df0b980 (dev ffffff803f775c00)
    [    6.319932] dsa_tree_setup_switches: Setting up port 2 of switch ffffff803df0b980 (dev ffffff803f775c00)
    [    6.329693] dsa_tree_setup_switches: Setting up port 3 of switch ffffff803df0b980 (dev ffffff803f775c00)
    [    6.339458] dsa_tree_setup_switches: Setting up port 4 of switch ffffff803df0b980 (dev ffffff803f775c00)
    [    6.349226] dsa_tree_setup_switches: Setting up port 5 of switch ffffff803df0b980 (dev ffffff803f775c00)
    [    6.358991] dsa_tree_setup_switches: Setting up port 6 of switch ffffff803df0b980 (dev ffffff803f775c00)
    [    6.368758] dsa_tree_setup_switches: Setting up port 7 of switch ffffff803df0b980 (dev ffffff803f775c00)
    [    6.378524] dsa_tree_setup_switches: Setting up port 8 of switch ffffff803df0b980 (dev ffffff803f775c00)
    [    6.388291] dsa_tree_setup_switches: Setting up port 9 of switch ffffff803df0b980 (dev ffffff803f775c00)
    [    6.398057] dsa_tree_setup_switches: Setting up port 10 of switch ffffff803df0b980 (dev ffffff803f775c00)
    [    6.407912] dsa_tree_setup_switches: Setting up port 0 of switch ffffff803da02f80 (dev 0000000000000000)
    [    6.417682] dsa_tree_setup_switches: Setting up port 1 of switch ffffff803da02f80 (dev 0000000000000000)
    [    6.427446] dsa_tree_setup_switches: Setting up port 2 of switch ffffff803da02f80 (dev 0000000000000000)
    [    6.437212] dsa_tree_setup_switches: Setting up port 3 of switch ffffff803da02f80 (dev 0000000000000000)
    [    6.446979] dsa_tree_setup_switches: Setting up port 4 of switch ffffff803da02f80 (dev 0000000000000000)
    [    6.456744] dsa_tree_setup_switches: Setting up port 5 of switch ffffff803da02f80 (dev 0000000000000000)
    [    6.466512] dsa_tree_setup_switches: Setting up port 6 of switch ffffff803da02f80 (dev 0000000000000000)
    [    6.476277] dsa_tree_setup_switches: Setting up port 7 of switch ffffff803da02f80 (dev 0000000000000000)
    [    6.486043] dsa_tree_setup_switches: Setting up port 8 of switch ffffff803da02f80 (dev 0000000000000000)
    [    6.495810] dsa_tree_setup_switches: Setting up port 9 of switch ffffff803da02f80 (dev 0000000000000000)
    [    6.505577] dsa_tree_setup_switches: Setting up port 10 of switch ffffff803da02f80 (dev 0000000000000000)
    [    6.515433] dsa_tree_setup_switches: Setting up port 0 of switch ffffff803db15b80 (dev ffffff803d8e4800)
    [    7.354120] dsa_tree_setup_switches: Setting up port 1 of switch ffffff803db15b80 (dev ffffff803d8e4800)
    [    7.361045] dsa_tree_setup_switches: Setting up port 2 of switch ffffff803db15b80 (dev ffffff803d8e4800)
    [    7.370805] dsa_tree_setup_switches: Setting up port 3 of switch ffffff803db15b80 (dev ffffff803d8e4800)
    [    7.380571] dsa_tree_setup_switches: Setting up port 4 of switch ffffff803db15b80 (dev ffffff803d8e4800)
    [    7.390337] dsa_tree_setup_switches: Setting up port 5 of switch ffffff803db15b80 (dev ffffff803d8e4800)
    [    7.400104] dsa_tree_setup_switches: Setting up port 6 of switch ffffff803db15b80 (dev ffffff803d8e4800)
    [    7.409872] dsa_tree_setup_switches: Setting up port 7 of switch ffffff803db15b80 (dev ffffff803d8e4800)
    [    7.419637] dsa_tree_setup_switches: Setting up port 8 of switch ffffff803db15b80 (dev ffffff803d8e4800)
    [    7.429403] dsa_tree_setup_switches: Setting up port 9 of switch ffffff803db15b80 (dev ffffff803d8e4800)
    [    7.439169] dsa_tree_setup_switches: Setting up port 10 of switch ffffff803db15b80 (dev ffffff803d8e4800)
    
    The solution is to recognize that the functions that call
    dsa_switch_touch_ports (dsa_switch_parse_of, dsa_switch_parse) have side
    effects, and therefore one should clean up their side effects on error
    path. The cleanup of dst->ports was taken from dsa_switch_remove and
    moved into a dedicated dsa_switch_release_ports function, which should
    really be per-switch (free only the members of dst->ports that are also
    members of ds, instead of all switch ports).
    Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    6dc43cd3
dsa2.c 18.3 KB