mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Praneeth Balasubramanian <praneethb2002@gmail.com>
To: Lee Duncan <lduncan@suse.com>, Chris Leech <cleech@redhat.com>,
	Mike Christie <michael.christie@oracle.com>
Cc: "Martin K . Petersen" <mkp@kernel.org>,
	"James E . J . Bottomley" <James.Bottomley@HansenPartnership.com>,
	open-iscsi@googlegroups.com, linux-scsi@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Praneeth Balasubramanian <praneeth@ucsb.edu>,
	stable@vger.kernel.org
Subject: [PATCH] scsi: iscsi_tcp: Don't read sk_user_data from the recv work
Date: Tue, 10 Nov 2026 14:24:03 -0800	[thread overview]
Message-ID: <20261110222403.205332-1-praneeth@ucsb.edu> (raw)

With recv_from_iscsi_q=1, iscsi_sw_tcp_data_ready() queues
conn->recvwork, and iscsi_sw_tcp_recv_data_work() runs the receive path
under lock_sock() only. At the end of iscsi_sw_tcp_recv_data(),
iscsi_sw_sk_state_check() reads the connection from sk->sk_user_data.
Its comment says it must be called with sk_callback_lock, which the work
does not hold.

On teardown, iscsi_sw_tcp_release_conn() first calls
iscsi_sw_tcp_conn_restore_callbacks(), which sets sk->sk_user_data to
NULL, and only then flushes recvwork in iscsi_suspend_rx(). If queued
work runs in between and the socket is in TCP_CLOSE or TCP_CLOSE_WAIT,
iscsi_sw_sk_state_check() dereferences NULL->session. With a delay
added at the start of the work to widen this window, a target-side
connection close gives:

  Oops: general protection fault, probably for non-canonical address ...
  KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
  Workqueue: iscsi_q_3 iscsi_sw_tcp_recv_data_work
  RIP: 0010:iscsi_sw_tcp_recv_data+0x219/0x420

Both callers already have the connection. iscsi_sw_tcp_recv_data() gets
it as an argument, and iscsi_sw_tcp_state_change() reads it under
sk_callback_lock. Pass it in instead of reading sk_user_data again. The
connection stays valid until iscsi_suspend_rx() has flushed the work.

The unlocked read was found with an LLM-based tool that checks callers
against the locking rules stated in kernel comments.

Fixes: f1d269765ee2 ("scsi: iscsi: Run recv path from workqueue")
Cc: stable@vger.kernel.org
Signed-off-by: Praneeth Balasubramanian <praneeth@ucsb.edu>
---

Notes:
    This was found by an LLM-based static analysis (GPT-5.6) that checks
    kernel code against the rules stated in its comments, as part of research
    at UCSB SecLab. The patch and changelog were drafted with Claude Opus 5.5.
    I reviewed the patch and take responsibility for it.
    
    recv_from_iscsi_q is off by default, and setting up the session needs
    root on the initiator, so I am sending this as a regular fix rather than
    a security report.
    
    Testing: two QEMU VMs on next-20261023, an LIO target and an open-iscsi
    initiator booted with iscsi_tcp.recv_from_iscsi_q=1, using CHAP and
    continuous reads. The initiator kernel had a 20 ms msleep() added at the
    start of iscsi_sw_tcp_recv_data_work(). Closing the TCP connection from
    the target side produced the oops above. I have only seen it with the
    added delay. 
    
    The patch applies to mainline 9ba46df34488 and to next-20261109.

 drivers/scsi/iscsi_tcp.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 9260b1c9b0e0..88b1f255ae9b 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -104,18 +104,20 @@ static int iscsi_sw_tcp_recv(read_descriptor_t *rd_desc, struct sk_buff *skb,
 
 /**
  * iscsi_sw_sk_state_check - check socket state
+ * @conn: iscsi connection the socket belongs to
  * @sk: socket
  *
  * If the socket is in CLOSE or CLOSE_WAIT we should
  * not close the connection if there is still some
  * data pending.
  *
- * Must be called with sk_callback_lock.
+ * The caller passes @conn rather than having it read from
+ * sk->sk_user_data. The recv work runs without sk_callback_lock,
+ * and connection teardown may already have cleared sk_user_data.
  */
-static inline int iscsi_sw_sk_state_check(struct sock *sk)
+static inline int iscsi_sw_sk_state_check(struct iscsi_conn *conn,
+					  struct sock *sk)
 {
-	struct iscsi_conn *conn = sk->sk_user_data;
-
 	if ((sk->sk_state == TCP_CLOSE_WAIT || sk->sk_state == TCP_CLOSE) &&
 	    (conn->session->state != ISCSI_STATE_LOGGING_OUT) &&
 	    !atomic_read(&sk->sk_rmem_alloc)) {
@@ -148,7 +150,7 @@ static void iscsi_sw_tcp_recv_data(struct iscsi_conn *conn)
 	 * unmap it now. */
 	iscsi_tcp_segment_unmap(&tcp_conn->in.segment);
 
-	iscsi_sw_sk_state_check(sk);
+	iscsi_sw_sk_state_check(conn, sk);
 }
 
 static void iscsi_sw_tcp_recv_data_work(struct work_struct *work)
@@ -202,7 +204,7 @@ static void iscsi_sw_tcp_state_change(struct sock *sk)
 		return;
 	}
 
-	iscsi_sw_sk_state_check(sk);
+	iscsi_sw_sk_state_check(conn, sk);
 
 	tcp_conn = conn->dd_data;
 	tcp_sw_conn = tcp_conn->dd_data;
-- 
2.25.1


                 reply	other threads:[~2026-11-10 22:24 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20261110222403.205332-1-praneeth@ucsb.edu \
    --to=praneethb2002@gmail.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=cleech@redhat.com \
    --cc=lduncan@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=michael.christie@oracle.com \
    --cc=mkp@kernel.org \
    --cc=open-iscsi@googlegroups.com \
    --cc=praneeth@ucsb.edu \
    --cc=stable@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®