• Vlad Buslov's avatar
    net: sched: flower: insert new filter to idr after setting its mask · ecb3dea4
    Vlad Buslov authored
    When adding new filter to flower classifier, fl_change() inserts it to
    handle_idr before initializing filter extensions and assigning it a mask.
    Normally this ordering doesn't matter because all flower classifier ops
    callbacks assume rtnl lock protection. However, when filter has an action
    that doesn't have its kernel module loaded, rtnl lock is released before
    call to request_module(). During this time the filter can be accessed bu
    concurrent task before its initialization is completed, which can lead to a
    crash.
    
    Example case of NULL pointer dereference in concurrent dump:
    
    Task 1                           Task 2
    
    tc_new_tfilter()
     fl_change()
      idr_alloc_u32(fnew)
      fl_set_parms()
       tcf_exts_validate()
        tcf_action_init()
         tcf_action_init_1()
          rtnl_unlock()
          request_module()
          ...                        rtnl_lock()
          				 tc_dump_tfilter()
          				  tcf_chain_dump()
    				   fl_walk()
    				    idr_get_next_ul()
    				    tcf_node_dump()
    				     tcf_fill_node()
    				      fl_dump()
    				       mask = &f->mask->key; <- NULL ptr
          rtnl_lock()
    
    Extension initialization and mask assignment don't depend on fnew->handle
    that is allocated by idr_alloc_u32(). Move idr allocation code after action
    creation and mask assignment in fl_change() to prevent concurrent access
    to not fully initialized filter when rtnl lock is released to load action
    module.
    
    Fixes: 01683a14 ("net: sched: refactor flower walk to iterate over idr")
    Signed-off-by: default avatarVlad Buslov <vladbu@mellanox.com>
    Reviewed-by: default avatarRoi Dayan <roid@mellanox.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    ecb3dea4
cls_flower.c 62.7 KB