On Wed, Sep 21 2016 at 10:22am -0400,
Rabin Vincent <rabin.vincent@xxxxxxxx> wrote:
> From: Rabin Vincent <rabinv@xxxxxxxx>
>
> 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 bcbd94ff481e ("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: bcbd94ff481e ("dm crypt: fix a possible hang due to race condition on exit")
> Cc: Mikulas Patocka <mpatocka@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx # v4.0+
> Signed-off-by: Rabin Vincent <rabinv@xxxxxxxx>
Thanks, I've staged this for the 4.9 merge window.
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
