Skip to content

Commit

Permalink
iscsi-target: Drop bogus struct file usage for iSCSI/SCTP
Browse files Browse the repository at this point in the history 
commit bf6932f upstream.

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>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
  • Loading branch information
@bwhacks
Al Viro authored and bwhacks committed Aug 2, 2012
1 parent 533c8ed commit e87ebd5
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 78 deletions.
22 changes: 3 additions & 19 deletions drivers/target/iscsi/iscsi_target.c
View file
Expand Up @@ -427,19 +427,8 @@ int iscsit_reset_np_thread(


int iscsit_del_np_comm(struct iscsi_np *np) int iscsit_del_np_comm(struct iscsi_np *np)
{ {
if (!np->np_socket) if (np->np_socket)
return 0; sock_release(np->np_socket);

/*
* 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);
return 0; return 0;
} }


Expand Down Expand Up @@ -4105,13 +4094,8 @@ int iscsit_close_connection(
kfree(conn->conn_ops); kfree(conn->conn_ops);
conn->conn_ops = NULL; conn->conn_ops = NULL;


if (conn->sock) { if (conn->sock)
if (conn->conn_flags & CONNFLAG_SCTP_STRUCT_FILE) {
kfree(conn->sock->file);
conn->sock->file = NULL;
}
sock_release(conn->sock); sock_release(conn->sock);
}
conn->thread_set = NULL; conn->thread_set = NULL;


pr_debug("Moving to TARG_CONN_STATE_FREE.\n"); pr_debug("Moving to TARG_CONN_STATE_FREE.\n");
Expand Down
2 changes: 0 additions & 2 deletions drivers/target/iscsi/iscsi_target_core.h
View file
Expand Up @@ -224,7 +224,6 @@ enum iscsi_timer_flags_table {
/* Used for struct iscsi_np->np_flags */ /* Used for struct iscsi_np->np_flags */
enum np_flags_table { enum np_flags_table {
NPF_IP_NETWORK = 0x00, NPF_IP_NETWORK = 0x00,
NPF_SCTP_STRUCT_FILE = 0x01 /* Bugfix */
}; };


/* Used for struct iscsi_np->np_thread_state */ /* Used for struct iscsi_np->np_thread_state */
Expand Down Expand Up @@ -511,7 +510,6 @@ struct iscsi_conn {
u16 local_port; u16 local_port;
int net_size; int net_size;
u32 auth_id; u32 auth_id;
#define CONNFLAG_SCTP_STRUCT_FILE 0x01
u32 conn_flags; u32 conn_flags;
/* Used for iscsi_tx_login_rsp() */ /* Used for iscsi_tx_login_rsp() */
u32 login_itt; u32 login_itt;
Expand Down
60 changes: 3 additions & 57 deletions drivers/target/iscsi/iscsi_target_login.c
View file
Expand Up @@ -792,22 +792,6 @@ int iscsi_target_setup_login_socket(
return ret; return ret;
} }
np->np_socket = sock; 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 * Setup the np->np_sockaddr from the passed sockaddr setup
* in iscsi_target_configfs.c code.. * in iscsi_target_configfs.c code..
Expand Down Expand Up @@ -857,21 +841,15 @@ int iscsi_target_setup_login_socket(


fail: fail:
np->np_socket = NULL; np->np_socket = NULL;
if (sock) { if (sock)
if (np->np_flags & NPF_SCTP_STRUCT_FILE) {
kfree(sock->file);
sock->file = NULL;
}

sock_release(sock); sock_release(sock);
}
return ret; return ret;
} }


static int __iscsi_target_login_thread(struct iscsi_np *np) static int __iscsi_target_login_thread(struct iscsi_np *np)
{ {
u8 buffer[ISCSI_HDR_LEN], iscsi_opcode, zero_tsih = 0; u8 buffer[ISCSI_HDR_LEN], iscsi_opcode, zero_tsih = 0;
int err, ret = 0, ip_proto, sock_type, set_sctp_conn_flag, stop; int err, ret = 0, ip_proto, sock_type, stop;
struct iscsi_conn *conn = NULL; struct iscsi_conn *conn = NULL;
struct iscsi_login *login; struct iscsi_login *login;
struct iscsi_portal_group *tpg = NULL; struct iscsi_portal_group *tpg = NULL;
Expand All @@ -882,7 +860,6 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
struct sockaddr_in6 sock_in6; struct sockaddr_in6 sock_in6;


flush_signals(current); flush_signals(current);
set_sctp_conn_flag = 0;
sock = np->np_socket; sock = np->np_socket;
ip_proto = np->np_ip_proto; ip_proto = np->np_ip_proto;
sock_type = np->np_sock_type; sock_type = np->np_sock_type;
Expand All @@ -907,35 +884,12 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
spin_unlock_bh(&np->np_thread_lock); spin_unlock_bh(&np->np_thread_lock);
goto out; 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); iscsi_start_login_thread_timer(np);


conn = kzalloc(sizeof(struct iscsi_conn), GFP_KERNEL); conn = kzalloc(sizeof(struct iscsi_conn), GFP_KERNEL);
if (!conn) { if (!conn) {
pr_err("Could not allocate memory for" pr_err("Could not allocate memory for"
" new connection\n"); " new connection\n");
if (set_sctp_conn_flag) {
kfree(new_sock->file);
new_sock->file = NULL;
}
sock_release(new_sock); sock_release(new_sock);
/* Get another socket */ /* Get another socket */
return 1; return 1;
Expand All @@ -945,9 +899,6 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
conn->conn_state = TARG_CONN_STATE_FREE; conn->conn_state = TARG_CONN_STATE_FREE;
conn->sock = new_sock; 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"); pr_debug("Moving to TARG_CONN_STATE_XPT_UP.\n");
conn->conn_state = TARG_CONN_STATE_XPT_UP; conn->conn_state = TARG_CONN_STATE_XPT_UP;


Expand Down Expand Up @@ -1195,13 +1146,8 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
iscsi_release_param_list(conn->param_list); iscsi_release_param_list(conn->param_list);
conn->param_list = NULL; conn->param_list = NULL;
} }
if (conn->sock) { if (conn->sock)
if (conn->conn_flags & CONNFLAG_SCTP_STRUCT_FILE) {
kfree(conn->sock->file);
conn->sock->file = NULL;
}
sock_release(conn->sock); sock_release(conn->sock);
}
kfree(conn); kfree(conn);


if (tpg) { if (tpg) {
Expand Down

0 comments on commit e87ebd5

Please sign in to comment.