lkml.org 
[lkml]   [2016]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.16.y-ckt 081/126] target: Fix race for SCF_COMPARE_AND_WRITE_POST checking
    Date
    3.16.7-ckt22 -stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Nicholas Bellinger <nab@linux-iscsi.org>

    commit 057085e522f8bf94c2e691a5b76880f68060f8ba upstream.

    This patch addresses a race + use after free where the first
    stage of COMPARE_AND_WRITE in compare_and_write_callback()
    is rescheduled after the backend sends the secondary WRITE,
    resulting in second stage compare_and_write_post() callback
    completing in target_complete_ok_work() before the first
    can return.

    Because current code depends on checking se_cmd->se_cmd_flags
    after return from se_cmd->transport_complete_callback(),
    this results in first stage having SCF_COMPARE_AND_WRITE_POST
    set, which incorrectly falls through into second stage CAW
    processing code, eventually triggering a NULL pointer
    dereference due to use after free.

    To address this bug, pass in a new *post_ret parameter into
    se_cmd->transport_complete_callback(), and depend upon this
    value instead of ->se_cmd_flags to determine when to return
    or fall through into ->queue_status() code for CAW.

    Cc: Sagi Grimberg <sagig@mellanox.com>
    Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
    [ luis: backported to 3.16: adjusted context ]
    Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
    ---
    drivers/target/target_core_sbc.c | 13 +++++++++----
    drivers/target/target_core_transport.c | 14 ++++++++------
    include/target/target_core_base.h | 2 +-
    3 files changed, 18 insertions(+), 11 deletions(-)

    diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
    index be2a1000e3dd..c59afca94080 100644
    --- a/drivers/target/target_core_sbc.c
    +++ b/drivers/target/target_core_sbc.c
    @@ -299,7 +299,8 @@ sbc_setup_write_same(struct se_cmd *cmd, unsigned char *flags, struct sbc_ops *o
    return 0;
    }

    -static sense_reason_t xdreadwrite_callback(struct se_cmd *cmd, bool success)
    +static sense_reason_t xdreadwrite_callback(struct se_cmd *cmd, bool success,
    + int *post_ret)
    {
    unsigned char *buf, *addr;
    struct scatterlist *sg;
    @@ -363,7 +364,8 @@ sbc_execute_rw(struct se_cmd *cmd)
    cmd->data_direction);
    }

    -static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success)
    +static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success,
    + int *post_ret)
    {
    struct se_device *dev = cmd->se_dev;

    @@ -373,8 +375,10 @@ static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success)
    * sent to the backend driver.
    */
    spin_lock_irq(&cmd->t_state_lock);
    - if ((cmd->transport_state & CMD_T_SENT) && !cmd->scsi_status)
    + if ((cmd->transport_state & CMD_T_SENT) && !cmd->scsi_status) {
    cmd->se_cmd_flags |= SCF_COMPARE_AND_WRITE_POST;
    + *post_ret = 1;
    + }
    spin_unlock_irq(&cmd->t_state_lock);

    /*
    @@ -386,7 +390,8 @@ static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success)
    return TCM_NO_SENSE;
    }

    -static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool success)
    +static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool success,
    + int *post_ret)
    {
    struct se_device *dev = cmd->se_dev;
    struct scatterlist *write_sg = NULL, *sg;
    diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
    index b32cbb8ab5b8..9eeeb78bf604 100644
    --- a/drivers/target/target_core_transport.c
    +++ b/drivers/target/target_core_transport.c
    @@ -1594,7 +1594,7 @@ bool target_stop_cmd(struct se_cmd *cmd, unsigned long *flags)
    void transport_generic_request_failure(struct se_cmd *cmd,
    sense_reason_t sense_reason)
    {
    - int ret = 0;
    + int ret = 0, post_ret = 0;

    pr_debug("-----[ Storage Engine Exception for cmd: %p ITT: 0x%08x"
    " CDB: 0x%02x\n", cmd, cmd->se_tfo->get_task_tag(cmd),
    @@ -1617,7 +1617,7 @@ void transport_generic_request_failure(struct se_cmd *cmd,
    */
    if ((cmd->se_cmd_flags & SCF_COMPARE_AND_WRITE) &&
    cmd->transport_complete_callback)
    - cmd->transport_complete_callback(cmd, false);
    + cmd->transport_complete_callback(cmd, false, &post_ret);

    switch (sense_reason) {
    case TCM_NON_EXISTENT_LUN:
    @@ -1977,11 +1977,13 @@ static void target_complete_ok_work(struct work_struct *work)
    */
    if (cmd->transport_complete_callback) {
    sense_reason_t rc;
    + bool caw = (cmd->se_cmd_flags & SCF_COMPARE_AND_WRITE);
    + bool zero_dl = !(cmd->data_length);
    + int post_ret = 0;

    - rc = cmd->transport_complete_callback(cmd, true);
    - if (!rc && !(cmd->se_cmd_flags & SCF_COMPARE_AND_WRITE_POST)) {
    - if ((cmd->se_cmd_flags & SCF_COMPARE_AND_WRITE) &&
    - !cmd->data_length)
    + rc = cmd->transport_complete_callback(cmd, true, &post_ret);
    + if (!rc && !post_ret) {
    + if (caw && zero_dl)
    goto queue_rsp;

    return;
    diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
    index edfd69a25884..0c37d70a4f57 100644
    --- a/include/target/target_core_base.h
    +++ b/include/target/target_core_base.h
    @@ -518,7 +518,7 @@ struct se_cmd {
    sense_reason_t (*execute_cmd)(struct se_cmd *);
    sense_reason_t (*execute_rw)(struct se_cmd *, struct scatterlist *,
    u32, enum dma_data_direction);
    - sense_reason_t (*transport_complete_callback)(struct se_cmd *, bool);
    + sense_reason_t (*transport_complete_callback)(struct se_cmd *, bool, int *);

    unsigned char *t_task_cdb;
    unsigned char __t_task_cdb[TCM_MAX_COMMAND_SIZE];

    \
     
     \ /
      Last update: 2016-01-06 12:41    [W:2.134 / U:0.240 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site