* [PATCH] scsi: iscsi_tcp: Don't read sk_user_data from the recv work
@ 2026-11-10 22:24 Praneeth Balasubramanian
0 siblings, 0 replies; only message in thread
From: Praneeth Balasubramanian @ 2026-11-10 22:24 UTC (permalink / raw)
To: Lee Duncan, Chris Leech, Mike Christie
Cc: Martin K . Petersen, James E . J . Bottomley, open-iscsi,
linux-scsi, linux-kernel, Praneeth Balasubramanian, stable
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
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2026-11-10 22:24 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-10 22:24 [PATCH] scsi: iscsi_tcp: Don't read sk_user_data from the recv work Praneeth Balasubramanian
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®