Thanks Vincent!
It looks pretty good and very clean in the end.
Arthur, as soon as you confirm it works for you I'll merge it. I'm keeping
it untouched below in case you missed it.
Thanks,
Willy
On Wed, May 18, 2016 at 04:17:44PM +0200, Vincent Bernat wrote:
> From: Vincent Bernat <vinc...@bernat.im>
>
> When compiled with GCC 6, the IP address specified for a frontend was
> ignored and HAProxy was listening on all addresses instead. This is
> caused by an incomplete copy of a "struct sockaddr_storage".
>
> With the GNU Libc, "struct sockaddr_storage" is defined as this:
>
> struct sockaddr_storage
> {
> sa_family_t ss_family;
> unsigned long int __ss_align;
> char __ss_padding[(128 - (2 * sizeof (unsigned long int)))];
> };
>
> Doing an aggregate copy (ss1 = ss2) is different than using memcpy():
> only members of the aggregate have to be copied. Notably, padding can be
> or not be copied. In GCC 6, some optimizations use this fact and if a
> "struct sockaddr_storage" contains a "struct sockaddr_in", the port and
> the address are part of the padding (between sa_family and __ss_align)
> and can be not copied over.
>
> Therefore, we replace any aggregate copy by a memcpy(). There is another
> place using the same pattern. We also fix a function receiving a "struct
> sockaddr_storage" by copy instead of by reference. Since it only needs a
> read-only copy, the function is converted to request a reference.
> ---
> include/proto/proto_http.h | 2 +-
> src/cfgparse.c | 2 +-
> src/connection.c | 2 +-
> src/hlua.c | 2 +-
> src/proto_http.c | 12 ++++++------
> src/proto_tcp.c | 2 +-
> 6 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/include/proto/proto_http.h b/include/proto/proto_http.h
> index 4d8f5365b625..0aa6643b98da 100644
> --- a/include/proto/proto_http.h
> +++ b/include/proto/proto_http.h
> @@ -110,7 +110,7 @@ void http_set_status(unsigned int status, struct stream
> *s);
> int http_transform_header_str(struct stream* s, struct http_msg *msg, const
> char* name,
> unsigned int name_len, const char *str, struct
> my_regex *re,
> int action);
> -void inet_set_tos(int fd, struct sockaddr_storage from, int tos);
> +void inet_set_tos(int fd, const struct sockaddr_storage *from, int tos);
> void http_perform_server_redirect(struct stream *s, struct stream_interface
> *si);
> void http_return_srv_error(struct stream *s, struct stream_interface *si);
> void http_capture_bad_message(struct error_snapshot *es, struct stream *s,
> diff --git a/src/cfgparse.c b/src/cfgparse.c
> index 3fee54e0db1d..48e584cf73e7 100644
> --- a/src/cfgparse.c
> +++ b/src/cfgparse.c
> @@ -287,7 +287,7 @@ int str2listener(char *str, struct proxy *curproxy,
> struct bind_conf *bind_conf,
> }
>
> /* OK the address looks correct */
> - ss = *ss2;
> + memcpy(&ss, ss2, sizeof(ss));
>
> for (; port <= end; port++) {
> l = calloc(1, sizeof(*l));
> diff --git a/src/connection.c b/src/connection.c
> index 330f3efbc995..5515188c6b10 100644
> --- a/src/connection.c
> +++ b/src/connection.c
> @@ -744,7 +744,7 @@ int make_proxy_line_v2(char *buf, int buf_len, struct
> server *srv, struct connec
> const char pp2_signature[] = PP2_SIGNATURE;
> int ret = 0;
> struct proxy_hdr_v2 *hdr = (struct proxy_hdr_v2 *)buf;
> - struct sockaddr_storage null_addr = {0};
> + struct sockaddr_storage null_addr = { .ss_family = 0 };
> struct sockaddr_storage *src = &null_addr;
> struct sockaddr_storage *dst = &null_addr;
>
> diff --git a/src/hlua.c b/src/hlua.c
> index f6eb8aa80ee0..94f97429c895 100644
> --- a/src/hlua.c
> +++ b/src/hlua.c
> @@ -4781,7 +4781,7 @@ __LJMP static int hlua_txn_set_tos(lua_State *L)
> tos = MAY_LJMP(luaL_checkinteger(L, 2));
>
> if ((cli_conn = objt_conn(htxn->s->sess->origin)) &&
> conn_ctrl_ready(cli_conn))
> - inet_set_tos(cli_conn->t.sock.fd, cli_conn->addr.from, tos);
> + inet_set_tos(cli_conn->t.sock.fd, &cli_conn->addr.from, tos);
>
> return 0;
> }
> diff --git a/src/proto_http.c b/src/proto_http.c
> index 21ad131c9f43..416504247a8d 100644
> --- a/src/proto_http.c
> +++ b/src/proto_http.c
> @@ -3189,15 +3189,15 @@ int http_handle_stats(struct stream *s, struct
> channel *req)
> /* Sets the TOS header in IPv4 and the traffic class header in IPv6 packets
> * (as per RFC3260 #4 and BCP37 #4.2 and #5.2).
> */
> -void inet_set_tos(int fd, struct sockaddr_storage from, int tos)
> +void inet_set_tos(int fd, const struct sockaddr_storage *from, int tos)
> {
> #ifdef IP_TOS
> - if (from.ss_family == AF_INET)
> + if (from->ss_family == AF_INET)
> setsockopt(fd, IPPROTO_IP, IP_TOS, &tos, sizeof(tos));
> #endif
> #ifdef IPV6_TCLASS
> - if (from.ss_family == AF_INET6) {
> - if (IN6_IS_ADDR_V4MAPPED(&((struct sockaddr_in6
> *)&from)->sin6_addr))
> + if (from->ss_family == AF_INET6) {
> + if (IN6_IS_ADDR_V4MAPPED(&((struct sockaddr_in6
> *)from)->sin6_addr))
> /* v4-mapped addresses need IP_TOS */
> setsockopt(fd, IPPROTO_IP, IP_TOS, &tos, sizeof(tos));
> else
> @@ -3363,7 +3363,7 @@ resume_execution:
>
> case ACT_HTTP_SET_TOS:
> if ((cli_conn = objt_conn(sess->origin)) &&
> conn_ctrl_ready(cli_conn))
> - inet_set_tos(cli_conn->t.sock.fd,
> cli_conn->addr.from, rule->arg.tos);
> + inet_set_tos(cli_conn->t.sock.fd,
> &cli_conn->addr.from, rule->arg.tos);
> break;
>
> case ACT_HTTP_SET_MARK:
> @@ -3646,7 +3646,7 @@ resume_execution:
>
> case ACT_HTTP_SET_TOS:
> if ((cli_conn = objt_conn(sess->origin)) &&
> conn_ctrl_ready(cli_conn))
> - inet_set_tos(cli_conn->t.sock.fd,
> cli_conn->addr.from, rule->arg.tos);
> + inet_set_tos(cli_conn->t.sock.fd,
> &cli_conn->addr.from, rule->arg.tos);
> break;
>
> case ACT_HTTP_SET_MARK:
> diff --git a/src/proto_tcp.c b/src/proto_tcp.c
> index a44912af4654..bbe12e2d4c0d 100644
> --- a/src/proto_tcp.c
> +++ b/src/proto_tcp.c
> @@ -435,7 +435,7 @@ int tcp_connect_server(struct connection *conn, int data,
> int delack)
> struct sockaddr_storage sa;
>
> ret = 1;
> - sa = src->source_addr;
> + memcpy(&sa, &src->source_addr, sizeof(sa));
>
> do {
> /* note: in case of retry, we may have to
> release a previously
> --
> 2.8.1
>
