From b909ec847cf0ed676dc8268d6eee235a12e50e7f Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 29 Jun 2015 19:00:55 +0200 Subject: [PATCH 1/2] lib: Fix rundown of open_socket_out() Under valgrind I've seen the abort in async_connect_cleanup kick in. Yes, it's good that we check these return codes! Bug: https://bugzilla.samba.org/show_bug.cgi?id=11316 Signed-off-by: Volker Lendecke Reviewed-by: "Stefan (metze) Metzmacher" Autobuild-User(master): Volker Lendecke Autobuild-Date(master): Tue Jun 30 20:24:37 CEST 2015 on sn-devel-104 (cherry picked from commit 6fc65aaf956f35e2068e2a6f8521af2f2351d31e) --- source3/lib/util_sock.c | 45 +++++++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/source3/lib/util_sock.c b/source3/lib/util_sock.c index d865ffb..e38a918 100644 --- a/source3/lib/util_sock.c +++ b/source3/lib/util_sock.c @@ -539,16 +539,34 @@ struct open_socket_out_state { socklen_t salen; uint16_t port; int wait_usec; + struct tevent_req *connect_subreq; }; static void open_socket_out_connected(struct tevent_req *subreq); -static int open_socket_out_state_destructor(struct open_socket_out_state *s) +static void open_socket_out_cleanup(struct tevent_req *req, + enum tevent_req_state req_state) { - if (s->fd != -1) { - close(s->fd); + struct open_socket_out_state *state = + tevent_req_data(req, struct open_socket_out_state); + + /* + * Make sure that the async_connect_send subreq has a chance to reset + * fcntl before the socket goes away. + */ + TALLOC_FREE(state->connect_subreq); + + if (req_state == TEVENT_REQ_DONE) { + /* + * we keep the socket open for the caller to use + */ + return; + } + + if (state->fd != -1) { + close(state->fd); + state->fd = -1; } - return 0; } /**************************************************************************** @@ -562,7 +580,7 @@ struct tevent_req *open_socket_out_send(TALLOC_CTX *mem_ctx, int timeout) { char addr[INET6_ADDRSTRLEN]; - struct tevent_req *result, *subreq; + struct tevent_req *result; struct open_socket_out_state *state; NTSTATUS status; @@ -582,7 +600,8 @@ struct tevent_req *open_socket_out_send(TALLOC_CTX *mem_ctx, status = map_nt_error_from_unix(errno); goto post_status; } - talloc_set_destructor(state, open_socket_out_state_destructor); + + tevent_req_set_cleanup_fn(result, open_socket_out_cleanup); if (!tevent_req_set_endtime( result, ev, timeval_current_ofs_msec(timeout))) { @@ -616,16 +635,17 @@ struct tevent_req *open_socket_out_send(TALLOC_CTX *mem_ctx, print_sockaddr(addr, sizeof(addr), &state->ss); DEBUG(3,("Connecting to %s at port %u\n", addr, (unsigned int)port)); - subreq = async_connect_send(state, state->ev, state->fd, - (struct sockaddr *)&state->ss, - state->salen, NULL, NULL, NULL); - if ((subreq == NULL) + state->connect_subreq = async_connect_send( + state, state->ev, state->fd, (struct sockaddr *)&state->ss, + state->salen, NULL, NULL, NULL); + if ((state->connect_subreq == NULL) || !tevent_req_set_endtime( - subreq, state->ev, + state->connect_subreq, state->ev, timeval_current_ofs(0, state->wait_usec))) { goto fail; } - tevent_req_set_callback(subreq, open_socket_out_connected, result); + tevent_req_set_callback(state->connect_subreq, + open_socket_out_connected, result); return result; post_status: @@ -647,6 +667,7 @@ static void open_socket_out_connected(struct tevent_req *subreq) ret = async_connect_recv(subreq, &sys_errno); TALLOC_FREE(subreq); + state->connect_subreq = NULL; if (ret == 0) { tevent_req_done(req); return; -- 1.9.1 From ec5fe94d13ad19602920ea0c14297355af40f083 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 14 Aug 2015 12:54:00 +0200 Subject: [PATCH 2/2] s3:lib: fix some corner cases of open_socket_out_cleanup() In case of timeouts we retry the async_connect_send() and forgot to remember it, this results in an abort() in async_connect_cleanup() as the fd is already closed when calling fcntl(F_SETFL). BUG: https://bugzilla.samba.org/show_bug.cgi?id=11316 Signed-off-by: Stefan Metzmacher Reviewed-by: Andreas Schneider Reviewed-by: Volker Lendecke (cherry picked from commit ce3c77fb45ccf4d45a0fa655325e30e748d89245) --- source3/lib/util_sock.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source3/lib/util_sock.c b/source3/lib/util_sock.c index e38a918..6cef378 100644 --- a/source3/lib/util_sock.c +++ b/source3/lib/util_sock.c @@ -701,6 +701,7 @@ static void open_socket_out_connected(struct tevent_req *subreq) tevent_req_nterror(req, NT_STATUS_NO_MEMORY); return; } + state->connect_subreq = subreq; tevent_req_set_callback(subreq, open_socket_out_connected, req); return; } @@ -723,10 +724,12 @@ NTSTATUS open_socket_out_recv(struct tevent_req *req, int *pfd) NTSTATUS status; if (tevent_req_is_nterror(req, &status)) { + tevent_req_received(req); return status; } *pfd = state->fd; state->fd = -1; + tevent_req_received(req); return NT_STATUS_OK; } -- 1.9.1