lkml.org 
[lkml]   [2020]   [Dec]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 18/42] scsi: libiscsi: Fix NOP race condition
    Date
    From: Lee Duncan <lduncan@suse.com>

    [ Upstream commit fe0a8a95e7134d0b44cd407bc0085b9ba8d8fe31 ]

    iSCSI NOPs are sometimes "lost", mistakenly sent to the user-land iscsid
    daemon instead of handled in the kernel, as they should be, resulting in a
    message from the daemon like:

    iscsid: Got nop in, but kernel supports nop handling.

    This can occur because of the new forward- and back-locks, and the fact
    that an iSCSI NOP response can occur before processing of the NOP send is
    complete. This can result in "conn->ping_task" being NULL in
    iscsi_nop_out_rsp(), when the pointer is actually in the process of being
    set.

    To work around this, we add a new state to the "ping_task" pointer. In
    addition to NULL (not assigned) and a pointer (assigned), we add the state
    "being set", which is signaled with an INVALID pointer (using "-1").

    Link: https://lore.kernel.org/r/20201106193317.16993-1-leeman.duncan@gmail.com
    Reviewed-by: Mike Christie <michael.christie@oracle.com>
    Signed-off-by: Lee Duncan <lduncan@suse.com>
    Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/scsi/libiscsi.c | 23 +++++++++++++++--------
    include/scsi/libiscsi.h | 3 +++
    2 files changed, 18 insertions(+), 8 deletions(-)

    diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
    index c4336b01db23c..a84b473d4a08b 100644
    --- a/drivers/scsi/libiscsi.c
    +++ b/drivers/scsi/libiscsi.c
    @@ -570,8 +570,8 @@ static void iscsi_complete_task(struct iscsi_task *task, int state)
    if (conn->task == task)
    conn->task = NULL;

    - if (conn->ping_task == task)
    - conn->ping_task = NULL;
    + if (READ_ONCE(conn->ping_task) == task)
    + WRITE_ONCE(conn->ping_task, NULL);

    /* release get from queueing */
    __iscsi_put_task(task);
    @@ -780,6 +780,9 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr,
    task->conn->session->age);
    }

    + if (unlikely(READ_ONCE(conn->ping_task) == INVALID_SCSI_TASK))
    + WRITE_ONCE(conn->ping_task, task);
    +
    if (!ihost->workq) {
    if (iscsi_prep_mgmt_task(conn, task))
    goto free_task;
    @@ -987,8 +990,11 @@ static int iscsi_send_nopout(struct iscsi_conn *conn, struct iscsi_nopin *rhdr)
    struct iscsi_nopout hdr;
    struct iscsi_task *task;

    - if (!rhdr && conn->ping_task)
    - return -EINVAL;
    + if (!rhdr) {
    + if (READ_ONCE(conn->ping_task))
    + return -EINVAL;
    + WRITE_ONCE(conn->ping_task, INVALID_SCSI_TASK);
    + }

    memset(&hdr, 0, sizeof(struct iscsi_nopout));
    hdr.opcode = ISCSI_OP_NOOP_OUT | ISCSI_OP_IMMEDIATE;
    @@ -1003,11 +1009,12 @@ static int iscsi_send_nopout(struct iscsi_conn *conn, struct iscsi_nopin *rhdr)

    task = __iscsi_conn_send_pdu(conn, (struct iscsi_hdr *)&hdr, NULL, 0);
    if (!task) {
    + if (!rhdr)
    + WRITE_ONCE(conn->ping_task, NULL);
    iscsi_conn_printk(KERN_ERR, conn, "Could not send nopout\n");
    return -EIO;
    } else if (!rhdr) {
    /* only track our nops */
    - conn->ping_task = task;
    conn->last_ping = jiffies;
    }

    @@ -1020,7 +1027,7 @@ static int iscsi_nop_out_rsp(struct iscsi_task *task,
    struct iscsi_conn *conn = task->conn;
    int rc = 0;

    - if (conn->ping_task != task) {
    + if (READ_ONCE(conn->ping_task) != task) {
    /*
    * If this is not in response to one of our
    * nops then it must be from userspace.
    @@ -1960,7 +1967,7 @@ static void iscsi_start_tx(struct iscsi_conn *conn)
    */
    static int iscsi_has_ping_timed_out(struct iscsi_conn *conn)
    {
    - if (conn->ping_task &&
    + if (READ_ONCE(conn->ping_task) &&
    time_before_eq(conn->last_recv + (conn->recv_timeout * HZ) +
    (conn->ping_timeout * HZ), jiffies))
    return 1;
    @@ -2095,7 +2102,7 @@ static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
    * Checking the transport already or nop from a cmd timeout still
    * running
    */
    - if (conn->ping_task) {
    + if (READ_ONCE(conn->ping_task)) {
    task->have_checked_conn = true;
    rc = BLK_EH_RESET_TIMER;
    goto done;
    diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
    index c7b1dc713cdd7..9c7f4aad6db66 100644
    --- a/include/scsi/libiscsi.h
    +++ b/include/scsi/libiscsi.h
    @@ -144,6 +144,9 @@ struct iscsi_task {
    void *dd_data; /* driver/transport data */
    };

    +/* invalid scsi_task pointer */
    +#define INVALID_SCSI_TASK (struct iscsi_task *)-1l
    +
    static inline int iscsi_task_has_unsol_data(struct iscsi_task *task)
    {
    return task->unsol_r2t.data_length > task->unsol_r2t.sent;
    --
    2.27.0


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