Oh my ! It's just happening to me on Fedora 24 with version haproxy-1.6.5-3.fc24.x86_64 !
Well, I have a huge problem since I just upgraded and the mess is all
around, listening on all ports and all IPv4 addresses.
Can you summarize what I should do ? Recompile with which options ?
Thanks !
Hoggins!
Le 18/05/2016 10:30, Willy Tarreau a écrit :
> Hi Vincent,
>
> On Wed, May 18, 2016 at 09:58:33AM +0200, Vincent Bernat wrote:
>> ??? 15 mai 2016 09:55 +0200, Vincent Bernat <ber...@luffy.cx> :
>>
>>>> I suppose that some new features of gcc started to rely on the
>>>> strict-aliasing rule without taking -fno-strict-aliasing into
>>>> consideration. I didn't find anything in the bugzilla, but it's easy to
>>>> miss something as there are about 500 bugs opened about aliasing.
>>>>
>>>> I'll open a bug about this.
>>> Here is the bug:
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71120
>>>
>>> With luck, we should get some pointers on why and how.
>> So, it appears this is not a bug in GCC and it is not an aliasing
>> problem but the fact that the compiler doesn't have to use memcpy() to
>> copy the struct, but can copy individual member, padding excluded.
> Yes, this is even a security problem which painfully affects the kernel.
>
>> And sin_addr would be in the padding, so not copied.
> That makes sense then.
>
>> A bug has been filed
>> for the glibc since the bug is in the definition:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=20111
> Great, thanks for the link and going through this. As was mentionned
> there, sockaddr_storage is expected by users and in literature to be
> able to carry other addresses.
>
>> We could either use an union like discussed previously or locally fix
>> this with a memcpy(). I suppose the union would be a longer term (and
>> safer since we can be exhaustive about it) fix but far more invasive.
> I have the same feeling, but it would make a few switch/case occurences
> cleaner or at least more explicit. For the short term and backports I
> guess only memcpy() will be usable, so we should probably start by this,
> backport it, then work on changing all this to a union.
>
>> I suspect there are other uses of sockaddr_storage that could lead to
>> some bugs.
> If there is one, there are many.
>
>> For example, connection.c:747. ss_family will be set to 0,
>> but eventual port and address can be random. However, it seems that in
>> the remaining of the code, only ss_family is important. But maybe, it
>> would be better to use memset() here.
> Good point.
>
> Maybe we would need to develop a few macros (or inline functions) to
> manipulate sockaddr_storage. We already have a few, I don't think we
> need that many. set_port, set_addr, set_family, copy_addr and I don't
> know what.
>
>> On the other hand, on proto_tcp.c:225, bind_addr is correctly zeroed out
>> with memset. Same on standard.c:829.
>>
>> I also note on proto_http.c:3192, a "struct sockaddr_storage from" that
>> may be more safe to replace by "struct sockaddr_storage *from" since it
>> is used read-only. But maybe the compiler is smart enough to not do a
>> copy and therefore the bug is not triggered here.
> I didn't notice this one, good catch. It should even be a const.
>
>> I didn't review all uses, only uses of "struct sockaddr_storage
>> something". I should check for "struct sockaddr_storage *something" and
>> eventual use of "*something =", but I suspect that it doesn't exist
>> without a "struct sockaddr_storage something_else" around.
> I think that once we get rid of *all* sockaddr_storage we'll find them
> all :-)
>
>> So, quick fix is a memcpy() in cfgparse.c and in proto_http.c:3192 and
>> using memset() in connection.c:747 for consistency (so that somebody
>> doesn't copy this code for something more general).
>>
>> Can do the patches but union would be more work.
> That works for me. Let's have Arthur test them to confirm the issue goes
> away for him.
>
> Thanks!
> Willy
>
>
signature.asc
Description: OpenPGP digital signature
