Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit bf6932f4 authored by Al Viro's avatar Al Viro Committed by Nicholas Bellinger
Browse files

iscsi-target: Drop bogus struct file usage for iSCSI/SCTP 



From Al Viro:

	BTW, speaking of struct file treatment related to sockets -
        there's this piece of code in iscsi:
        /*
         * The SCTP stack needs struct socket->file.
         */
        if ((np->np_network_transport == ISCSI_SCTP_TCP) ||
            (np->np_network_transport == ISCSI_SCTP_UDP)) {
                if (!new_sock->file) {
                        new_sock->file = kzalloc(
                                        sizeof(struct file), GFP_KERNEL);

For one thing, as far as I can see it'not true - sctp does *not* depend on
socket->file being non-NULL; it does, in one place, check socket->file->f_flags
for O_NONBLOCK, but there it treats NULL socket->file as "flag not set".
Which is the case here anyway - the fake struct file created in
__iscsi_target_login_thread() (and in iscsi_target_setup_login_socket(), with
the same excuse) do *not* get that flag set.

Moreover, it's a bloody serious violation of a bunch of asserts in VFS;
all struct file instances should come from filp_cachep, via get_empty_filp()
(or alloc_file(), which is a wrapper for it).  FWIW, I'm very tempted to
do this and be done with the entire mess:

Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
Cc: Andy Grover <agrover@redhat.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: stable@vger.kernel.org
Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
parent 2962846d

drivers/target/iscsi/iscsi_target.c

+3 −19
Original line number Diff line number Diff line
@@ -429,18 +429,7 @@ int iscsit_reset_np_thread( 


int iscsit_del_np_comm(struct iscsi_np *np)

{

	if (!np->np_socket)

		return 0;



	/*

	 * Some network transports allocate their own struct sock->file,

	 * see  if we need to free any additional allocated resources.

	 */

	if (np->np_flags & NPF_SCTP_STRUCT_FILE) {

		kfree(np->np_socket->file);

		np->np_socket->file = NULL;

	}



	if (np->np_socket)

		sock_release(np->np_socket);

	return 0;

}
@@ -4079,13 +4068,8 @@ int iscsit_close_connection( 
	kfree(conn->conn_ops);

	conn->conn_ops = NULL;



	if (conn->sock) {

		if (conn->conn_flags & CONNFLAG_SCTP_STRUCT_FILE) {

			kfree(conn->sock->file);

			conn->sock->file = NULL;

		}

	if (conn->sock)

		sock_release(conn->sock);

	}

	conn->thread_set = NULL;



	pr_debug("Moving to TARG_CONN_STATE_FREE.\n");

drivers/target/iscsi/iscsi_target_core.h

+0 −2
Original line number Diff line number Diff line
@@ -224,7 +224,6 @@ enum iscsi_timer_flags_table { 
/* Used for struct iscsi_np->np_flags */

enum np_flags_table {

	NPF_IP_NETWORK		= 0x00,

	NPF_SCTP_STRUCT_FILE	= 0x01 /* Bugfix */

};



/* Used for struct iscsi_np->np_thread_state */
@@ -504,7 +503,6 @@ struct iscsi_conn { 
	u16			local_port;

	int			net_size;

	u32			auth_id;

#define CONNFLAG_SCTP_STRUCT_FILE			0x01

	u32			conn_flags;

	/* Used for iscsi_tx_login_rsp() */

	u32			login_itt;

drivers/target/iscsi/iscsi_target_login.c

+3 −57
Original line number Diff line number Diff line
@@ -794,22 +794,6 @@ int iscsi_target_setup_login_socket( 
		return ret;

	}

	np->np_socket = sock;

	/*

	 * The SCTP stack needs struct socket->file.

	 */

	if ((np->np_network_transport == ISCSI_SCTP_TCP) ||

	    (np->np_network_transport == ISCSI_SCTP_UDP)) {

		if (!sock->file) {

			sock->file = kzalloc(sizeof(struct file), GFP_KERNEL);

			if (!sock->file) {

				pr_err("Unable to allocate struct"

						" file for SCTP\n");

				ret = -ENOMEM;

				goto fail;

			}

			np->np_flags |= NPF_SCTP_STRUCT_FILE;

		}

	}

	/*

	 * Setup the np->np_sockaddr from the passed sockaddr setup

	 * in iscsi_target_configfs.c code..
@@ -869,21 +853,15 @@ int iscsi_target_setup_login_socket( 


fail:

	np->np_socket = NULL;

	if (sock) {

		if (np->np_flags & NPF_SCTP_STRUCT_FILE) {

			kfree(sock->file);

			sock->file = NULL;

		}



	if (sock)

		sock_release(sock);

	}

	return ret;

}



static int __iscsi_target_login_thread(struct iscsi_np *np)

{

	u8 buffer[ISCSI_HDR_LEN], iscsi_opcode, zero_tsih = 0;

	int err, ret = 0, set_sctp_conn_flag, stop;

	int err, ret = 0, stop;

	struct iscsi_conn *conn = NULL;

	struct iscsi_login *login;

	struct iscsi_portal_group *tpg = NULL;
@@ -894,7 +872,6 @@ static int __iscsi_target_login_thread(struct iscsi_np *np) 
	struct sockaddr_in6 sock_in6;



	flush_signals(current);

	set_sctp_conn_flag = 0;

	sock = np->np_socket;



	spin_lock_bh(&np->np_thread_lock);
@@ -917,35 +894,12 @@ static int __iscsi_target_login_thread(struct iscsi_np *np) 
		spin_unlock_bh(&np->np_thread_lock);

		goto out;

	}

	/*

	 * The SCTP stack needs struct socket->file.

	 */

	if ((np->np_network_transport == ISCSI_SCTP_TCP) ||

	    (np->np_network_transport == ISCSI_SCTP_UDP)) {

		if (!new_sock->file) {

			new_sock->file = kzalloc(

					sizeof(struct file), GFP_KERNEL);

			if (!new_sock->file) {

				pr_err("Unable to allocate struct"

						" file for SCTP\n");

				sock_release(new_sock);

				/* Get another socket */

				return 1;

			}

			set_sctp_conn_flag = 1;

		}

	}



	iscsi_start_login_thread_timer(np);



	conn = kzalloc(sizeof(struct iscsi_conn), GFP_KERNEL);

	if (!conn) {

		pr_err("Could not allocate memory for"

			" new connection\n");

		if (set_sctp_conn_flag) {

			kfree(new_sock->file);

			new_sock->file = NULL;

		}

		sock_release(new_sock);

		/* Get another socket */

		return 1;
@@ -955,9 +909,6 @@ static int __iscsi_target_login_thread(struct iscsi_np *np) 
	conn->conn_state = TARG_CONN_STATE_FREE;

	conn->sock = new_sock;



	if (set_sctp_conn_flag)

		conn->conn_flags |= CONNFLAG_SCTP_STRUCT_FILE;



	pr_debug("Moving to TARG_CONN_STATE_XPT_UP.\n");

	conn->conn_state = TARG_CONN_STATE_XPT_UP;
@@ -1205,13 +1156,8 @@ static int __iscsi_target_login_thread(struct iscsi_np *np) 
		iscsi_release_param_list(conn->param_list);

		conn->param_list = NULL;

	}

	if (conn->sock) {

		if (conn->conn_flags & CONNFLAG_SCTP_STRUCT_FILE) {

			kfree(conn->sock->file);

			conn->sock->file = NULL;

		}

	if (conn->sock)

		sock_release(conn->sock);

	}

	kfree(conn);



	if (tpg) {