• Duoming Zhou's avatar
    ax25: Fix ax25 session cleanup problems · 7d8a3a47
    Duoming Zhou authored
    There are session cleanup problems in ax25_release() and
    ax25_disconnect(). If we setup a session and then disconnect,
    the disconnected session is still in "LISTENING" state that
    is shown below.
    
    Active AX.25 sockets
    Dest       Source     Device  State        Vr/Vs    Send-Q  Recv-Q
    DL9SAU-4   DL9SAU-3   ???     LISTENING    000/000  0       0
    DL9SAU-3   DL9SAU-4   ???     LISTENING    000/000  0       0
    
    The first reason is caused by del_timer_sync() in ax25_release().
    The timers of ax25 are used for correct session cleanup. If we use
    ax25_release() to close ax25 sessions and ax25_dev is not null,
    the del_timer_sync() functions in ax25_release() will execute.
    As a result, the sessions could not be cleaned up correctly,
    because the timers have stopped.
    
    In order to solve this problem, this patch adds a device_up flag
    in ax25_dev in order to judge whether the device is up. If there
    are sessions to be cleaned up, the del_timer_sync() in
    ax25_release() will not execute. What's more, we add ax25_cb_del()
    in ax25_kill_by_device(), because the timers have been stopped
    and there are no functions that could delete ax25_cb if we do not
    call ax25_release(). Finally, we reorder the position of
    ax25_list_lock in ax25_cb_del() in order to synchronize among
    different functions that call ax25_cb_del().
    
    The second reason is caused by improper check in ax25_disconnect().
    The incoming ax25 sessions which ax25->sk is null will close
    heartbeat timer, because the check "if(!ax25->sk || ..)" is
    satisfied. As a result, the session could not be cleaned up properly.
    
    In order to solve this problem, this patch changes the improper
    check to "if(ax25->sk && ..)" in ax25_disconnect().
    
    What`s more, the ax25_disconnect() may be called twice, which is
    not necessary. For example, ax25_kill_by_device() calls
    ax25_disconnect() and sets ax25->state to AX25_STATE_0, but
    ax25_release() calls ax25_disconnect() again.
    
    In order to solve this problem, this patch add a check in
    ax25_release(). If the flag of ax25->sk equals to SOCK_DEAD,
    the ax25_disconnect() in ax25_release() should not be executed.
    
    Fixes: 82e31755 ("ax25: Fix UAF bugs in ax25 timers")
    Fixes: 8a367e74 ("ax25: Fix segfault after sock connection timeout")
    Reported-and-tested-by: default avatarThomas Osterried <thomas@osterried.de>
    Signed-off-by: default avatarDuoming Zhou <duoming@zju.edu.cn>
    Link: https://lore.kernel.org/r/20220530152158.108619-1-duoming@zju.edu.cnSigned-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
    7d8a3a47
ax25_dev.c 4.95 KB