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.
This commit is contained in:
Gleb Chesnokov
2022-02-14 23:39:23 +03:00
parent 7e484c59b3
commit c2f9ce8df8

View File

@@ -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);