Commit 52afcca1 authored by Mike Christie's avatar Mike Christie Committed by Zhong Jinghua
Browse files

scsi: iscsi: iscsi_tcp: Fix null-ptr-deref while calling getpeername() 

mainline inclusion
from mainline-v6.1-rc1
commit 57569c37
category: bugfix
bugzilla: https://gitee.com/openeuler/kernel/issues/I6I8YD
CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=57569c37f0add1b6489e1a1563c71519daf732cf

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

Fix a NULL pointer crash that occurs when we are freeing the socket at the
same time we access it via sysfs.

The problem is that:

 1. iscsi_sw_tcp_conn_get_param() and iscsi_sw_tcp_host_get_param() take
    the frwd_lock and do sock_hold() then drop the frwd_lock. sock_hold()
    does a get on the "struct sock".

 2. iscsi_sw_tcp_release_conn() does sockfd_put() which does the last put
    on the "struct socket" and that does __sock_release() which sets the
    sock->ops to NULL.

 3. iscsi_sw_tcp_conn_get_param() and iscsi_sw_tcp_host_get_param() then
    call kernel_getpeername() which accesses the NULL sock->ops.

Above we do a get on the "struct sock", but we needed a get on the "struct
socket". Originally, we just held the frwd_lock the entire time but in
commit bcf3a295 ("scsi: iscsi: iscsi_tcp: Avoid holding spinlock while
calling getpeername()") we switched to refcount based because the network
layer changed and started taking a mutex in that path, so we could no
longer hold the frwd_lock.

Instead of trying to maintain multiple refcounts, this just has us use a
mutex for accessing the socket in the interface code paths.

Link: https://lore.kernel.org/r/20220907221700.10302-1-michael.christie@oracle.com


Fixes: bcf3a295 ("scsi: iscsi: iscsi_tcp: Avoid holding spinlock while calling getpeername()")
Signed-off-by: default avatarMike Christie <michael.christie@oracle.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: default avatarZhong Jinghua <zhongjinghua@huawei.com>
parent 03d56375

drivers/scsi/iscsi_tcp.c

+52 −21
Original line number Diff line number Diff line
@@ -566,6 +566,8 @@ iscsi_sw_tcp_conn_create(struct iscsi_cls_session *cls_session, 
	tcp_conn = conn->dd_data;

	tcp_sw_conn = tcp_conn->dd_data;



	mutex_init(&tcp_sw_conn->sock_lock);



	tfm = crypto_alloc_ahash("crc32c", 0, CRYPTO_ALG_ASYNC);

	if (IS_ERR(tfm))

		goto free_conn;
@@ -600,11 +602,15 @@ iscsi_sw_tcp_conn_create(struct iscsi_cls_session *cls_session, 


static void iscsi_sw_tcp_release_conn(struct iscsi_conn *conn)

{

	struct iscsi_session *session = conn->session;

	struct iscsi_tcp_conn *tcp_conn = conn->dd_data;

	struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;

	struct socket *sock = tcp_sw_conn->sock;



	/*

	 * The iscsi transport class will make sure we are not called in

	 * parallel with start, stop, bind and destroys. However, this can be

	 * called twice if userspace does a stop then a destroy.

	 */

	if (!sock)

		return;
@@ -612,9 +618,9 @@ static void iscsi_sw_tcp_release_conn(struct iscsi_conn *conn) 
	iscsi_sw_tcp_conn_restore_callbacks(conn);

	sock_put(sock->sk);



	spin_lock_bh(&session->frwd_lock);

	mutex_lock(&tcp_sw_conn->sock_lock);

	tcp_sw_conn->sock = NULL;

	spin_unlock_bh(&session->frwd_lock);

	mutex_unlock(&tcp_sw_conn->sock_lock);

	sockfd_put(sock);

}
@@ -666,7 +672,6 @@ iscsi_sw_tcp_conn_bind(struct iscsi_cls_session *cls_session, 
		       struct iscsi_cls_conn *cls_conn, uint64_t transport_eph,

		       int is_leading)

{

	struct iscsi_session *session = cls_session->dd_data;

	struct iscsi_conn *conn = cls_conn->dd_data;

	struct iscsi_tcp_conn *tcp_conn = conn->dd_data;

	struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
@@ -686,10 +691,10 @@ iscsi_sw_tcp_conn_bind(struct iscsi_cls_session *cls_session, 
	if (err)

		goto free_socket;



	spin_lock_bh(&session->frwd_lock);

	mutex_lock(&tcp_sw_conn->sock_lock);

	/* bind iSCSI connection and socket */

	tcp_sw_conn->sock = sock;

	spin_unlock_bh(&session->frwd_lock);

	mutex_unlock(&tcp_sw_conn->sock_lock);



	/* setup Socket parameters */

	sk = sock->sk;
@@ -725,8 +730,15 @@ static int iscsi_sw_tcp_conn_set_param(struct iscsi_cls_conn *cls_conn, 
		break;

	case ISCSI_PARAM_DATADGST_EN:

		iscsi_set_param(cls_conn, param, buf, buflen);



		mutex_lock(&tcp_sw_conn->sock_lock);

		if (!tcp_sw_conn->sock) {

			mutex_unlock(&tcp_sw_conn->sock_lock);

			return -ENOTCONN;

		}

		tcp_sw_conn->sendpage = conn->datadgst_en ?

			sock_no_sendpage : tcp_sw_conn->sock->ops->sendpage;

		mutex_unlock(&tcp_sw_conn->sock_lock);

		break;

	case ISCSI_PARAM_MAX_R2T:

		return iscsi_tcp_set_max_r2t(conn, buf);
@@ -741,8 +753,8 @@ static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn, 
				       enum iscsi_param param, char *buf)

{

	struct iscsi_conn *conn = cls_conn->dd_data;

	struct iscsi_tcp_conn *tcp_conn = conn->dd_data;

	struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;

	struct iscsi_sw_tcp_conn *tcp_sw_conn;

	struct iscsi_tcp_conn *tcp_conn;

	struct sockaddr_in6 addr;

	struct socket *sock;

	int rc;
@@ -752,21 +764,36 @@ static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn, 
	case ISCSI_PARAM_CONN_ADDRESS:

	case ISCSI_PARAM_LOCAL_PORT:

		spin_lock_bh(&conn->session->frwd_lock);

		if (!tcp_sw_conn || !tcp_sw_conn->sock) {

		if (!conn->session->leadconn) {

			spin_unlock_bh(&conn->session->frwd_lock);

			return -ENOTCONN;

		}

		sock = tcp_sw_conn->sock;

		sock_hold(sock->sk);

		/*

		 * The conn has been setup and bound, so just grab a ref

		 * incase a destroy runs while we are in the net layer.

		 */

		iscsi_get_conn(conn->cls_conn);

		spin_unlock_bh(&conn->session->frwd_lock);



		tcp_conn = conn->dd_data;

		tcp_sw_conn = tcp_conn->dd_data;



		mutex_lock(&tcp_sw_conn->sock_lock);

		sock = tcp_sw_conn->sock;

		if (!sock) {

			rc = -ENOTCONN;

			goto sock_unlock;

		}



		if (param == ISCSI_PARAM_LOCAL_PORT)

			rc = kernel_getsockname(sock,

						(struct sockaddr *)&addr);

		else

			rc = kernel_getpeername(sock,

						(struct sockaddr *)&addr);

		sock_put(sock->sk);

sock_unlock:

		mutex_unlock(&tcp_sw_conn->sock_lock);

		iscsi_put_conn(conn->cls_conn);

		if (rc < 0)

			return rc;
@@ -805,17 +832,21 @@ static int iscsi_sw_tcp_host_get_param(struct Scsi_Host *shost, 
		}

		tcp_conn = conn->dd_data;

		tcp_sw_conn = tcp_conn->dd_data;

		sock = tcp_sw_conn->sock;

		if (!sock) {

			spin_unlock_bh(&session->frwd_lock);

			return -ENOTCONN;

		}

		sock_hold(sock->sk);

		/*

		 * The conn has been setup and bound, so just grab a ref

		 * incase a destroy runs while we are in the net layer.

		 */

		iscsi_get_conn(conn->cls_conn);

		spin_unlock_bh(&session->frwd_lock);



		rc = kernel_getsockname(sock,

					(struct sockaddr *)&addr);

		sock_put(sock->sk);

		mutex_lock(&tcp_sw_conn->sock_lock);

		sock = tcp_sw_conn->sock;

		if (!sock)

			rc = -ENOTCONN;

		else

			rc = kernel_getsockname(sock, (struct sockaddr *)&addr);

		mutex_unlock(&tcp_sw_conn->sock_lock);

		iscsi_put_conn(conn->cls_conn);

		if (rc < 0)

			return rc;

drivers/scsi/iscsi_tcp.h

+2 −0
Original line number Diff line number Diff line
@@ -28,6 +28,8 @@ struct iscsi_sw_tcp_send { 


struct iscsi_sw_tcp_conn {

	struct socket		*sock;

	/* Taken when accessing the sock from the netlink/sysfs interface */

	struct mutex		sock_lock;



	struct iscsi_sw_tcp_send out;

	/* old values for socket callbacks */