Hi,
在 2023/03/15 17:30, Guoqing Jiang 写道:
Just borrow this thread to discuss, I think this commit might have
problem in some corner cases:
t1: t2:
action_store
mddev_lock
if (mddev->sync_thread)
mddev_unlock
md_unregister_thread
md_check_recovery
set_bit(MD_RECOVERY_RUNNING, &mddev->recovery)
queue_work(md_misc_wq, &mddev->del_work)
mddev_lock_nointr
md_reap_sync_thread
// clear running
mddev_lock
t3:
md_start_sync
// running is not set
What does 'running' mean? MD_RECOVERY_RUNNING?
Our test report a problem that can be cause by this in theory, by we
can't be sure for now...
I guess you tried to describe racy between
action_store -> md_register_thread
and
md_start_sync -> md_register_thread
Didn't you already fix them in the series?
[PATCH -next 0/5] md: fix uaf for sync_thread
Sorry, I didn't follow the problem and also your series, I might try your
test with latest mainline kernel if the test is available somewhere.
We thought about how to fix this, instead of calling
md_register_thread() here to wait for sync_thread to be done
synchronisely,
IMO, md_register_thread just create and wake a thread, not sure why it
waits for sync_thread.
we do this asynchronously like what md_set_readonly() and do_md_stop()
does.
Still, I don't have clear picture about the problem, so I can't judge it.
Sorry that I didn't explain the problem clear. Let me explain the
problem we meet first:
1) raid10d is waiting for sync_thread to stop:
raid10d
md_unregister_thread
kthread_stop
2) sync_thread is waiting for io to finish:
md_do_sync
wait_event(... atomic_read(&mddev->recovery_active) == 0)
3) io is waiting for raid10d to finish(online crash found 2 io in
conf->retry_list)
Additional information from online crash:
mddev->recovery = 29, // DONE, RUNING, INTR is set
PID: 138293 TASK: ffff0000de89a900 CPU: 7 COMMAND: "md0_resync"
#0 [ffffa00107c178a0] __switch_to at ffffa0010001d75c
#1 [ffffa00107c178d0] __schedule at ffffa001017c7f14
#2 [ffffa00107c179f0] schedule at ffffa001017c880c
#3 [ffffa00107c17a20] md_do_sync at ffffa0010129cdb4
#4 [ffffa00107c17d50] md_thread at ffffa00101290d9c
#5 [ffffa00107c17e50] kthread at ffffa00100187a74
PID: 138294 TASK: ffff0000eba13d80 CPU: 5 COMMAND: "md0_resync"
#0 [ffffa00107e47a60] __switch_to at ffffa0010001d75c
#1 [ffffa00107e47a90] __schedule at ffffa001017c7f14
#2 [ffffa00107e47bb0] schedule at ffffa001017c880c
#3 [ffffa00107e47be0] schedule_timeout at ffffa001017d1298
#4 [ffffa00107e47d50] md_thread at ffffa00101290ee8
#5 [ffffa00107e47e50] kthread at ffffa00100187a74
// there are two sync_thread for md0
I believe the root cause is that two sync_thread exist for the same
mddev, and this is how I think this is possible:
t1: t2:
action_store
mddev_lock
if (mddev->sync_thread)
mddev_unlock
md_unregister_thread
// first sync_thread is done
md_check_recovery
set_bit(MD_RECOVERY_RUNNING, &mddev->recovery)
queue_work(md_misc_wq, &mddev->del_work)
mddev_lock_nointr
md_reap_sync_thread
// MD_RECOVERY_RUNNING is cleared
mddev_unlock
t3:
md_start_sync
// second sync_thread is registed
t3:
md_check_recovery
queue_work(md_misc_wq, &mddev->del_work)
// MD_RECOVERY_RUNNING is not set, a new sync_thread can be started
This is just guess, I can't reporduce the problem yet. Please let me
know if you have any questions
Thanks,
Kuai
