Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e0172455 authored by Lee Duncan's avatar Lee Duncan Committed by Greg Kroah-Hartman
Browse files

scsi: libiscsi: Fix NOP race condition 

[ 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: default avatarMike Christie <michael.christie@oracle.com>
Signed-off-by: default avatarLee Duncan <lduncan@suse.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
parent fc52f37b

drivers/scsi/libiscsi.c

+15 −8
Original line number Diff line number Diff line
@@ -571,8 +571,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);
@@ -781,6 +781,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;
@@ -988,8 +991,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)

	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;
@@ -1004,11 +1010,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;

	}
@@ -1021,7 +1028,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.
@@ -1961,7 +1968,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;
@@ -2096,7 +2103,7 @@ 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;

include/scsi/libiscsi.h

+3 −0
Original line number Diff line number Diff line
@@ -145,6 +145,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;