lkml.org 
[lkml]   [2021]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.14 710/849] scsi: target: core: Remove from tmr_list during LUN unlink
    Date
    From: Dmitry Bogdanov <d.bogdanov@yadro.com>

    [ Upstream commit 12b6fcd0ea7f3cb7c3b34668fc678779924123ae ]

    Currently TMF commands are removed from de_device.dev_tmf_list at the very
    end of se_cmd lifecycle. However, se_lun unlinks from se_cmd upon a command
    status (response) being queued in transport layer. This means that LUN and
    backend device can be deleted in the meantime and a panic will occur:

    target_tmr_work()
    cmd->se_tfo->queue_tm_rsp(cmd); // send abort_rsp to a wire
    transport_lun_remove_cmd(cmd) // unlink se_cmd from se_lun
    - // - // - // -
    <<<--- lun remove
    <<<--- core backend device remove
    - // - // - // -
    qlt_handle_abts_completion()
    tfo->free_mcmd()
    transport_generic_free_cmd()
    target_put_sess_cmd()
    core_tmr_release_req() {
    if (dev) { // backend device, can not be null
    spin_lock_irqsave(&dev->se_tmr_lock, flags); //<<<--- CRASH

    Call Trace:
    NIP [c000000000e1683c] _raw_spin_lock_irqsave+0x2c/0xc0
    LR [c00800000e433338] core_tmr_release_req+0x40/0xa0 [target_core_mod]
    Call Trace:
    (unreliable)
    0x0
    target_put_sess_cmd+0x2a0/0x370 [target_core_mod]
    transport_generic_free_cmd+0x6c/0x1b0 [target_core_mod]
    tcm_qla2xxx_complete_mcmd+0x28/0x50 [tcm_qla2xxx]
    process_one_work+0x2c4/0x5c0
    worker_thread+0x88/0x690

    For the iSCSI protocol this is easily reproduced:

    - Send some SCSI sommand

    - Send Abort of that command over iSCSI

    - Remove LUN on target

    - Send next iSCSI command to acknowledge the Abort_Response

    - Target panics

    There is no need to keep the command in tmr_list until response completion,
    so move the removal from tmr_list from the response completion to the
    response queueing when the LUN is unlinked. Move the removal from state
    list too as it is a subject to the same race condition.

    Link: https://lore.kernel.org/r/20211018135753.15297-1-d.bogdanov@yadro.com
    Fixes: c66ac9db8d4a ("[SCSI] target: Add LIO target core v4.0.0-rc6")
    Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
    Reviewed-by: Mike Christie <michael.christie@oracle.com>
    Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
    Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/target/target_core_tmr.c | 17 +--------------
    drivers/target/target_core_transport.c | 30 ++++++++++++++++++++------
    2 files changed, 24 insertions(+), 23 deletions(-)

    diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
    index e7fcbc09f9dbc..bac111456fa1d 100644
    --- a/drivers/target/target_core_tmr.c
    +++ b/drivers/target/target_core_tmr.c
    @@ -50,15 +50,6 @@ EXPORT_SYMBOL(core_tmr_alloc_req);

    void core_tmr_release_req(struct se_tmr_req *tmr)
    {
    - struct se_device *dev = tmr->tmr_dev;
    - unsigned long flags;
    -
    - if (dev) {
    - spin_lock_irqsave(&dev->se_tmr_lock, flags);
    - list_del_init(&tmr->tmr_list);
    - spin_unlock_irqrestore(&dev->se_tmr_lock, flags);
    - }
    -
    kfree(tmr);
    }

    @@ -156,13 +147,6 @@ void core_tmr_abort_task(
    se_cmd->state_active = false;
    spin_unlock_irqrestore(&dev->queues[i].lock, flags);

    - /*
    - * Ensure that this ABORT request is visible to the LU
    - * RESET code.
    - */
    - if (!tmr->tmr_dev)
    - WARN_ON_ONCE(transport_lookup_tmr_lun(tmr->task_cmd) < 0);
    -
    if (dev->transport->tmr_notify)
    dev->transport->tmr_notify(dev, TMR_ABORT_TASK,
    &aborted_list);
    @@ -234,6 +218,7 @@ static void core_tmr_drain_tmr_list(
    }

    list_move_tail(&tmr_p->tmr_list, &drain_tmr_list);
    + tmr_p->tmr_dev = NULL;
    }
    spin_unlock_irqrestore(&dev->se_tmr_lock, flags);

    diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
    index 26ceabe34de55..4b41fbc54fa5e 100644
    --- a/drivers/target/target_core_transport.c
    +++ b/drivers/target/target_core_transport.c
    @@ -676,6 +676,21 @@ static void target_remove_from_state_list(struct se_cmd *cmd)
    spin_unlock_irqrestore(&dev->queues[cmd->cpuid].lock, flags);
    }

    +static void target_remove_from_tmr_list(struct se_cmd *cmd)
    +{
    + struct se_device *dev = NULL;
    + unsigned long flags;
    +
    + if (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
    + dev = cmd->se_tmr_req->tmr_dev;
    +
    + if (dev) {
    + spin_lock_irqsave(&dev->se_tmr_lock, flags);
    + if (cmd->se_tmr_req->tmr_dev)
    + list_del_init(&cmd->se_tmr_req->tmr_list);
    + spin_unlock_irqrestore(&dev->se_tmr_lock, flags);
    + }
    +}
    /*
    * This function is called by the target core after the target core has
    * finished processing a SCSI command or SCSI TMF. Both the regular command
    @@ -687,13 +702,6 @@ static int transport_cmd_check_stop_to_fabric(struct se_cmd *cmd)
    {
    unsigned long flags;

    - target_remove_from_state_list(cmd);
    -
    - /*
    - * Clear struct se_cmd->se_lun before the handoff to FE.
    - */
    - cmd->se_lun = NULL;
    -
    spin_lock_irqsave(&cmd->t_state_lock, flags);
    /*
    * Determine if frontend context caller is requesting the stopping of
    @@ -728,8 +736,16 @@ static void transport_lun_remove_cmd(struct se_cmd *cmd)
    if (!lun)
    return;

    + target_remove_from_state_list(cmd);
    + target_remove_from_tmr_list(cmd);
    +
    if (cmpxchg(&cmd->lun_ref_active, true, false))
    percpu_ref_put(&lun->lun_ref);
    +
    + /*
    + * Clear struct se_cmd->se_lun before the handoff to FE.
    + */
    + cmd->se_lun = NULL;
    }

    static void target_complete_failure_work(struct work_struct *work)
    --
    2.33.0


    \
     
     \ /
      Last update: 2021-11-16 00:21    [W:3.572 / U:0.724 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site