All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maurizio Lombardi <mlombard@redhat.com>
To: martin.petersen@oracle.com
Cc: michael.christie@oracle.com, target-devel@vger.kernel.org
Subject: [PATCH V3] target: fix a race condition between login_work and the login thread
Date: Tue, 15 Nov 2022 13:56:38 +0100	[thread overview]
Message-ID: <20221115125638.102517-1-mlombard@redhat.com> (raw)

In case a malicious initiator sends some random data immediately after a
login PDU; the iscsi_target_sk_data_ready() callback will
schedule the login_work and, at the same time,
the negotiation may end without clearing the LOGIN_FLAGS_INITIAL_PDU flag
(because no additional PDU exchanges are required to complete the login).

The login has been completed but the login_work function
will find the LOGIN_FLAGS_INITIAL_PDU flag set and will
never stop from rescheduling itself;
at this point, if the initiator drops the connection, the iscsit_conn
structure will be freed, login_work will dereference a released
socket structure and the kernel crashes.

BUG: kernel NULL pointer dereference, address: 0000000000000230
PF: supervisor write access in kernel mode
PF: error_code(0x0002) - not-present page
Workqueue: events iscsi_target_do_login_rx [iscsi_target_mod]
RIP: 0010:_raw_read_lock_bh+0x15/0x30
Call trace:
 iscsi_target_do_login_rx+0x75/0x3f0 [iscsi_target_mod]
 process_one_work+0x1e8/0x3c0

Fix this bug by forcing login_work to stop after the login has been
completed and the socket callbacks have been restored.

Add a comment to clearify the return values of iscsi_target_do_login()

v3: cancel_delayed_work_sync() should be called by
    iscsi_target_start_negotiation(), because the latter is only executed
    in login_thread context

V2: remove an unnecessary call to cancel_delayed_work();
    fix a potential race condition in iscsi_start_negotiation() and
    in iscsi_target_do_login_rx()'s error paths

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/target/iscsi/iscsi_target_nego.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c
index f2919319ad38..ff49c8f3fe24 100644
--- a/drivers/target/iscsi/iscsi_target_nego.c
+++ b/drivers/target/iscsi/iscsi_target_nego.c
@@ -1018,6 +1018,13 @@ static int iscsi_target_handle_csg_one(struct iscsit_conn *conn, struct iscsi_lo
 	return 0;
 }
 
+/*
+ * RETURN VALUE:
+ *
+ *  1 = Login successful
+ * -1 = Login failed
+ *  0 = More PDU exchanges required
+ */
 static int iscsi_target_do_login(struct iscsit_conn *conn, struct iscsi_login *login)
 {
 	int pdu_count = 0;
@@ -1363,12 +1370,13 @@ int iscsi_target_start_negotiation(
 		ret = -1;
 
 	if (ret < 0) {
-		cancel_delayed_work_sync(&conn->login_work);
 		iscsi_target_restore_sock_callbacks(conn);
 		iscsi_remove_failed_auth_entry(conn);
 	}
-	if (ret != 0)
+	if (ret != 0) {
+		cancel_delayed_work_sync(&conn->login_work);
 		iscsi_target_nego_release(conn);
+	}
 
 	return ret;
 }
-- 
2.31.1


             reply	other threads:[~2022-11-15 12:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-15 12:56 Maurizio Lombardi [this message]
2022-11-16 21:18 ` [PATCH V3] target: fix a race condition between login_work and the login thread Mike Christie
2022-11-24  3:28 ` Martin K. Petersen
2022-12-01  3:45 ` Martin K. Petersen
  -- strict thread matches above, loose matches on Subject: below --
2022-11-15 12:52 Maurizio Lombardi
2022-11-15 13:58 ` Maurizio Lombardi

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=20221115125638.102517-1-mlombard@redhat.com \
    --to=mlombard@redhat.com \
    --cc=martin.petersen@oracle.com \
    --cc=michael.christie@oracle.com \
    --cc=target-devel@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.