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: Al 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: Nicholas Bellinger <nab@linux-iscsi.org>

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 5d56ce44a6c58ccbaddb69290d14847dc1bd49bd..97c0f78c3c9ca87232fceff82943d293a036d2e1 100644 (file)
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -429,19 +429,8 @@ 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;
-       }
-
-       sock_release(np->np_socket);
+       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");

diff --git a/drivers/target/iscsi/iscsi_target_core.h b/drivers/target/iscsi/iscsi_target_core.h
index 63a8ed26f1191ad473ba843d700ee27bdf4eaa7c..8a908b28d8b2b1455b0cb4c0816131d418912ae8 100644 (file)
--- a/drivers/target/iscsi/iscsi_target_core.h
+++ b/drivers/target/iscsi/iscsi_target_core.h
@@ -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;

diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
index 1434110b052c598e6f5038c1a7a93ecf27ace645..0694d9b1bce6a4e066a7bba1b1ef5f870b4b51d7 100644 (file)
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -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 @@ old_sess_out:
                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) {