From c2f9ce8df820aa58d4318aa90405132d9fc69329 Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Mon, 14 Feb 2022 23:39:23 +0300 Subject: [PATCH] scst_targ: Fix tgt_dev_cmd_count ref counter management Decrementing the tgt_dev_cmd_count ref happens in scst_pre_xmit_response1(). This may lead to a race condition between scst_acg_repl_lun() and scst_tgt_cmd_done()'s cmd processing, which may lead to a general protection fault. cmd processing replace LUN -------------- ----------- [1] atomic_dec(tgt_dev->tgt_dev_cmd_count) [2] scst_wait_for_tgt_devs() [3] scst_free_tgt_dev() [4] scst_tgt_dev_stop_threads() [5] scst_tgt_cmd_done(cmd, SCST_CONTEXT_THREAD) [6] scst_process_redirect_cmd() [7] if (cmd->cmd_thr != NULL) { [8] active_cmd_list = &cmd->cmd_thr->thr_active_cmd_list; cmd->cmd_thr is already dead pointer here, because it is pointing on thread which was freed in scst_tgt_dev_stop_threads(). [9] list_add(&cmd->cmd_list_entry, active_cmd_list); general protection fault: 0000 RIP: 0010:__list_add_valid+0x1/0x74 Call Trace: scst_tgt_cmd_done+0x12b/0x160 [scst] ... Hence, to avoid this race condition, move the decrement of tgt_dev_cmd_count from the SCST_CMD_STATE_PRE_XMIT_RESP1 to the SCST_CMD_STATE_FINISHED state, which is assigned after scst_tgt_cmd_done() was called. --- scst/src/scst_targ.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index f737e0609..f64acb7a9 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -3664,11 +3664,11 @@ static int scst_pre_xmit_response1(struct scst_cmd *cmd) /* * Those counters protect from not getting too long processing * latency, so we should decrement them after cmd completed. + * + * @cmd processing for SCST device is complete. */ - smp_mb__before_atomic_dec(); WARN_ON_ONCE(!cmd->owns_refcnt); cmd->owns_refcnt = false; - atomic_dec(&cmd->tgt_dev->tgt_dev_cmd_count); percpu_ref_put(&cmd->dev->refcnt); #ifdef CONFIG_SCST_PER_DEVICE_CMD_COUNT_LIMIT atomic_dec(&cmd->dev->dev_cmd_count); @@ -3875,6 +3875,22 @@ static int scst_finish_cmd(struct scst_cmd *cmd) } } + if (likely(cmd->tgt_dev != NULL)) { + /* + * We must decrement @tgt_dev->tgt_dev_cmd_count + * after scst_tgt_cmd_done() was called. Otherwise, + * this may lead to a race condition between + * scst_acg_repl_lun() and scst_tgt_cmd_done()'s + * cmd processing. + * + * See also https://github.com/SCST-project/scst/pull/27 + * + * @cmd processing for target device is complete. + */ + smp_mb__before_atomic_dec(); + atomic_dec(&cmd->tgt_dev->tgt_dev_cmd_count); + } + atomic_dec(&sess->sess_cmd_count); spin_lock_irq(&sess->sess_list_lock);