• WANG Cong's avatar
    net_sched: move the empty tp check from ->destroy() to ->delete() · 763dbf63
    WANG Cong authored
    We could have a race condition where in ->classify() path we
    dereference tp->root and meanwhile a parallel ->destroy() makes it
    a NULL. Daniel cured this bug in commit d9363774
    ("net, sched: respect rcu grace period on cls destruction").
    
    This happens when ->destroy() is called for deleting a filter to
    check if we are the last one in tp, this tp is still linked and
    visible at that time. The root cause of this problem is the semantic
    of ->destroy(), it does two things (for non-force case):
    
    1) check if tp is empty
    2) if tp is empty we could really destroy it
    
    and its caller, if cares, needs to check its return value to see if it
    is really destroyed. Therefore we can't unlink tp unless we know it is
    empty.
    
    As suggested by Daniel, we could actually move the test logic to ->delete()
    so that we can safely unlink tp after ->delete() tells us the last one is
    just deleted and before ->destroy().
    
    Fixes: 1e052be6 ("net_sched: destroy proto tp when all filters are gone")
    Cc: Roi Dayan <roid@mellanox.com>
    Cc: Daniel Borkmann <daniel@iogearbox.net>
    Cc: John Fastabend <john.fastabend@gmail.com>
    Cc: Jamal Hadi Salim <jhs@mojatatu.com>
    Signed-off-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
    Acked-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    763dbf63
cls_flow.c 16.4 KB