From 64cc9bbc40faaec9cfc541377c331ce34de9ae17 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 21 Mar 2017 17:31:53 +0100 Subject: [PATCH] gsocketlistener: Fix IPv4 listen() error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In add_any_inet_port() for the case where we successfully listen()ed on IPv6, but failed to listen on IPv4, we would erroneously unref the IPv6 socket which we just gave away ownership of by adding it to priv->sockets. However, given that we have a successful IPv6 socket, we shouldn’t report it as an error if the IPv4 listen() fails. While this code tries quite hard to return both sockets, returning only one seems to be the best we can do in this situation. This issue was observed in an Android 4.4 ARM emulator. It’s possible the emulator deferred the bind() on the host until the listen() on the client. Based on a patch by Ole André Vadla Ravnås . https://gitlab.gnome.org/GNOME/glib/issues/1250 --- gio/gsocketlistener.c | 167 ++++++++++++++++++++++++++---------------- 1 file changed, 103 insertions(+), 64 deletions(-) diff --git a/gio/gsocketlistener.c b/gio/gsocketlistener.c index 8a296a5b5..4c63f009f 100644 --- a/gio/gsocketlistener.c +++ b/gio/gsocketlistener.c @@ -442,6 +442,8 @@ g_socket_listener_add_inet_port (GSocketListener *listener, gboolean need_ipv4_socket = TRUE; GSocket *socket4 = NULL; GSocket *socket6; + GError *socket4_create_error = NULL; + GError *socket4_listen_error = NULL, *socket6_listen_error = NULL; g_return_val_if_fail (listener != NULL, FALSE); g_return_val_if_fail (port != 0, FALSE); @@ -487,23 +489,20 @@ g_socket_listener_add_inet_port (GSocketListener *listener, g_signal_emit (listener, signals[EVENT], 0, G_SOCKET_LISTENER_LISTENING, socket6); - if (!g_socket_listen (socket6, error)) + if (g_socket_listen (socket6, &socket6_listen_error)) { - g_object_unref (socket6); - return FALSE; + g_signal_emit (listener, signals[EVENT], 0, + G_SOCKET_LISTENER_LISTENED, socket6); + + if (source_object) + g_object_set_qdata_full (G_OBJECT (socket6), source_quark, + g_object_ref (source_object), + g_object_unref); + + /* If this socket already speaks IPv4 then we are done. */ + if (g_socket_speaks_ipv4 (socket6)) + need_ipv4_socket = FALSE; } - - g_signal_emit (listener, signals[EVENT], 0, - G_SOCKET_LISTENER_LISTENED, socket6); - - if (source_object) - g_object_set_qdata_full (G_OBJECT (socket6), source_quark, - g_object_ref (source_object), - g_object_unref); - - /* If this socket already speaks IPv4 then we are done. */ - if (g_socket_speaks_ipv4 (socket6)) - need_ipv4_socket = FALSE; } if (need_ipv4_socket) @@ -519,7 +518,7 @@ g_socket_listener_add_inet_port (GSocketListener *listener, socket4 = g_socket_new (G_SOCKET_FAMILY_IPV4, G_SOCKET_TYPE_STREAM, G_SOCKET_PROTOCOL_DEFAULT, - error); + &socket4_create_error); if (socket4 != NULL) /* IPv4 is supported on this platform, so if we fail now it is @@ -546,6 +545,7 @@ g_socket_listener_add_inet_port (GSocketListener *listener, g_object_unref (socket4); if (socket6 != NULL) g_object_unref (socket6); + g_clear_error (&socket6_listen_error); return FALSE; } @@ -557,22 +557,16 @@ g_socket_listener_add_inet_port (GSocketListener *listener, g_signal_emit (listener, signals[EVENT], 0, G_SOCKET_LISTENER_LISTENING, socket4); - if (!g_socket_listen (socket4, error)) + if (g_socket_listen (socket4, &socket4_listen_error)) { - g_object_unref (socket4); - if (socket6 != NULL) - g_object_unref (socket6); + g_signal_emit (listener, signals[EVENT], 0, + G_SOCKET_LISTENER_LISTENED, socket4); - return FALSE; + if (source_object) + g_object_set_qdata_full (G_OBJECT (socket4), source_quark, + g_object_ref (source_object), + g_object_unref); } - - g_signal_emit (listener, signals[EVENT], 0, - G_SOCKET_LISTENER_LISTENED, socket4); - - if (source_object) - g_object_set_qdata_full (G_OBJECT (socket4), source_quark, - g_object_ref (source_object), - g_object_unref); } else /* Ok. So IPv4 is not supported on this platform. If we @@ -580,13 +574,34 @@ g_socket_listener_add_inet_port (GSocketListener *listener, * otherwise we need to tell the user we failed. */ { - if (socket6 != NULL) - g_clear_error (error); - else - return FALSE; + if (socket6 == NULL || socket6_listen_error != NULL) + { + g_clear_object (&socket6); + g_clear_error (&socket6_listen_error); + g_propagate_error (error, g_steal_pointer (&socket4_create_error)); + return FALSE; + } } } + /* Only error out if both listen() calls failed. */ + if ((socket6 == NULL || socket6_listen_error != NULL) && + (socket4 == NULL || socket4_listen_error != NULL)) + { + GError **set_error = ((socket6_listen_error != NULL) ? + &socket6_listen_error : + &socket4_listen_error); + g_propagate_error (error, g_steal_pointer (set_error)); + + g_clear_error (&socket6_listen_error); + g_clear_error (&socket4_listen_error); + + g_clear_object (&socket6); + g_clear_object (&socket4); + + return FALSE; + } + g_assert (socket6 != NULL || socket4 != NULL); if (socket6 != NULL) @@ -598,6 +613,9 @@ g_socket_listener_add_inet_port (GSocketListener *listener, if (G_SOCKET_LISTENER_GET_CLASS (listener)->changed) G_SOCKET_LISTENER_GET_CLASS (listener)->changed (listener); + g_clear_error (&socket6_listen_error); + g_clear_error (&socket4_listen_error); + return TRUE; } @@ -1061,6 +1079,7 @@ g_socket_listener_add_any_inet_port (GSocketListener *listener, GSocket *socket6 = NULL; GSocket *socket4 = NULL; gint attempts = 37; + GError *socket4_listen_error = NULL, *socket6_listen_error = NULL; /* * multi-step process: @@ -1223,7 +1242,9 @@ g_socket_listener_add_any_inet_port (GSocketListener *listener, sockets_to_close); } - /* now we actually listen() the sockets and add them to the listener */ + /* now we actually listen() the sockets and add them to the listener. If + * either of the listen()s fails, only return the other socket. Fail if both + * failed. */ if (socket6 != NULL) { g_socket_set_listen_backlog (socket6, listener->priv->listen_backlog); @@ -1231,24 +1252,22 @@ g_socket_listener_add_any_inet_port (GSocketListener *listener, g_signal_emit (listener, signals[EVENT], 0, G_SOCKET_LISTENER_LISTENING, socket6); - if (!g_socket_listen (socket6, error)) + if (g_socket_listen (socket6, &socket6_listen_error)) { - g_object_unref (socket6); - if (socket4) - g_object_unref (socket4); + g_signal_emit (listener, signals[EVENT], 0, + G_SOCKET_LISTENER_LISTENED, socket6); - return 0; + if (source_object) + g_object_set_qdata_full (G_OBJECT (socket6), source_quark, + g_object_ref (source_object), + g_object_unref); + + g_ptr_array_add (listener->priv->sockets, socket6); + } + else + { + g_clear_object (&socket6); } - - g_signal_emit (listener, signals[EVENT], 0, - G_SOCKET_LISTENER_LISTENED, socket6); - - if (source_object) - g_object_set_qdata_full (G_OBJECT (socket6), source_quark, - g_object_ref (source_object), - g_object_unref); - - g_ptr_array_add (listener->priv->sockets, socket6); } if (socket4 != NULL) @@ -1258,26 +1277,46 @@ g_socket_listener_add_any_inet_port (GSocketListener *listener, g_signal_emit (listener, signals[EVENT], 0, G_SOCKET_LISTENER_LISTENING, socket4); - if (!g_socket_listen (socket4, error)) + if (g_socket_listen (socket4, &socket4_listen_error)) { - g_object_unref (socket4); - if (socket6) - g_object_unref (socket6); + g_signal_emit (listener, signals[EVENT], 0, + G_SOCKET_LISTENER_LISTENED, socket4); - return 0; + if (source_object) + g_object_set_qdata_full (G_OBJECT (socket4), source_quark, + g_object_ref (source_object), + g_object_unref); + + g_ptr_array_add (listener->priv->sockets, socket4); + } + else + { + g_clear_object (&socket4); } - - g_signal_emit (listener, signals[EVENT], 0, - G_SOCKET_LISTENER_LISTENED, socket4); - - if (source_object) - g_object_set_qdata_full (G_OBJECT (socket4), source_quark, - g_object_ref (source_object), - g_object_unref); - - g_ptr_array_add (listener->priv->sockets, socket4); } + /* Error out if both listen() calls failed (or if there’s no separate IPv4 + * socket and the IPv6 listen() call failed). */ + if (socket6_listen_error != NULL && + (socket4 == NULL || socket4_listen_error != NULL)) + { + GError **set_error = ((socket6_listen_error != NULL) ? + &socket6_listen_error : + &socket4_listen_error); + g_propagate_error (error, g_steal_pointer (set_error)); + + g_clear_error (&socket6_listen_error); + g_clear_error (&socket4_listen_error); + + g_clear_object (&socket6); + g_clear_object (&socket4); + + return 0; + } + + g_clear_error (&socket6_listen_error); + g_clear_error (&socket4_listen_error); + if ((socket4 != NULL || socket6 != NULL) && G_SOCKET_LISTENER_GET_CLASS (listener)->changed) G_SOCKET_LISTENER_GET_CLASS (listener)->changed (listener);