Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Willy Tarreau]

On Thu, May 19, 2016 at 11:15:51AM +0200, Vincent Bernat wrote:
(...)
> This is a backport for 1.5 of 3baab74e32ceec987e7ff3db1627b760bbac3027.

Thank you Vincent. I merged the BUG/MINOR patch into 1.7 and 1.6, merged
this one into 1.5 after fixing the commit ID above and adding a reference
to the new one (since it's included).

Cheers,
Willy

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Vincent Bernat]

 ❦ 19 mai 2016 11:23 +0200, Cyril Bonté  :

>> De: "Vincent Bernat" 
>>  for (; port <= end; port++) {
>>  l = (struct listener *)calloc(1, sizeof(struct 
>> listener));
>> @@ -291,7 +291,7 @@ int str2listener(char *str, struct proxy
>> *curproxy, struct bind_conf *bind_conf,
>>  l->bind_conf = bind_conf;
>>  
>>  l->fd = fd;
>> -l->addr = ss;
>> +memcpy(>addr, , sizeof(ss));
>>  l->xprt = _sock;
>>  l->state = LI_INIT;
>
> This one was not present in the patch for haproxy 1.6/1.7 ! Maybe it
> should ?

Yes, I just noticed it too. Sending a patch.
-- 
Make sure special cases are truly special.
- The Elements of Programming Style (Kernighan & Plauger)

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Vincent Bernat]

 ❦ 19 mai 2016 11:15 +0200, Vincent Bernat  :

> This is a backport for 1.5 of
> 3baab74e32ceec987e7ff3db1627b760bbac3027.

Wrong commit number. This should be 6e6158.
-- 
Keep it simple to make it faster.
- The Elements of Programming Style (Kernighan & Plauger)

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Cyril Bonté]

Hi Vincent,

> De: "Vincent Bernat" 
>   for (; port <= end; port++) {
>   l = (struct listener *)calloc(1, sizeof(struct 
> listener));
> @@ -291,7 +291,7 @@ int str2listener(char *str, struct proxy
> *curproxy, struct bind_conf *bind_conf,
>   l->bind_conf = bind_conf;
>  
>   l->fd = fd;
> - l->addr = ss;
> + memcpy(>addr, , sizeof(ss));
>   l->xprt = _sock;
>   l->state = LI_INIT;

This one was not present in the patch for haproxy 1.6/1.7 ! Maybe it should ?

Cheers,
Cyril

[PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Vincent Bernat]

From: Vincent Bernat 

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.

This is a backport for 1.5 of 3baab74e32ceec987e7ff3db1627b760bbac3027.
---
 src/cfgparse.c   |  4 ++--
 src/connection.c |  2 +-
 src/proto_http.c | 12 ++--
 src/proto_tcp.c  |  2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 39abf6b40429..9331415e12b3 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -280,7 +280,7 @@ int str2listener(char *str, struct proxy *curproxy, struct 
bind_conf *bind_conf,
}
 
/* OK the address looks correct */
-   ss = *ss2;
+   memcpy(, ss2, sizeof(ss));
 
for (; port <= end; port++) {
l = (struct listener *)calloc(1, sizeof(struct 
listener));
@@ -291,7 +291,7 @@ int str2listener(char *str, struct proxy *curproxy, struct 
bind_conf *bind_conf,
l->bind_conf = bind_conf;
 
l->fd = fd;
-   l->addr = ss;
+   memcpy(>addr, , sizeof(ss));
l->xprt = _sock;
l->state = LI_INIT;
 
diff --git a/src/connection.c b/src/connection.c
index dab1c9076166..09fd04e5286a 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -620,7 +620,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 = _addr;
struct sockaddr_storage *dst = _addr;
 #ifdef USE_OPENSSL
diff --git a/src/proto_http.c b/src/proto_http.c
index b3aa4d83308f..0b13c5ebc2cf 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -3220,15 +3220,15 @@ int http_handle_stats(struct session *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).
  */
-static inline void inet_set_tos(int fd, struct sockaddr_storage from, int tos)
+static inline void inet_set_tos(int fd, 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, , sizeof(tos));
 #endif
 #ifdef IPV6_TCLASS
-   if (from.ss_family == AF_INET6) {
-   if (IN6_IS_ADDR_V4MAPPED(&((struct sockaddr_in6 
*))->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, , sizeof(tos));
else
@@ -3366,7 +3366,7 @@ http_req_get_intercept_rule(struct proxy *px, struct list 
*rules, struct session
 
case HTTP_REQ_ACT_SET_TOS:
if ((cli_conn = objt_conn(s->req->prod->end)) && 
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, 
_conn->addr.from, rule->arg.tos);
break;
 
case HTTP_REQ_ACT_SET_MARK:
@@ -3563,7 +3563,7 @@ http_res_get_intercept_rule(struct proxy *px, struct list 
*rules, struct session
 
case HTTP_RES_ACT_SET_TOS:
if ((cli_conn = objt_conn(s->req->prod->end)) && 
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, 
_conn->addr.from, rule->arg.tos);
   

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Willy Tarreau]

On Thu, May 19, 2016 at 11:10:09AM +0200, Vincent Bernat wrote:
>  ??? 19 mai 2016 10:54 +0200, Willy Tarreau  :
> 
> >> >> 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".
> >> >
> >> > Patch applied to both 1.7-dev and 1.6. I haven't checked yet if/what 
> >> > needs
> >> > to be adjusted for 1.5.
> >> 
> >> Do you want me to do that?
> >
> > As you like. I can't hide the fact that it will significantly help me
> > considering you've already done the job for 1.6/1.7. But if you don't
> > have time I'll do it as part of my next backport session.
> 
> I'll do it. Should I keep BUG/MAJOR? HAProxy 1.5 is far less likely to
> be compiled with GCC 6.

I think it's fine to keep the same tag given the impact. Most MAJOR bugs
are very unlikely to hit but when they hit you'd rather not be there :-)

Thanks Vincent!
willy

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Vincent Bernat]

 ❦ 19 mai 2016 10:54 +0200, Willy Tarreau  :

>> >> 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".
>> >
>> > Patch applied to both 1.7-dev and 1.6. I haven't checked yet if/what needs
>> > to be adjusted for 1.5.
>> 
>> Do you want me to do that?
>
> As you like. I can't hide the fact that it will significantly help me
> considering you've already done the job for 1.6/1.7. But if you don't
> have time I'll do it as part of my next backport session.

I'll do it. Should I keep BUG/MAJOR? HAProxy 1.5 is far less likely to
be compiled with GCC 6.
-- 
Don't comment bad code - rewrite it.
- The Elements of Programming Style (Kernighan & Plauger)

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Willy Tarreau]

On Thu, May 19, 2016 at 10:49:46AM +0200, Vincent Bernat wrote:
>  ??? 19 mai 2016 10:46 +0200, Willy Tarreau  :
> 
> >> 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".
> >
> > Patch applied to both 1.7-dev and 1.6. I haven't checked yet if/what needs
> > to be adjusted for 1.5.
> 
> Do you want me to do that?

As you like. I can't hide the fact that it will significantly help me
considering you've already done the job for 1.6/1.7. But if you don't
have time I'll do it as part of my next backport session.

Thanks,
Willy

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Vincent Bernat]

 ❦ 19 mai 2016 10:46 +0200, Willy Tarreau  :

>> 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".
>
> Patch applied to both 1.7-dev and 1.6. I haven't checked yet if/what needs
> to be adjusted for 1.5.

Do you want me to do that?
-- 
Use debugging compilers.
- The Elements of Programming Style (Kernighan & Plauger)

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Willy Tarreau]

On Wed, May 18, 2016 at 04:17:44PM +0200, Vincent Bernat wrote:
> 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".
(...)

Patch applied to both 1.7-dev and 1.6. I haven't checked yet if/what needs
to be adjusted for 1.5.

thanks!
Willy

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Willy Tarreau]

Hi Arthur,

On Thu, May 19, 2016 at 11:06:06AM +0300, Arthur ??i??eic?? wrote:
> I tried again this morning with fresh eyes so this time a was able to 
> discover 
> the horror of web interfaces [1] that make "helpful" transformations like 
>  
> for @ together with random spaces around.

Wow I didn't notice this, that's indeed horribly crippled!

> And of course the patch utility will happily just say "patching file include/
> proto/proto_http.h" when in fact the patch is flawed and it's not changing 
> anything.

Of course since it found no hunk in it.

> Finally being aware and able to overcome these unexpected obstacles I can 
> indeed confirm that the issue seems fixed with haproxy 1.6.5 and gcc 6. 
> Haproxy 
> listens only on configured IPv4 addresses.

Thanks! I'm applying it now.

Willy

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Arthur Țițeică]

Hi,

În ziua de miercuri, 18 mai 2016, la 22:58:06 EEST, Cyril Bonté a scris:
> Le 18/05/2016 22:52, Arthur Țițeică a écrit :
> > În ziua de miercuri, 18 mai 2016, la 22:38:45 EEST, Cyril Bonté a scris:
> >> It looks like you didn't recompile with USE_OPENSSL=1
> >> haproxy -vv should give some hints.
> > 
> > Indeed, sorry about that. And error on my build script.
> > 
> > I fixed that, haproxy starts successfully but the issue remains, it still
> > listens on the wildcard IPv4 addresses.
> 
> Are you sure to run the binary with the patch applied ?
> 
>  From my quick test, Vincent's patch is working well with gcc 6 (and
> that's already what I saw during my diagnostics about memcpy).


I tried again this morning with fresh eyes so this time a was able to discover 
the horror of web interfaces [1] that make "helpful" transformations like  
for @ together with random spaces around.

And of course the patch utility will happily just say "patching file include/
proto/proto_http.h" when in fact the patch is flawed and it's not changing 
anything.


Finally being aware and able to overcome these unexpected obstacles I can 
indeed confirm that the issue seems fixed with haproxy 1.6.5 and gcc 6. Haproxy 
listens only on configured IPv4 addresses.

Thank you all

[1] http://article.gmane.org/gmane.comp.web.haproxy/27986

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Cyril Bonté]

Le 18/05/2016 22:52, Arthur Țițeică a écrit :

În ziua de miercuri, 18 mai 2016, la 22:38:45 EEST, Cyril Bonté a scris:

It looks like you didn't recompile with USE_OPENSSL=1
haproxy -vv should give some hints.


Indeed, sorry about that. And error on my build script.

I fixed that, haproxy starts successfully but the issue remains, it still
listens on the wildcard IPv4 addresses.


Are you sure to run the binary with the patch applied ?

From my quick test, Vincent's patch is working well with gcc 6 (and 
that's already what I saw during my diagnostics about memcpy).




--
Cyril Bonté

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Arthur Țițeică]

În ziua de miercuri, 18 mai 2016, la 22:38:45 EEST, Cyril Bonté a scris:
> It looks like you didn't recompile with USE_OPENSSL=1
> haproxy -vv should give some hints.

Indeed, sorry about that. And error on my build script.

I fixed that, haproxy starts successfully but the issue remains, it still 
listens on the wildcard IPv4 addresses.

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Cyril Bonté]

Hi,

Le 18/05/2016 22:36, Arthur Țițeică a écrit :

Hi all,

În ziua de miercuri, 18 mai 2016, la 20:51:13 EEST, Willy Tarreau a scris:

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.


Something seems a bit off now.

This is what happens when I manually start haproxy with the patch applied

# /usr/bin/haproxy -f /etc/haproxy/haproxy.cfg -p /run/haproxy.pid -Ds
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:25] : unknown
keyword 'tune.ssl.default-dh-param' in 'global' section
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:26] : 'ssl-
default-bind-ciphers' is not implemented.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:27] : unknown
keyword 'ssl-default-bind-options' in 'global' section
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:28] : 'ssl-
default-server-ciphers' is not implemented.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:29] : unknown
keyword 'ssl-default-server-options' in 'global' section
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:31] : 'crt-
base' is not implemented.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:78] : 'bind
163.172.36.33:443' unknown keyword 'ssl'. Registered keywords :
 [ ALL] accept-proxy
 [ ALL] backlog 
 [ ALL] id 
 [ ALL] maxconn 
 [ ALL] name 
 [ ALL] nice 
 [ ALL] process 
 [UNIX] gid 
 [UNIX] group 
 [UNIX] mode 
 [UNIX] uid 
 [UNIX] user 
 [STAT] level 
 [ TCP] defer-accept
 [ TCP] interface 
 [ TCP] mss 
 [ TCP] tcp-ut 
 [ TCP] tfo
 [ TCP] transparent
 [ TCP] v4v6
 [ TCP] v6only
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:79] : 'bind
2001:bc8:2377:200::1:443' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:80] : 'bind
163.172.36.33:59091' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:81] : 'bind
2001:bc8:2377:200::1:59091' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:82] : 'bind
163.172.36.33:59092' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:83] : 'bind
2001:bc8:2377:200::1:59092' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:84] : 'bind
163.172.36.33:8099' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:85] : 'bind
2001:bc8:2377:200::1:8099' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:142] : error
detected while parsing an 'http-request auth' condition : unknown fetch method
'ssl_fc' in ACL expression 'ssl_fc'.
[ALERT] 138/232913 (14342) : Error(s) found in configuration file : /etc/
haproxy/haproxy.cfg

Needless to say that this is a valid config that works with 1.6.4.


It looks like you didn't recompile with USE_OPENSSL=1
haproxy -vv should give some hints.

--
Cyril Bonté

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Arthur Țițeică]

Hi all,

În ziua de miercuri, 18 mai 2016, la 20:51:13 EEST, Willy Tarreau a scris:
> 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.

Something seems a bit off now.

This is what happens when I manually start haproxy with the patch applied

# /usr/bin/haproxy -f /etc/haproxy/haproxy.cfg -p /run/haproxy.pid -Ds
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:25] : unknown 
keyword 'tune.ssl.default-dh-param' in 'global' section
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:26] : 'ssl-
default-bind-ciphers' is not implemented.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:27] : unknown 
keyword 'ssl-default-bind-options' in 'global' section
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:28] : 'ssl-
default-server-ciphers' is not implemented.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:29] : unknown 
keyword 'ssl-default-server-options' in 'global' section
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:31] : 'crt-
base' is not implemented.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:78] : 'bind 
163.172.36.33:443' unknown keyword 'ssl'. Registered keywords :
[ ALL] accept-proxy
[ ALL] backlog 
[ ALL] id 
[ ALL] maxconn 
[ ALL] name 
[ ALL] nice 
[ ALL] process 
[UNIX] gid 
[UNIX] group 
[UNIX] mode 
[UNIX] uid 
[UNIX] user 
[STAT] level 
[ TCP] defer-accept
[ TCP] interface 
[ TCP] mss 
[ TCP] tcp-ut 
[ TCP] tfo
[ TCP] transparent
[ TCP] v4v6
[ TCP] v6only
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:79] : 'bind 
2001:bc8:2377:200::1:443' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:80] : 'bind 
163.172.36.33:59091' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:81] : 'bind 
2001:bc8:2377:200::1:59091' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:82] : 'bind 
163.172.36.33:59092' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:83] : 'bind 
2001:bc8:2377:200::1:59092' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:84] : 'bind 
163.172.36.33:8099' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:85] : 'bind 
2001:bc8:2377:200::1:8099' unknown keyword 'ssl'.
[ALERT] 138/232913 (14342) : parsing [/etc/haproxy/haproxy.cfg:142] : error 
detected while parsing an 'http-request auth' condition : unknown fetch method 
'ssl_fc' in ACL expression 'ssl_fc'.
[ALERT] 138/232913 (14342) : Error(s) found in configuration file : /etc/
haproxy/haproxy.cfg

Needless to say that this is a valid config that works with 1.6.4.

Thanks

Re: [PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Willy Tarreau]

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 
> 
> 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(, 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 = _addr;
>   struct sockaddr_storage *dst = _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, _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, , sizeof(tos));
>  #endif
>  #ifdef IPV6_TCLASS
> - if 

[PATCH] BUG/MAJOR: fix listening IP address storage for frontends

[Vincent Bernat]

From: Vincent Bernat 

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(, 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 = _addr;
struct sockaddr_storage *dst = _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, _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, , sizeof(tos));
 #endif
 #ifdef IPV6_TCLASS
-   if (from.ss_family == AF_INET6) {
-   if (IN6_IS_ADDR_V4MAPPED(&((struct sockaddr_in6 
*))->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, , sizeof(tos));
else
@@ -3363,7 +3363,7 @@ resume_execution: