From 86a6566b77471dd26f7634660b08a6b1af38f6b8 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 8 Mar 2018 14:27:36 +0000 Subject: [PATCH] sysdeps-unix: Treat bind() failures as non-fatal When we use AF_UNSPEC, we are likely to get multiple addresses back from getaddrinfo(), and perhaps we won't be able to use them all. Give that failure mode, or any other bind() failure, the same treatment as EADDRINUSE failures here and all connect() failures in _dbus_connect_tcp_socket_with_nonce(): if any address succeeds, then the overall operation succeeds, but if all of them fail, then the overall operation fails. I've made combine_tcp_errors() generic enough that in principle _dbus_connect_tcp_socket_with_nonce() could use it too, although that isn't implemented yet. Signed-off-by: Simon McVittie https://bugs.freedesktop.org/show_bug.cgi?id=61922 --- dbus/dbus-sysdeps-unix.c | 79 +++++++++++++++++++++++++++++++++++------------- dbus/dbus-sysdeps.c | 68 +++++++++++++++++++++++++++++++++++++++++ dbus/dbus-sysdeps.h | 5 +++ 3 files changed, 131 insertions(+), 21 deletions(-) diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index 0ab034ff..4bcd5325 100644 --- a/dbus/dbus-sysdeps-unix.c +++ b/dbus/dbus-sysdeps-unix.c @@ -1473,6 +1473,8 @@ _dbus_listen_tcp_socket (const char *host, { int saved_errno; int nlisten_fd = 0, res, i; + DBusList *bind_errors = NULL; + DBusError *bind_error = NULL; DBusSocket *listen_fd = NULL; struct addrinfo hints; struct addrinfo *ai, *tmp; @@ -1545,28 +1547,50 @@ _dbus_listen_tcp_socket (const char *host, { saved_errno = errno; _dbus_close(fd, NULL); - if (saved_errno == EADDRINUSE) + + /* + * We don't treat this as a fatal error, because there might be + * other addresses that we can listen on. In particular: + * + * - If saved_errno is EADDRINUSE after we + * "goto redo_lookup_with_port" after binding a port on one of the + * possible addresses, we will try to bind that same port on + * every address, including the same address again for a second + * time, which will fail with EADDRINUSE. + * + * - If saved_errno is EADDRINUSE, it might be because binding to + * an IPv6 address implicitly binds to a corresponding IPv4 + * address or vice versa (e.g. Linux with bindv6only=0). + * + * - If saved_errno is EADDRNOTAVAIL when we asked for family + * AF_UNSPEC, it might be because IPv6 is disabled for this + * particular interface (e.g. Linux with + * /proc/sys/net/ipv6/conf/lo/disable_ipv6). + */ + bind_error = dbus_new0 (DBusError, 1); + + if (bind_error == NULL) { - /* Depending on kernel policy, binding to an IPv6 address - might implicitly bind to a corresponding IPv4 - address or vice versa, resulting in EADDRINUSE for the - other one (e.g. bindv6only=0 on Linux). - - Also, after we "goto redo_lookup_with_port" after binding - a port on one of the possible addresses, we will - try to bind that same port on every address, including the - same address again for a second time; that one will - also fail with EADDRINUSE. - - For both those reasons, ignore EADDRINUSE here */ - tmp = tmp->ai_next; - continue; + _DBUS_SET_OOM (error); + goto failed; } - _dbus_set_error_with_inet_sockaddr (error, tmp->ai_addr, tmp->ai_addrlen, + dbus_error_init (bind_error); + _dbus_set_error_with_inet_sockaddr (bind_error, tmp->ai_addr, tmp->ai_addrlen, "Failed to bind socket", saved_errno); - goto failed; + + if (!_dbus_list_append (&bind_errors, bind_error)) + { + dbus_error_free (bind_error); + dbus_free (bind_error); + _DBUS_SET_OOM (error); + goto failed; + } + + /* Try the next address, maybe it will work better */ + tmp = tmp->ai_next; + continue; } if (listen (fd, 30 /* backlog */) < 0) @@ -1645,10 +1669,8 @@ _dbus_listen_tcp_socket (const char *host, if (!nlisten_fd) { - errno = EADDRINUSE; - dbus_set_error (error, _dbus_error_from_errno (errno), - "Failed to bind socket \"%s:%s\": %s", - host ? host : "*", port, _dbus_strerror (errno)); + _dbus_combine_tcp_errors (&bind_errors, "Failed to bind", host, + port, error); goto failed; } @@ -1662,6 +1684,14 @@ _dbus_listen_tcp_socket (const char *host, *fds_p = listen_fd; + /* This list might be non-empty even on success, because we might be + * ignoring EADDRINUSE or EADDRNOTAVAIL */ + while ((bind_error = _dbus_list_pop_first (&bind_errors))) + { + dbus_error_free (bind_error); + dbus_free (bind_error); + } + return nlisten_fd; failed: @@ -1669,6 +1699,13 @@ _dbus_listen_tcp_socket (const char *host, freeaddrinfo(ai); for (i = 0 ; i < nlisten_fd ; i++) _dbus_close(listen_fd[i].fd, NULL); + + while ((bind_error = _dbus_list_pop_first (&bind_errors))) + { + dbus_error_free (bind_error); + dbus_free (bind_error); + } + dbus_free(listen_fd); return -1; } diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c index 43fa109f..98d5a925 100644 --- a/dbus/dbus-sysdeps.c +++ b/dbus/dbus-sysdeps.c @@ -908,6 +908,74 @@ _dbus_set_error_with_inet_sockaddr (DBusError *error, } } +void +_dbus_combine_tcp_errors (DBusList **sources, + const char *summary, + const char *host, + const char *port, + DBusError *dest) +{ + DBusString message = _DBUS_STRING_INIT_INVALID; + + if (_dbus_list_length_is_one (sources)) + { + /* If there was exactly one error, just use it */ + dbus_move_error (_dbus_list_get_first (sources), dest); + } + else + { + DBusList *iter; + const char *name; + + /* If there was more than one error, concatenate all the + * errors' diagnostic messages, and use their common error + * name, or DBUS_ERROR_FAILED if more than one name is + * represented */ + if (!_dbus_string_init (&message)) + { + _DBUS_SET_OOM (dest); + goto out; + } + + for (iter = _dbus_list_get_first_link (sources); + iter != NULL; + iter = _dbus_list_get_next_link (sources, iter)) + { + DBusError *error = iter->data; + + if (name == NULL) + { + /* no error names known yet, try to use this one */ + name = error->name; + } + else if (strcmp (name, error->name) != 0) + { + /* errors of two different names exist, reconcile by + * using FAILED */ + name = DBUS_ERROR_FAILED; + } + + if ((_dbus_string_get_length (&message) > 0 && + !_dbus_string_append (&message, "; ")) || + !_dbus_string_append (&message, error->message)) + { + _DBUS_SET_OOM (dest); + goto out; + } + } + + if (name == NULL) + name = DBUS_ERROR_FAILED; + + dbus_set_error (dest, name, "%s to \"%s\":%s (%s)", + summary, host ? host : "*", port, + _dbus_string_get_const_data (&message)); + } + +out: + _dbus_string_free (&message); +} + /** @} end of sysdeps */ /* tests in dbus-sysdeps-util.c */ diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index 376f6802..c9d630a7 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -706,6 +706,11 @@ void _dbus_set_error_with_inet_sockaddr (DBusError *error, size_t len, const char *description, int saved_errno); +void _dbus_combine_tcp_errors (DBusList **sources, + const char *summary, + const char *host, + const char *port, + DBusError *dest); /** @} */ -- 2.12.3