linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: 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
Date: Wed, 4 Nov 2020 21:40:54 -0800 (PST)	[thread overview]
Message-ID: <9df96d73-015c-4de6-96fa-2f315b066909@default> (raw)
In-Reply-To: <1a8aaa17-b1a3-4d6a-b87a-ff49d61a0d0b@default>

Description
=========
1. This Kernel panic could be due to a timing issue when there is a race between 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.	Sessions sync thread when the iscsid restart
          b.	From iscsid through iscsi error handler
2. The session reopen sequence is as follows in user-space (iscsi-initiator-utils)
          a.	Disconnect the connection
          b.	Then send the stop connection request to the kernel which releases the connection (releases the socket)
          c.	Queues the reopen for 2 seconds delay
         d.	Once the delay expires, create the TCP connection again by calling the connect() call
         e.	Poll for the connection
          f.	When poll is successful i.e when the TCP connection is established, it performs
	i. Creation of session and connection data structures
	ii. Bind the connection to the session. This is the place where we assign the sock to tcp_sw_conn->sock
	iii. Sets the parameters like target name, persistent address etc .
	iv. Creates the login pdu
	v. Sends the login pdu to kernel
	vi. Returns to the main loop to process further events. The kernel then sends the login request over to the target node
	g. Once login response with success is received, it enters into full feature phase and sets the negotiable parameters like max_recv_data_length, max_transmit_length, data_digest etc .
3. While setting the negotiable parameters by calling "iscsi_session_set_neg_params()", kernel panicked as sock was NULL

What happened here is
--------------------------------
1.	Before 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 further events (as 
	mentioned in point 2.f.vi above). 
2.	While 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.	The kernel passed over this response to user-space which then sent the set_neg_params request to kernel
4.	As the connection was stopped, the sock was NULL and hence while the kernel was processing the set param request from user-space, it panicked

Fix
----
1.	While setting the set_param in kernel, we need to check if sock is NULL
2.	If the sock is NULL, then return EPERM (operation not permitted)
3.	Due to this error handler will be invoked in user-space again to recover 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_cls_conn *cls_conn,
        struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
        struct iscsi_sw_tcp_conn *tcp_sw_conn = 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);
-- 
2.18.4

       reply	other threads:[~2020-11-05  5:41 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1a8aaa17-b1a3-4d6a-b87a-ff49d61a0d0b@default>
2020-11-05  5:40 ` Gulam Mohamed [this message]
2020-11-17  5:08   ` [PATCH] iscsi: Do Not set param when sock is NULL Gulam Mohamed
2020-11-23 23:07   ` Michael Christie
2021-01-07 15:48     ` Gulam Mohamed
2021-01-18 18:50       ` Mike Christie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9df96d73-015c-4de6-96fa-2f315b066909@default \
    --to=gulam.mohamed@oracle.com \
    --cc=cleech@redhat.com \
    --cc=jejb@linux.ibm.com \
    --cc=junxiao.bi@oracle.com \
    --cc=lduncan@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=open-iscsi@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).