linux-kernel - [156/244] [SCSI] iscsi_tcp: fix locking around iscsi sk user data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 28 Sep 2011 15:02:00 -0700
From:	Greg KH <gregkh@...e.de>
To:	linux-kernel@...r.kernel.org, stable@...nel.org
Cc:	stable-review@...nel.org, torvalds@...ux-foundation.org,
	akpm@...ux-foundation.org, alan@...rguk.ukuu.org.uk,
	Mike Christie <michaelc@...wisc.edu>,
	James Bottomley <JBottomley@...allels.com>,
	Hannes Reinecke <hare@...e.de>
Subject: [156/244] [SCSI] iscsi_tcp: fix locking around iscsi sk user data

3.0-stable review patch.  If anyone has any objections, please let us know.

------------------

From: Mike Christie <michaelc@...wisc.edu>

commit 03adb5f91280b433c3685d0ee86b2e1424af3d88 upstream.

iscsi_sw_tcp_conn_restore_callbacks could have set
the sk_user_data field to NULL then iscsi_sw_tcp_data_ready
could read that and try to access the NULL pointer. This
adds some checks for NULL sk_user_data in the sk
callback functions and it uses the sk_callback_lock to
set/get that sk_user_data field.

Signed-off-by: Mike Christie <michaelc@...wisc.edu>
Signed-off-by: James Bottomley <JBottomley@...allels.com>
Signed-off-by: Hannes Reinecke <hare@...e.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@...e.de>

---
 drivers/scsi/iscsi_tcp.c |   61 +++++++++++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 20 deletions(-)

--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -107,10 +107,12 @@ static int iscsi_sw_tcp_recv(read_descri
  * 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.
  */
 static inline int iscsi_sw_sk_state_check(struct sock *sk)
 {
-	struct iscsi_conn *conn = (struct iscsi_conn*)sk->sk_user_data;
+	struct iscsi_conn *conn = sk->sk_user_data;
 
 	if ((sk->sk_state == TCP_CLOSE_WAIT || sk->sk_state == TCP_CLOSE) &&
 	    !atomic_read(&sk->sk_rmem_alloc)) {
@@ -123,11 +125,17 @@ static inline int iscsi_sw_sk_state_chec
 
 static void iscsi_sw_tcp_data_ready(struct sock *sk, int flag)
 {
-	struct iscsi_conn *conn = sk->sk_user_data;
-	struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
+	struct iscsi_conn *conn;
+	struct iscsi_tcp_conn *tcp_conn;
 	read_descriptor_t rd_desc;
 
 	read_lock(&sk->sk_callback_lock);
+	conn = sk->sk_user_data;
+	if (!conn) {
+		read_unlock(&sk->sk_callback_lock);
+		return;
+	}
+	tcp_conn = conn->dd_data;
 
 	/*
 	 * Use rd_desc to pass 'conn' to iscsi_tcp_recv.
@@ -141,11 +149,10 @@ static void iscsi_sw_tcp_data_ready(stru
 
 	iscsi_sw_sk_state_check(sk);
 
-	read_unlock(&sk->sk_callback_lock);
-
 	/* If we had to (atomically) map a highmem page,
 	 * unmap it now. */
 	iscsi_tcp_segment_unmap(&tcp_conn->in.segment);
+	read_unlock(&sk->sk_callback_lock);
 }
 
 static void iscsi_sw_tcp_state_change(struct sock *sk)
@@ -157,8 +164,11 @@ static void iscsi_sw_tcp_state_change(st
 	void (*old_state_change)(struct sock *);
 
 	read_lock(&sk->sk_callback_lock);
-
-	conn = (struct iscsi_conn*)sk->sk_user_data;
+	conn = sk->sk_user_data;
+	if (!conn) {
+		read_unlock(&sk->sk_callback_lock);
+		return;
+	}
 	session = conn->session;
 
 	iscsi_sw_sk_state_check(sk);
@@ -178,11 +188,25 @@ static void iscsi_sw_tcp_state_change(st
  **/
 static void iscsi_sw_tcp_write_space(struct sock *sk)
 {
-	struct iscsi_conn *conn = (struct iscsi_conn*)sk->sk_user_data;
-	struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
-	struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
+	struct iscsi_conn *conn;
+	struct iscsi_tcp_conn *tcp_conn;
+	struct iscsi_sw_tcp_conn *tcp_sw_conn;
+	void (*old_write_space)(struct sock *);
+
+	read_lock_bh(&sk->sk_callback_lock);
+	conn = sk->sk_user_data;
+	if (!conn) {
+		read_unlock_bh(&sk->sk_callback_lock);
+		return;
+	}
+
+	tcp_conn = conn->dd_data;
+	tcp_sw_conn = tcp_conn->dd_data;
+	old_write_space = tcp_sw_conn->old_write_space;
+	read_unlock_bh(&sk->sk_callback_lock);
+
+	old_write_space(sk);
 
-	tcp_sw_conn->old_write_space(sk);
 	ISCSI_SW_TCP_DBG(conn, "iscsi_write_space\n");
 	iscsi_conn_queue_work(conn);
 }
@@ -592,20 +616,17 @@ static void iscsi_sw_tcp_conn_stop(struc
 	/* userspace may have goofed up and not bound us */
 	if (!sock)
 		return;
-	/*
-	 * Make sure our recv side is stopped.
-	 * Older tools called conn stop before ep_disconnect
-	 * so IO could still be coming in.
-	 */
-	write_lock_bh(&tcp_sw_conn->sock->sk->sk_callback_lock);
-	set_bit(ISCSI_SUSPEND_BIT, &conn->suspend_rx);
-	write_unlock_bh(&tcp_sw_conn->sock->sk->sk_callback_lock);
 
 	sock->sk->sk_err = EIO;
 	wake_up_interruptible(sk_sleep(sock->sk));
 
-	iscsi_conn_stop(cls_conn, flag);
+	/* stop xmit side */
+	iscsi_suspend_tx(conn);
+
+	/* stop recv side and release socket */
 	iscsi_sw_tcp_release_conn(conn);
+
+	iscsi_conn_stop(cls_conn, flag);
 }
 
 static int


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/