diff options
author | Al Viro <viro@zeniv.linux.org.uk> | 2009-08-05 18:02:43 -0400 |
---|---|---|
committer | Al Viro <viro@zeniv.linux.org.uk> | 2009-12-16 12:16:41 -0500 |
commit | 6b18662e239a032f908b7f6e164bdf7e2e0a32c9 (patch) | |
tree | b63bf63d7d2b0d0fac2529a3c3bd906f85388c79 | |
parent | 7cbe66b6b53b6615f1033bd5b3dbad8162886373 (diff) |
9p connect fixes
* if we fail in p9_conn_create(), we shouldn't leak references to struct file.
Logics in ->close() doesn't help - ->trans is already gone by the time it's
called.
* sock_create_kern() can fail.
* use of sock_map_fd() is all fscked up; I'd fixed most of that, but the
rest will have to wait for a bit more work in net/socket.c (we still are
violating the basic rule of working with descriptor table: "once the reference
is installed there, don't rely on finding it there again").
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
-rw-r--r-- | net/9p/trans_fd.c | 112 |
1 files changed, 46 insertions, 66 deletions
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c index 4dd873e3a1bb..be1cb909d8c0 100644 --- a/net/9p/trans_fd.c +++ b/net/9p/trans_fd.c | |||
@@ -42,6 +42,8 @@ | |||
42 | #include <net/9p/client.h> | 42 | #include <net/9p/client.h> |
43 | #include <net/9p/transport.h> | 43 | #include <net/9p/transport.h> |
44 | 44 | ||
45 | #include <linux/syscalls.h> /* killme */ | ||
46 | |||
45 | #define P9_PORT 564 | 47 | #define P9_PORT 564 |
46 | #define MAX_SOCK_BUF (64*1024) | 48 | #define MAX_SOCK_BUF (64*1024) |
47 | #define MAXPOLLWADDR 2 | 49 | #define MAXPOLLWADDR 2 |
@@ -788,24 +790,41 @@ static int p9_fd_open(struct p9_client *client, int rfd, int wfd) | |||
788 | 790 | ||
789 | static int p9_socket_open(struct p9_client *client, struct socket *csocket) | 791 | static int p9_socket_open(struct p9_client *client, struct socket *csocket) |
790 | { | 792 | { |
791 | int fd, ret; | 793 | struct p9_trans_fd *p; |
794 | int ret, fd; | ||
795 | |||
796 | p = kmalloc(sizeof(struct p9_trans_fd), GFP_KERNEL); | ||
797 | if (!p) | ||
798 | return -ENOMEM; | ||
792 | 799 | ||
793 | csocket->sk->sk_allocation = GFP_NOIO; | 800 | csocket->sk->sk_allocation = GFP_NOIO; |
794 | fd = sock_map_fd(csocket, 0); | 801 | fd = sock_map_fd(csocket, 0); |
795 | if (fd < 0) { | 802 | if (fd < 0) { |
796 | P9_EPRINTK(KERN_ERR, "p9_socket_open: failed to map fd\n"); | 803 | P9_EPRINTK(KERN_ERR, "p9_socket_open: failed to map fd\n"); |
804 | sock_release(csocket); | ||
805 | kfree(p); | ||
797 | return fd; | 806 | return fd; |
798 | } | 807 | } |
799 | 808 | ||
800 | ret = p9_fd_open(client, fd, fd); | 809 | get_file(csocket->file); |
801 | if (ret < 0) { | 810 | get_file(csocket->file); |
802 | P9_EPRINTK(KERN_ERR, "p9_socket_open: failed to open fd\n"); | 811 | p->wr = p->rd = csocket->file; |
812 | client->trans = p; | ||
813 | client->status = Connected; | ||
814 | |||
815 | sys_close(fd); /* still racy */ | ||
816 | |||
817 | p->rd->f_flags |= O_NONBLOCK; | ||
818 | |||
819 | p->conn = p9_conn_create(client); | ||
820 | if (IS_ERR(p->conn)) { | ||
821 | ret = PTR_ERR(p->conn); | ||
822 | p->conn = NULL; | ||
823 | kfree(p); | ||
824 | sockfd_put(csocket); | ||
803 | sockfd_put(csocket); | 825 | sockfd_put(csocket); |
804 | return ret; | 826 | return ret; |
805 | } | 827 | } |
806 | |||
807 | ((struct p9_trans_fd *)client->trans)->rd->f_flags |= O_NONBLOCK; | ||
808 | |||
809 | return 0; | 828 | return 0; |
810 | } | 829 | } |
811 | 830 | ||
@@ -883,7 +902,6 @@ p9_fd_create_tcp(struct p9_client *client, const char *addr, char *args) | |||
883 | struct socket *csocket; | 902 | struct socket *csocket; |
884 | struct sockaddr_in sin_server; | 903 | struct sockaddr_in sin_server; |
885 | struct p9_fd_opts opts; | 904 | struct p9_fd_opts opts; |
886 | struct p9_trans_fd *p = NULL; /* this gets allocated in p9_fd_open */ | ||
887 | 905 | ||
888 | err = parse_opts(args, &opts); | 906 | err = parse_opts(args, &opts); |
889 | if (err < 0) | 907 | if (err < 0) |
@@ -897,12 +915,11 @@ p9_fd_create_tcp(struct p9_client *client, const char *addr, char *args) | |||
897 | sin_server.sin_family = AF_INET; | 915 | sin_server.sin_family = AF_INET; |
898 | sin_server.sin_addr.s_addr = in_aton(addr); | 916 | sin_server.sin_addr.s_addr = in_aton(addr); |
899 | sin_server.sin_port = htons(opts.port); | 917 | sin_server.sin_port = htons(opts.port); |
900 | sock_create_kern(PF_INET, SOCK_STREAM, IPPROTO_TCP, &csocket); | 918 | err = sock_create_kern(PF_INET, SOCK_STREAM, IPPROTO_TCP, &csocket); |
901 | 919 | ||
902 | if (!csocket) { | 920 | if (err) { |
903 | P9_EPRINTK(KERN_ERR, "p9_trans_tcp: problem creating socket\n"); | 921 | P9_EPRINTK(KERN_ERR, "p9_trans_tcp: problem creating socket\n"); |
904 | err = -EIO; | 922 | return err; |
905 | goto error; | ||
906 | } | 923 | } |
907 | 924 | ||
908 | err = csocket->ops->connect(csocket, | 925 | err = csocket->ops->connect(csocket, |
@@ -912,30 +929,11 @@ p9_fd_create_tcp(struct p9_client *client, const char *addr, char *args) | |||
912 | P9_EPRINTK(KERN_ERR, | 929 | P9_EPRINTK(KERN_ERR, |
913 | "p9_trans_tcp: problem connecting socket to %s\n", | 930 | "p9_trans_tcp: problem connecting socket to %s\n", |
914 | addr); | 931 | addr); |
915 | goto error; | ||
916 | } | ||
917 | |||
918 | err = p9_socket_open(client, csocket); | ||
919 | if (err < 0) | ||
920 | goto error; | ||
921 | |||
922 | p = (struct p9_trans_fd *) client->trans; | ||
923 | p->conn = p9_conn_create(client); | ||
924 | if (IS_ERR(p->conn)) { | ||
925 | err = PTR_ERR(p->conn); | ||
926 | p->conn = NULL; | ||
927 | goto error; | ||
928 | } | ||
929 | |||
930 | return 0; | ||
931 | |||
932 | error: | ||
933 | if (csocket) | ||
934 | sock_release(csocket); | 932 | sock_release(csocket); |
933 | return err; | ||
934 | } | ||
935 | 935 | ||
936 | kfree(p); | 936 | return p9_socket_open(client, csocket); |
937 | |||
938 | return err; | ||
939 | } | 937 | } |
940 | 938 | ||
941 | static int | 939 | static int |
@@ -944,49 +942,33 @@ p9_fd_create_unix(struct p9_client *client, const char *addr, char *args) | |||
944 | int err; | 942 | int err; |
945 | struct socket *csocket; | 943 | struct socket *csocket; |
946 | struct sockaddr_un sun_server; | 944 | struct sockaddr_un sun_server; |
947 | struct p9_trans_fd *p = NULL; /* this gets allocated in p9_fd_open */ | ||
948 | 945 | ||
949 | csocket = NULL; | 946 | csocket = NULL; |
950 | 947 | ||
951 | if (strlen(addr) > UNIX_PATH_MAX) { | 948 | if (strlen(addr) > UNIX_PATH_MAX) { |
952 | P9_EPRINTK(KERN_ERR, "p9_trans_unix: address too long: %s\n", | 949 | P9_EPRINTK(KERN_ERR, "p9_trans_unix: address too long: %s\n", |
953 | addr); | 950 | addr); |
954 | err = -ENAMETOOLONG; | 951 | return -ENAMETOOLONG; |
955 | goto error; | ||
956 | } | 952 | } |
957 | 953 | ||
958 | sun_server.sun_family = PF_UNIX; | 954 | sun_server.sun_family = PF_UNIX; |
959 | strcpy(sun_server.sun_path, addr); | 955 | strcpy(sun_server.sun_path, addr); |
960 | sock_create_kern(PF_UNIX, SOCK_STREAM, 0, &csocket); | 956 | err = sock_create_kern(PF_UNIX, SOCK_STREAM, 0, &csocket); |
957 | if (err < 0) { | ||
958 | P9_EPRINTK(KERN_ERR, "p9_trans_unix: problem creating socket\n"); | ||
959 | return err; | ||
960 | } | ||
961 | err = csocket->ops->connect(csocket, (struct sockaddr *)&sun_server, | 961 | err = csocket->ops->connect(csocket, (struct sockaddr *)&sun_server, |
962 | sizeof(struct sockaddr_un) - 1, 0); | 962 | sizeof(struct sockaddr_un) - 1, 0); |
963 | if (err < 0) { | 963 | if (err < 0) { |
964 | P9_EPRINTK(KERN_ERR, | 964 | P9_EPRINTK(KERN_ERR, |
965 | "p9_trans_unix: problem connecting socket: %s: %d\n", | 965 | "p9_trans_unix: problem connecting socket: %s: %d\n", |
966 | addr, err); | 966 | addr, err); |
967 | goto error; | ||
968 | } | ||
969 | |||
970 | err = p9_socket_open(client, csocket); | ||
971 | if (err < 0) | ||
972 | goto error; | ||
973 | |||
974 | p = (struct p9_trans_fd *) client->trans; | ||
975 | p->conn = p9_conn_create(client); | ||
976 | if (IS_ERR(p->conn)) { | ||
977 | err = PTR_ERR(p->conn); | ||
978 | p->conn = NULL; | ||
979 | goto error; | ||
980 | } | ||
981 | |||
982 | return 0; | ||
983 | |||
984 | error: | ||
985 | if (csocket) | ||
986 | sock_release(csocket); | 967 | sock_release(csocket); |
968 | return err; | ||
969 | } | ||
987 | 970 | ||
988 | kfree(p); | 971 | return p9_socket_open(client, csocket); |
989 | return err; | ||
990 | } | 972 | } |
991 | 973 | ||
992 | static int | 974 | static int |
@@ -994,7 +976,7 @@ p9_fd_create(struct p9_client *client, const char *addr, char *args) | |||
994 | { | 976 | { |
995 | int err; | 977 | int err; |
996 | struct p9_fd_opts opts; | 978 | struct p9_fd_opts opts; |
997 | struct p9_trans_fd *p = NULL; /* this get allocated in p9_fd_open */ | 979 | struct p9_trans_fd *p; |
998 | 980 | ||
999 | parse_opts(args, &opts); | 981 | parse_opts(args, &opts); |
1000 | 982 | ||
@@ -1005,21 +987,19 @@ p9_fd_create(struct p9_client *client, const char *addr, char *args) | |||
1005 | 987 | ||
1006 | err = p9_fd_open(client, opts.rfd, opts.wfd); | 988 | err = p9_fd_open(client, opts.rfd, opts.wfd); |
1007 | if (err < 0) | 989 | if (err < 0) |
1008 | goto error; | 990 | return err; |
1009 | 991 | ||
1010 | p = (struct p9_trans_fd *) client->trans; | 992 | p = (struct p9_trans_fd *) client->trans; |
1011 | p->conn = p9_conn_create(client); | 993 | p->conn = p9_conn_create(client); |
1012 | if (IS_ERR(p->conn)) { | 994 | if (IS_ERR(p->conn)) { |
1013 | err = PTR_ERR(p->conn); | 995 | err = PTR_ERR(p->conn); |
1014 | p->conn = NULL; | 996 | p->conn = NULL; |
1015 | goto error; | 997 | fput(p->rd); |
998 | fput(p->wr); | ||
999 | return err; | ||
1016 | } | 1000 | } |
1017 | 1001 | ||
1018 | return 0; | 1002 | return 0; |
1019 | |||
1020 | error: | ||
1021 | kfree(p); | ||
1022 | return err; | ||
1023 | } | 1003 | } |
1024 | 1004 | ||
1025 | static struct p9_trans_module p9_tcp_trans = { | 1005 | static struct p9_trans_module p9_tcp_trans = { |