• Rabin Vincent's avatar
    dm crypt: fix crash on exit · acc97010
    Rabin Vincent authored
    commit f659b100 upstream.
    
    As the documentation for kthread_stop() says, "if threadfn() may call
    do_exit() itself, the caller must ensure task_struct can't go away".
    dm-crypt does not ensure this and therefore crashes when crypt_dtr()
    calls kthread_stop().  The crash is trivially reproducible by adding a
    delay before the call to kthread_stop() and just opening and closing a
    dm-crypt device.
    
     general protection fault: 0000 [#1] PREEMPT SMP
     CPU: 0 PID: 533 Comm: cryptsetup Not tainted 4.8.0-rc7+ #7
     task: ffff88003bd0df40 task.stack: ffff8800375b4000
     RIP: 0010: kthread_stop+0x52/0x300
     Call Trace:
      crypt_dtr+0x77/0x120
      dm_table_destroy+0x6f/0x120
      __dm_destroy+0x130/0x250
      dm_destroy+0x13/0x20
      dev_remove+0xe6/0x120
      ? dev_suspend+0x250/0x250
      ctl_ioctl+0x1fc/0x530
      ? __lock_acquire+0x24f/0x1b10
      dm_ctl_ioctl+0x13/0x20
      do_vfs_ioctl+0x91/0x6a0
      ? ____fput+0xe/0x10
      ? entry_SYSCALL_64_fastpath+0x5/0xbd
      ? trace_hardirqs_on_caller+0x151/0x1e0
      SyS_ioctl+0x41/0x70
      entry_SYSCALL_64_fastpath+0x1f/0xbd
    
    This problem was introduced by bcbd94ff ("dm crypt: fix a possible
    hang due to race condition on exit").
    
    Looking at the description of that patch (excerpted below), it seems
    like the problem it addresses can be solved by just using
    set_current_state instead of __set_current_state, since we obviously
    need the memory barrier.
    
    | dm crypt: fix a possible hang due to race condition on exit
    |
    | A kernel thread executes __set_current_state(TASK_INTERRUPTIBLE),
    | __add_wait_queue, spin_unlock_irq and then tests kthread_should_stop().
    | It is possible that the processor reorders memory accesses so that
    | kthread_should_stop() is executed before __set_current_state().  If
    | such reordering happens, there is a possible race on thread
    | termination: [...]
    
    So this patch just reverts the aforementioned patch and changes the
    __set_current_state(TASK_INTERRUPTIBLE) to set_current_state(...).  This
    fixes the crash and should also fix the potential hang.
    
    Fixes: bcbd94ff ("dm crypt: fix a possible hang due to race condition on exit")
    Cc: Mikulas Patocka <mpatocka@redhat.com>
    Signed-off-by: default avatarRabin Vincent <rabinv@axis.com>
    Signed-off-by: default avatarMike Snitzer <snitzer@redhat.com>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    acc97010
dm-crypt.c 50.6 KB