• Vladimir Oltean's avatar
    net: dsa: felix: suppress non-changes to the tagging protocol · 4c46bb49
    Vladimir Oltean authored
    The way in which dsa_tree_change_tag_proto() works is that when
    dsa_tree_notify() fails, it doesn't know whether the operation failed
    mid way in a multi-switch tree, or it failed for a single-switch tree.
    So even though drivers need to fail cleanly in
    ds->ops->change_tag_protocol(), DSA will still call dsa_tree_notify()
    again, to restore the old tag protocol for potential switches in the
    tree where the change did succeeed (before failing for others).
    
    This means for the felix driver that if we report an error in
    felix_change_tag_protocol(), we'll get another call where proto_ops ==
    old_proto_ops. If we proceed to act upon that, we may do unexpected
    things. For example, we will call dsa_tag_8021q_register() twice in a
    row, without any dsa_tag_8021q_unregister() in between. Then we will
    actually call dsa_tag_8021q_unregister() via old_proto_ops->teardown,
    which (if it manages to run at all, after walking through corrupted data
    structures) will leave the ports inoperational anyway.
    
    The bug can be readily reproduced if we force an error while in
    tag_8021q mode; this crashes the kernel.
    
    echo ocelot-8021q > /sys/class/net/eno2/dsa/tagging
    echo edsa > /sys/class/net/eno2/dsa/tagging # -EPROTONOSUPPORT
    
    Unable to handle kernel NULL pointer dereference at virtual address 0000000000000014
    Call trace:
     vcap_entry_get+0x24/0x124
     ocelot_vcap_filter_del+0x198/0x270
     felix_tag_8021q_vlan_del+0xd4/0x21c
     dsa_switch_tag_8021q_vlan_del+0x168/0x2cc
     dsa_switch_event+0x68/0x1170
     dsa_tree_notify+0x14/0x34
     dsa_port_tag_8021q_vlan_del+0x84/0x110
     dsa_tag_8021q_unregister+0x15c/0x1c0
     felix_tag_8021q_teardown+0x16c/0x180
     felix_change_tag_protocol+0x1bc/0x230
     dsa_switch_event+0x14c/0x1170
     dsa_tree_change_tag_proto+0x118/0x1c0
    
    Fixes: 7a29d220 ("net: dsa: felix: reimplement tagging protocol change with function pointers")
    Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
    Link: https://lore.kernel.org/r/20220808125127.3344094-1-vladimir.oltean@nxp.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
    4c46bb49
felix.c 51.5 KB