BUG/MINOR: server/event_hdl: properly handle AF_UNSPEC for INETADDR event

It is possible that a server's addr family is temporarily set to AF_UNSPEC
even if we're certain to be in INET context (ipv4, ipv6).

Indeed, as soon as IP address resolving is involved, srv->addr family will
be set to AF_UNSPEC when the resolution fails (could happen at anytime).

However, _srv_event_hdl_prepare_inetaddr() wrongly assumed that it would
only be called with AF_INET or AF_INET6 families. Because of that, the
function will handle AF_UNSPEC address as an IPV6 address: not only
we could risk reading from an unititialized area, but we would then
propagate false information when publishing the event.

In this patch we make sure to properly handle the AF_UNSPEC family in
both the "prev" and the "next" part for SERVER_INETADDR event and that
every members are explicitly initialized.

This bug was introduced by 6fde37e046 ("MINOR: server/event_hdl: add
SERVER_INETADDR event"), no backport needed.

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 3ff98cd1e40fee5f46cbbc970cec09c974cd4409..1708bc62055484a004c5fc7dc30262141dce408c 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -610,7 +610,7 @@ struct event_hdl_cb_data_server_inetaddr {
        struct event_hdl_cb_data_server server;                 /* must be at the beginning */
        struct {
                struct  {
-                       int family; /* AF_INET or AF_INET6 */
+                       int family; /* AF_UNSPEC, AF_INET or AF_INET6 */
                        union {
                                struct in_addr v4;
                                struct in6_addr v6;
@@ -618,7 +618,7 @@ struct event_hdl_cb_data_server_inetaddr {
                        unsigned int svc_port;
                } prev;
                struct {
-                       int family; /* AF_INET or AF_INET6 */
+                       int family; /* AF_UNSPEC, AF_INET or AF_INET6 */
                        union {
                                struct in_addr v4;
                                struct in6_addr v6;

diff --git a/src/server.c b/src/server.c
index 5f79c7db6c170bb2beddd696272e401b8c657701..0824ae695629f1f871ccfc40d3fe7ae4195cea01 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -270,6 +270,9 @@ static struct task *server_atomic_sync(struct task *task, void *context, unsigne
                                               &data->safe.next.addr.v6,
                                               sizeof(struct in6_addr));
                                        break;
+                               case AF_UNSPEC:
+                                       /* addr reset, nothing to do */
+                                       break;
                                default:
                                        /* should not happen */
                                        break;
@@ -426,32 +429,27 @@ void _srv_event_hdl_prepare_inetaddr(struct event_hdl_cb_data_server_inetaddr *c
                next_addr->ss_family != AF_INET && next_addr->ss_family != AF_INET6));
 
        /* prev */
-       if (prev_addr->ss_family == AF_INET) {
-               cb_data->safe.prev.family = AF_INET;
+       cb_data->safe.prev.family = prev_addr->ss_family;
+       memset(&cb_data->safe.prev.addr, 0, sizeof(cb_data->safe.prev.addr));
+       if (prev_addr->ss_family == AF_INET)
                cb_data->safe.prev.addr.v4.s_addr =
                        ((struct sockaddr_in *)prev_addr)->sin_addr.s_addr;
-       }
-       else {
-               cb_data->safe.prev.family = AF_INET6;
+       else if (prev_addr->ss_family == AF_INET6)
                memcpy(&cb_data->safe.prev.addr.v6,
                       &((struct sockaddr_in6 *)prev_addr)->sin6_addr,
                       sizeof(struct in6_addr));
-       }
        cb_data->safe.prev.svc_port = prev_port;
 
        /* next */
-       if (next_addr->ss_family == AF_INET) {
-               cb_data->safe.next.family = AF_INET;
+       cb_data->safe.next.family = next_addr->ss_family;
+       memset(&cb_data->safe.next.addr, 0, sizeof(cb_data->safe.next.addr));
+       if (next_addr->ss_family == AF_INET)
                cb_data->safe.next.addr.v4.s_addr =
                        ((struct sockaddr_in *)next_addr)->sin_addr.s_addr;
-       }
-       else {
-               cb_data->safe.next.family = AF_INET6;
+       else if (next_addr->ss_family == AF_INET6)
                memcpy(&cb_data->safe.next.addr.v6,
                       &((struct sockaddr_in6 *)next_addr)->sin6_addr,
                       sizeof(struct in6_addr));
-       }
-
        cb_data->safe.next.svc_port = next_port;
 
        cb_data->safe.purge_conn = purge_conn;
@@ -3695,7 +3693,7 @@ int srv_update_addr(struct server *s, void *ip, int ip_sin_family, const char *u
                struct event_hdl_cb_data_server_inetaddr addr;
                struct event_hdl_cb_data_server common;
        } cb_data;
-       struct sockaddr_storage new_addr;
+       struct sockaddr_storage new_addr = { }; // shut up gcc warning
 
        /* save the new IP family & address if necessary */
        switch (ip_sin_family) {