scsi: target: iscsi: Fix login error when receiving

iscsi_target_sk_data_ready() could be invoked indirectly by
iscsi_target_do_login_rx() from the workqueue like this:

iscsi_target_do_login_rx()
  iscsi_target_do_login()
    iscsi_target_do_tx_login_io()
      iscsit_put_login_tx()
        iscsi_login_tx_data()
          tx_data()
            sock_sendmsg_nosec()
              tcp_sendmsg()
                release_sock()
                  sk_backlog_rcv()
                    tcp_v4_do_rcv()
                      tcp_data_ready()
                        iscsi_target_sk_data_ready()

At that time LOGIN_FLAGS_READ_ACTIVE is not cleared and
iscsi_target_sk_data_ready will not read data from the socket. Some iscsi
initiators (libiscsi) will wait forever for a reply.

LOGIN_FLAGS_READ_ACTIVE should be cleared early just after doing the
receive and before writing to the socket in iscsi_target_do_login_rx.

Unfortunately, LOGIN_FLAGS_READ_ACTIVE is also used by sk_state_change to
do login cleanup if a socket was closed at login time. It is supposed to be
cleared after the login PDU is successfully processed and replied.

Introduce another flag, LOGIN_FLAGS_WRITE_ACTIVE, to cover the transmit
part.

Link: https://lore.kernel.org/r/20200716100212.4237-2-houpu@bytedance.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Hou Pu <houpu@bytedance.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c
index 685d771..43154fd 100644 (file)
--- a/drivers/target/iscsi/iscsi_target_nego.c
+++ b/drivers/target/iscsi/iscsi_target_nego.c
@@ -625,13 +625,37 @@ static void iscsi_target_do_login_rx(struct work_struct *work)
        pr_debug("iscsi_target_do_login_rx after rx_login_io, %p, %s:%d\n",
                        conn, current->comm, current->pid);
 
+       /*
+        * LOGIN_FLAGS_READ_ACTIVE is cleared so that sk_data_ready
+        * could be triggered again after this.
+        *
+        * LOGIN_FLAGS_WRITE_ACTIVE is cleared after we successfully
+        * process a login PDU, so that sk_state_chage can do login
+        * cleanup as needed if the socket is closed. If a delayed work is
+        * ongoing (LOGIN_FLAGS_WRITE_ACTIVE or LOGIN_FLAGS_READ_ACTIVE),
+        * sk_state_change will leave the cleanup to the delayed work or
+        * it will schedule a delayed work to do cleanup.
+        */
+       if (conn->sock) {
+               struct sock *sk = conn->sock->sk;
+
+               write_lock_bh(&sk->sk_callback_lock);
+               if (!test_bit(LOGIN_FLAGS_INITIAL_PDU, &conn->login_flags)) {
+                       clear_bit(LOGIN_FLAGS_READ_ACTIVE, &conn->login_flags);
+                       set_bit(LOGIN_FLAGS_WRITE_ACTIVE, &conn->login_flags);
+               }
+               write_unlock_bh(&sk->sk_callback_lock);
+       }
+
        rc = iscsi_target_do_login(conn, login);
        if (rc < 0) {
                goto err;
        } else if (!rc) {
-               if (iscsi_target_sk_check_and_clear(conn, LOGIN_FLAGS_READ_ACTIVE))
+               if (iscsi_target_sk_check_and_clear(conn,
+                                                   LOGIN_FLAGS_WRITE_ACTIVE))
                        goto err;
        } else if (rc == 1) {
+               cancel_delayed_work(&conn->login_work);
                iscsi_target_nego_release(conn);
                iscsi_post_login_handler(np, conn, zero_tsih);
                iscsit_deaccess_np(np, tpg, tpg_np);
@@ -640,6 +664,7 @@ static void iscsi_target_do_login_rx(struct work_struct *work)
 
 err:
        iscsi_target_restore_sock_callbacks(conn);
+       cancel_delayed_work(&conn->login_work);
        iscsi_target_login_drop(conn, login);
        iscsit_deaccess_np(np, tpg, tpg_np);
 }
@@ -670,9 +695,10 @@ static void iscsi_target_sk_state_change(struct sock *sk)
        state = __iscsi_target_sk_check_close(sk);
        pr_debug("__iscsi_target_sk_close_change: state: %d\n", state);
 
-       if (test_bit(LOGIN_FLAGS_READ_ACTIVE, &conn->login_flags)) {
-               pr_debug("Got LOGIN_FLAGS_READ_ACTIVE=1 sk_state_change"
-                        " conn: %p\n", conn);
+       if (test_bit(LOGIN_FLAGS_READ_ACTIVE, &conn->login_flags) ||
+           test_bit(LOGIN_FLAGS_WRITE_ACTIVE, &conn->login_flags)) {
+               pr_debug("Got LOGIN_FLAGS_{READ|WRITE}_ACTIVE=1"
+                        " sk_state_change conn: %p\n", conn);
                if (state)
                        set_bit(LOGIN_FLAGS_CLOSED, &conn->login_flags);
                write_unlock_bh(&sk->sk_callback_lock);

diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h
index 4fda324..1eccb2a 100644 (file)
--- a/include/target/iscsi/iscsi_target_core.h
+++ b/include/target/iscsi/iscsi_target_core.h
@@ -556,10 +556,11 @@ struct iscsi_conn {
        struct socket           *sock;
        void                    (*orig_data_ready)(struct sock *);
        void                    (*orig_state_change)(struct sock *);
-#define LOGIN_FLAGS_READ_ACTIVE                1
-#define LOGIN_FLAGS_CLOSED             2
-#define LOGIN_FLAGS_READY              4
-#define LOGIN_FLAGS_INITIAL_PDU                8
+#define LOGIN_FLAGS_READY              0
+#define LOGIN_FLAGS_INITIAL_PDU                1
+#define LOGIN_FLAGS_READ_ACTIVE                2
+#define LOGIN_FLAGS_WRITE_ACTIVE       3
+#define LOGIN_FLAGS_CLOSED             4
        unsigned long           login_flags;
        struct delayed_work     login_work;
        struct iscsi_login      *login;