• Duoming Zhou's avatar
    ax25: Fix NULL pointer dereferences in ax25 timers · fc6d01ff
    Duoming Zhou authored
    The previous commit 7ec02f5a ("ax25: fix NPD bug in ax25_disconnect")
    move ax25_disconnect into lock_sock() in order to prevent NPD bugs. But
    there are race conditions that may lead to null pointer dereferences in
    ax25_heartbeat_expiry(), ax25_t1timer_expiry(), ax25_t2timer_expiry(),
    ax25_t3timer_expiry() and ax25_idletimer_expiry(), when we use
    ax25_kill_by_device() to detach the ax25 device.
    
    One of the race conditions that cause null pointer dereferences can be
    shown as below:
    
          (Thread 1)                    |      (Thread 2)
    ax25_connect()                      |
     ax25_std_establish_data_link()     |
      ax25_start_t1timer()              |
       mod_timer(&ax25->t1timer,..)     |
                                        | ax25_kill_by_device()
       (wait a time)                    |  ...
                                        |  s->ax25_dev = NULL; //(1)
       ax25_t1timer_expiry()            |
        ax25->ax25_dev->values[..] //(2)|  ...
         ...                            |
    
    We set null to ax25_cb->ax25_dev in position (1) and dereference
    the null pointer in position (2).
    
    The corresponding fail log is shown below:
    ===============================================================
    BUG: kernel NULL pointer dereference, address: 0000000000000050
    CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.17.0-rc6-00794-g45690b7d0
    RIP: 0010:ax25_t1timer_expiry+0x12/0x40
    ...
    Call Trace:
     call_timer_fn+0x21/0x120
     __run_timers.part.0+0x1ca/0x250
     run_timer_softirq+0x2c/0x60
     __do_softirq+0xef/0x2f3
     irq_exit_rcu+0xb6/0x100
     sysvec_apic_timer_interrupt+0xa2/0xd0
    ...
    
    This patch moves ax25_disconnect() before s->ax25_dev = NULL
    and uses del_timer_sync() to delete timers in ax25_disconnect().
    If ax25_disconnect() is called by ax25_kill_by_device() or
    ax25->ax25_dev is NULL, the reason in ax25_disconnect() will be
    equal to ENETUNREACH, it will wait all timers to stop before we
    set null to s->ax25_dev in ax25_kill_by_device().
    
    Fixes: 7ec02f5a ("ax25: fix NPD bug in ax25_disconnect")
    Signed-off-by: default avatarDuoming Zhou <duoming@zju.edu.cn>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    fc6d01ff
ax25_subr.c 7.04 KB