diff options
author | Al Viro <viro@ZenIV.linux.org.uk> | 2012-07-21 03:55:18 -0400 |
---|---|---|
committer | Nicholas Bellinger <nab@linux-iscsi.org> | 2012-07-21 05:44:13 -0400 |
commit | bf6932f44a7b3fa7e2246a8b18a44670e5eab6c2 (patch) | |
tree | d1a0ac63a2e2a04263b36ab6b4ff97bacbe88fa2 /drivers/target | |
parent | 2962846d14769e526b5d266f4af998b5a027b1d7 (diff) |
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>
Diffstat (limited to 'drivers/target')
-rw-r--r-- | drivers/target/iscsi/iscsi_target.c | 22 | ||||
-rw-r--r-- | drivers/target/iscsi/iscsi_target_core.h | 2 | ||||
-rw-r--r-- | drivers/target/iscsi/iscsi_target_login.c | 60 |
3 files changed, 6 insertions, 78 deletions
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 5d56ce44a6c5..97c0f78c3c9c 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c | |||
@@ -429,19 +429,8 @@ int iscsit_reset_np_thread( | |||
429 | 429 | ||
430 | int iscsit_del_np_comm(struct iscsi_np *np) | 430 | int iscsit_del_np_comm(struct iscsi_np *np) |
431 | { | 431 | { |
432 | if (!np->np_socket) | 432 | if (np->np_socket) |
433 | return 0; | 433 | sock_release(np->np_socket); |
434 | |||
435 | /* | ||
436 | * Some network transports allocate their own struct sock->file, | ||
437 | * see if we need to free any additional allocated resources. | ||
438 | */ | ||
439 | if (np->np_flags & NPF_SCTP_STRUCT_FILE) { | ||
440 | kfree(np->np_socket->file); | ||
441 | np->np_socket->file = NULL; | ||
442 | } | ||
443 | |||
444 | sock_release(np->np_socket); | ||
445 | return 0; | 434 | return 0; |
446 | } | 435 | } |
447 | 436 | ||
@@ -4079,13 +4068,8 @@ int iscsit_close_connection( | |||
4079 | kfree(conn->conn_ops); | 4068 | kfree(conn->conn_ops); |
4080 | conn->conn_ops = NULL; | 4069 | conn->conn_ops = NULL; |
4081 | 4070 | ||
4082 | if (conn->sock) { | 4071 | if (conn->sock) |
4083 | if (conn->conn_flags & CONNFLAG_SCTP_STRUCT_FILE) { | ||
4084 | kfree(conn->sock->file); | ||
4085 | conn->sock->file = NULL; | ||
4086 | } | ||
4087 | sock_release(conn->sock); | 4072 | sock_release(conn->sock); |
4088 | } | ||
4089 | conn->thread_set = NULL; | 4073 | conn->thread_set = NULL; |
4090 | 4074 | ||
4091 | pr_debug("Moving to TARG_CONN_STATE_FREE.\n"); | 4075 | 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 63a8ed26f119..8a908b28d8b2 100644 --- 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 { | |||
224 | /* Used for struct iscsi_np->np_flags */ | 224 | /* Used for struct iscsi_np->np_flags */ |
225 | enum np_flags_table { | 225 | enum np_flags_table { |
226 | NPF_IP_NETWORK = 0x00, | 226 | NPF_IP_NETWORK = 0x00, |
227 | NPF_SCTP_STRUCT_FILE = 0x01 /* Bugfix */ | ||
228 | }; | 227 | }; |
229 | 228 | ||
230 | /* Used for struct iscsi_np->np_thread_state */ | 229 | /* Used for struct iscsi_np->np_thread_state */ |
@@ -504,7 +503,6 @@ struct iscsi_conn { | |||
504 | u16 local_port; | 503 | u16 local_port; |
505 | int net_size; | 504 | int net_size; |
506 | u32 auth_id; | 505 | u32 auth_id; |
507 | #define CONNFLAG_SCTP_STRUCT_FILE 0x01 | ||
508 | u32 conn_flags; | 506 | u32 conn_flags; |
509 | /* Used for iscsi_tx_login_rsp() */ | 507 | /* Used for iscsi_tx_login_rsp() */ |
510 | u32 login_itt; | 508 | u32 login_itt; |
diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c index 1434110b052c..0694d9b1bce6 100644 --- a/drivers/target/iscsi/iscsi_target_login.c +++ b/drivers/target/iscsi/iscsi_target_login.c | |||
@@ -795,22 +795,6 @@ int iscsi_target_setup_login_socket( | |||
795 | } | 795 | } |
796 | np->np_socket = sock; | 796 | np->np_socket = sock; |
797 | /* | 797 | /* |
798 | * The SCTP stack needs struct socket->file. | ||
799 | */ | ||
800 | if ((np->np_network_transport == ISCSI_SCTP_TCP) || | ||
801 | (np->np_network_transport == ISCSI_SCTP_UDP)) { | ||
802 | if (!sock->file) { | ||
803 | sock->file = kzalloc(sizeof(struct file), GFP_KERNEL); | ||
804 | if (!sock->file) { | ||
805 | pr_err("Unable to allocate struct" | ||
806 | " file for SCTP\n"); | ||
807 | ret = -ENOMEM; | ||
808 | goto fail; | ||
809 | } | ||
810 | np->np_flags |= NPF_SCTP_STRUCT_FILE; | ||
811 | } | ||
812 | } | ||
813 | /* | ||
814 | * Setup the np->np_sockaddr from the passed sockaddr setup | 798 | * Setup the np->np_sockaddr from the passed sockaddr setup |
815 | * in iscsi_target_configfs.c code.. | 799 | * in iscsi_target_configfs.c code.. |
816 | */ | 800 | */ |
@@ -869,21 +853,15 @@ int iscsi_target_setup_login_socket( | |||
869 | 853 | ||
870 | fail: | 854 | fail: |
871 | np->np_socket = NULL; | 855 | np->np_socket = NULL; |
872 | if (sock) { | 856 | if (sock) |
873 | if (np->np_flags & NPF_SCTP_STRUCT_FILE) { | ||
874 | kfree(sock->file); | ||
875 | sock->file = NULL; | ||
876 | } | ||
877 | |||
878 | sock_release(sock); | 857 | sock_release(sock); |
879 | } | ||
880 | return ret; | 858 | return ret; |
881 | } | 859 | } |
882 | 860 | ||
883 | static int __iscsi_target_login_thread(struct iscsi_np *np) | 861 | static int __iscsi_target_login_thread(struct iscsi_np *np) |
884 | { | 862 | { |
885 | u8 buffer[ISCSI_HDR_LEN], iscsi_opcode, zero_tsih = 0; | 863 | u8 buffer[ISCSI_HDR_LEN], iscsi_opcode, zero_tsih = 0; |
886 | int err, ret = 0, set_sctp_conn_flag, stop; | 864 | int err, ret = 0, stop; |
887 | struct iscsi_conn *conn = NULL; | 865 | struct iscsi_conn *conn = NULL; |
888 | struct iscsi_login *login; | 866 | struct iscsi_login *login; |
889 | struct iscsi_portal_group *tpg = NULL; | 867 | struct iscsi_portal_group *tpg = NULL; |
@@ -894,7 +872,6 @@ static int __iscsi_target_login_thread(struct iscsi_np *np) | |||
894 | struct sockaddr_in6 sock_in6; | 872 | struct sockaddr_in6 sock_in6; |
895 | 873 | ||
896 | flush_signals(current); | 874 | flush_signals(current); |
897 | set_sctp_conn_flag = 0; | ||
898 | sock = np->np_socket; | 875 | sock = np->np_socket; |
899 | 876 | ||
900 | spin_lock_bh(&np->np_thread_lock); | 877 | spin_lock_bh(&np->np_thread_lock); |
@@ -917,35 +894,12 @@ static int __iscsi_target_login_thread(struct iscsi_np *np) | |||
917 | spin_unlock_bh(&np->np_thread_lock); | 894 | spin_unlock_bh(&np->np_thread_lock); |
918 | goto out; | 895 | goto out; |
919 | } | 896 | } |
920 | /* | ||
921 | * The SCTP stack needs struct socket->file. | ||
922 | */ | ||
923 | if ((np->np_network_transport == ISCSI_SCTP_TCP) || | ||
924 | (np->np_network_transport == ISCSI_SCTP_UDP)) { | ||
925 | if (!new_sock->file) { | ||
926 | new_sock->file = kzalloc( | ||
927 | sizeof(struct file), GFP_KERNEL); | ||
928 | if (!new_sock->file) { | ||
929 | pr_err("Unable to allocate struct" | ||
930 | " file for SCTP\n"); | ||
931 | sock_release(new_sock); | ||
932 | /* Get another socket */ | ||
933 | return 1; | ||
934 | } | ||
935 | set_sctp_conn_flag = 1; | ||
936 | } | ||
937 | } | ||
938 | |||
939 | iscsi_start_login_thread_timer(np); | 897 | iscsi_start_login_thread_timer(np); |
940 | 898 | ||
941 | conn = kzalloc(sizeof(struct iscsi_conn), GFP_KERNEL); | 899 | conn = kzalloc(sizeof(struct iscsi_conn), GFP_KERNEL); |
942 | if (!conn) { | 900 | if (!conn) { |
943 | pr_err("Could not allocate memory for" | 901 | pr_err("Could not allocate memory for" |
944 | " new connection\n"); | 902 | " new connection\n"); |
945 | if (set_sctp_conn_flag) { | ||
946 | kfree(new_sock->file); | ||
947 | new_sock->file = NULL; | ||
948 | } | ||
949 | sock_release(new_sock); | 903 | sock_release(new_sock); |
950 | /* Get another socket */ | 904 | /* Get another socket */ |
951 | return 1; | 905 | return 1; |
@@ -955,9 +909,6 @@ static int __iscsi_target_login_thread(struct iscsi_np *np) | |||
955 | conn->conn_state = TARG_CONN_STATE_FREE; | 909 | conn->conn_state = TARG_CONN_STATE_FREE; |
956 | conn->sock = new_sock; | 910 | conn->sock = new_sock; |
957 | 911 | ||
958 | if (set_sctp_conn_flag) | ||
959 | conn->conn_flags |= CONNFLAG_SCTP_STRUCT_FILE; | ||
960 | |||
961 | pr_debug("Moving to TARG_CONN_STATE_XPT_UP.\n"); | 912 | pr_debug("Moving to TARG_CONN_STATE_XPT_UP.\n"); |
962 | conn->conn_state = TARG_CONN_STATE_XPT_UP; | 913 | conn->conn_state = TARG_CONN_STATE_XPT_UP; |
963 | 914 | ||
@@ -1205,13 +1156,8 @@ old_sess_out: | |||
1205 | iscsi_release_param_list(conn->param_list); | 1156 | iscsi_release_param_list(conn->param_list); |
1206 | conn->param_list = NULL; | 1157 | conn->param_list = NULL; |
1207 | } | 1158 | } |
1208 | if (conn->sock) { | 1159 | if (conn->sock) |
1209 | if (conn->conn_flags & CONNFLAG_SCTP_STRUCT_FILE) { | ||
1210 | kfree(conn->sock->file); | ||
1211 | conn->sock->file = NULL; | ||
1212 | } | ||
1213 | sock_release(conn->sock); | 1160 | sock_release(conn->sock); |
1214 | } | ||
1215 | kfree(conn); | 1161 | kfree(conn); |
1216 | 1162 | ||
1217 | if (tpg) { | 1163 | if (tpg) { |