• Eric Dumazet's avatar
    tcp: properly terminate timers for kernel sockets · 151c9c72
    Eric Dumazet authored
    We had various syzbot reports about tcp timers firing after
    the corresponding netns has been dismantled.
    
    Fortunately Josef Bacik could trigger the issue more often,
    and could test a patch I wrote two years ago.
    
    When TCP sockets are closed, we call inet_csk_clear_xmit_timers()
    to 'stop' the timers.
    
    inet_csk_clear_xmit_timers() can be called from any context,
    including when socket lock is held.
    This is the reason it uses sk_stop_timer(), aka del_timer().
    This means that ongoing timers might finish much later.
    
    For user sockets, this is fine because each running timer
    holds a reference on the socket, and the user socket holds
    a reference on the netns.
    
    For kernel sockets, we risk that the netns is freed before
    timer can complete, because kernel sockets do not hold
    reference on the netns.
    
    This patch adds inet_csk_clear_xmit_timers_sync() function
    that using sk_stop_timer_sync() to make sure all timers
    are terminated before the kernel socket is released.
    Modules using kernel sockets close them in their netns exit()
    handler.
    
    Also add sock_not_owned_by_me() helper to get LOCKDEP
    support : inet_csk_clear_xmit_timers_sync() must not be called
    while socket lock is held.
    
    It is very possible we can revert in the future commit
    3a58f13a ("net: rds: acquire refcount on TCP sockets")
    which attempted to solve the issue in rds only.
    (net/smc/af_smc.c and net/mptcp/subflow.c have similar code)
    
    We probably can remove the check_net() tests from
    tcp_out_of_resources() and __tcp_close() in the future.
    Reported-by: default avatarJosef Bacik <josef@toxicpanda.com>
    Closes: https://lore.kernel.org/netdev/20240314210740.GA2823176@perftesting/
    Fixes: 26abe143 ("net: Modify sk_alloc to not reference count the netns of kernel sockets.")
    Fixes: 8a681736 ("net: sk_clone_lock() should only do get_net() if the parent is not a kernel socket")
    Link: https://lore.kernel.org/bpf/CANn89i+484ffqb93aQm1N-tjxxvb3WDKX0EbD7318RwRgsatjw@mail.gmail.com/Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
    Tested-by: default avatarJosef Bacik <josef@toxicpanda.com>
    Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Link: https://lore.kernel.org/r/20240322135732.1535772-1-edumazet@google.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
    151c9c72
inet_connection_sock.h 11.7 KB