Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
Message-ID: <9df96d73-015c-4de6-96fa-2f315b066909@default>
Date:   Wed, 4 Nov 2020 21:40:54 -0800 (PST)
From:   Gulam Mohamed <gulam.mohamed@oracle.com>
Sender: Gulam Mohamed <gulam.mohamed@oracle.com>
To:     Lee Duncan <lduncan@suse.com>, Chris Leech <cleech@redhat.com>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        open-iscsi@googlegroups.com, linux-scsi@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     Junxiao Bi <junxiao.bi@oracle.com>
Subject: [PATCH] iscsi: Do Not set param when sock is NULL
References: <1a8aaa17-b1a3-4d6a-b87a-ff49d61a0d0b@default>
In-Reply-To: <1a8aaa17-b1a3-4d6a-b87a-ff49d61a0d0b@default>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Description
=3D=3D=3D=3D=3D=3D=3D=3D=3D
1. This Kernel panic could be due to a timing issue when there is a race be=
tween the sync thread and the initiator was processing of a login response =
from the target. The session re-open can be invoked from two places
          a.=09Sessions sync thread when the iscsid restart
          b.=09From iscsid through iscsi error handler
2. The session reopen sequence is as follows in user-space (iscsi-initiator=
-utils)
          a.=09Disconnect the connection
          b.=09Then send the stop connection request to the kernel which re=
leases the connection (releases the socket)
          c.=09Queues the reopen for 2 seconds delay
         d.=09Once the delay expires, create the TCP connection again by ca=
lling the connect() call
         e.=09Poll for the connection
          f.=09When poll is successful i.e when the TCP connection is estab=
lished, it performs
=09i. Creation of session and connection data structures
=09ii. Bind the connection to the session. This is the place where we assig=
n the sock to tcp_sw_conn->sock
=09iii. Sets the parameters like target name, persistent address etc .
=09iv. Creates the login pdu
=09v. Sends the login pdu to kernel
=09vi. Returns to the main loop to process further events. The kernel then =
sends the login request over to the target node
=09g. Once login response with success is received, it enters into full fea=
ture phase and sets the negotiable parameters like max_recv_data_length, ma=
x_transmit_length, data_digest etc .
3. While setting the negotiable parameters by calling "iscsi_session_set_ne=
g_params()", kernel panicked as sock was NULL

What happened here is
--------------------------------
1.=09Before initiator received the login response mentioned in above point =
2.f.v, another reopen request was sent from the error handler/sync session =
for the same session, as the initiator utils was in main loop to process fu=
rther events (as=20
=09mentioned in point 2.f.vi above).=20
2.=09While processing this reopen, it stopped the connection which released=
 the socket and queued this connection and at this point of time the login =
response was received for the earlier one
3.=09The kernel passed over this response to user-space which then sent the=
 set_neg_params request to kernel
4.=09As the connection was stopped, the sock was NULL and hence while the k=
ernel was processing the set param request from user-space, it panicked

Fix
----
1.=09While setting the set_param in kernel, we need to check if sock is NUL=
L
2.=09If the sock is NULL, then return EPERM (operation not permitted)
3.=09Due to this error handler will be invoked in user-space again to recov=
er the session

Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
Reviewed-by: Junxiao Bi <junxiao.bi@oracle.com>
---
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index df47557a02a3..fd668a194053 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -711,6 +711,12 @@ static int iscsi_sw_tcp_conn_set_param(struct iscsi_cl=
s_conn *cls_conn,
        struct iscsi_tcp_conn *tcp_conn =3D conn->dd_data;
        struct iscsi_sw_tcp_conn *tcp_sw_conn =3D tcp_conn->dd_data;

+       if (!tcp_sw_conn->sock) {
+               iscsi_conn_printk(KERN_ERR, conn,
+                               "Cannot set param as sock is NULL\n");
+               return -ENOTCONN;
+       }
+
        switch(param) {
        case ISCSI_PARAM_HDRDGST_EN:
                iscsi_set_param(cls_conn, param, buf, buflen);
--=20
2.18.4