Skip to content

Commit a35c9d6

Browse files
vwaxgregkh
authored andcommitted
dm crypt: fix crash on exit
commit f659b10 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 bcbd94f ("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: bcbd94f ("dm crypt: fix a possible hang due to race condition on exit") Cc: Mikulas Patocka <mpatocka@redhat.com> Signed-off-by: Rabin Vincent <rabinv@axis.com> Signed-off-by: Mike Snitzer <snitzer@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 293441e commit a35c9d6

File tree

1 file changed

+10
-14
lines changed

1 file changed

+10
-14
lines changed

drivers/md/dm-crypt.c

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,7 @@ struct iv_tcw_private {
112112
* and encrypts / decrypts at the same time.
113113
*/
114114
enum flags { DM_CRYPT_SUSPENDED, DM_CRYPT_KEY_VALID,
115-
DM_CRYPT_SAME_CPU, DM_CRYPT_NO_OFFLOAD,
116-
DM_CRYPT_EXIT_THREAD};
115+
DM_CRYPT_SAME_CPU, DM_CRYPT_NO_OFFLOAD };
117116

118117
/*
119118
* The fields in here must be read only after initialization.
@@ -1204,18 +1203,20 @@ static int dmcrypt_write(void *data)
12041203
if (!RB_EMPTY_ROOT(&cc->write_tree))
12051204
goto pop_from_list;
12061205

1207-
if (unlikely(test_bit(DM_CRYPT_EXIT_THREAD, &cc->flags))) {
1208-
spin_unlock_irq(&cc->write_thread_wait.lock);
1209-
break;
1210-
}
1211-
1212-
__set_current_state(TASK_INTERRUPTIBLE);
1206+
set_current_state(TASK_INTERRUPTIBLE);
12131207
__add_wait_queue(&cc->write_thread_wait, &wait);
12141208

12151209
spin_unlock_irq(&cc->write_thread_wait.lock);
12161210

1211+
if (unlikely(kthread_should_stop())) {
1212+
set_task_state(current, TASK_RUNNING);
1213+
remove_wait_queue(&cc->write_thread_wait, &wait);
1214+
break;
1215+
}
1216+
12171217
schedule();
12181218

1219+
set_task_state(current, TASK_RUNNING);
12191220
spin_lock_irq(&cc->write_thread_wait.lock);
12201221
__remove_wait_queue(&cc->write_thread_wait, &wait);
12211222
goto continue_locked;
@@ -1530,13 +1531,8 @@ static void crypt_dtr(struct dm_target *ti)
15301531
if (!cc)
15311532
return;
15321533

1533-
if (cc->write_thread) {
1534-
spin_lock_irq(&cc->write_thread_wait.lock);
1535-
set_bit(DM_CRYPT_EXIT_THREAD, &cc->flags);
1536-
wake_up_locked(&cc->write_thread_wait);
1537-
spin_unlock_irq(&cc->write_thread_wait.lock);
1534+
if (cc->write_thread)
15381535
kthread_stop(cc->write_thread);
1539-
}
15401536

15411537
if (cc->io_queue)
15421538
destroy_workqueue(cc->io_queue);

0 commit comments

Comments
 (0)