Hi Roberto,
On Sun, Jun 12, 2016 at 01:13:16PM +0200, Willy Tarreau wrote:
> On Sat, Jun 11, 2016 at 03:58:10PM -0700, Roberto Guimaraes wrote:
> > Valgrind reports that the memory allocated in ssl_get_dh_1024() was
> > leaking. Upon further inspection of openssl code, it seems that
> > SSL_CTX_set_tmp_dh makes a copy of the data, so calling DH_free afterwards
> > makes sense.
> >
> > thanks,
> > roberto
>
> Patch applied, thank you Roberto!
I had to revert your patch. We found that on some systems using
openssl 1.0.2, certain very trivial configs segfault on startup and
even with a simple check (haproxy -c). The conf in question only
contains this :
global
tune.ssl.default-dh-param 1024
frontend test
bind 0.0.0.0:7443 ssl crt test.pem
mode http
redirect location /
Interesting point to note, on a machine it passes -c but crashes on
startup and on another one with a slightly different config it crashes
on both. Reverting the patch is enough to fix this. I strongly suspect
a use-after-free.
I checked the man page for SSL_CTX_set_tmp_dh() and it does not mention
anything regarding the need to free the param or not. And the example
that comes with it doesn't involve a call to DH_free().
Thus for now I'd rather leave valgrind unhappy until someone finds what
exactly needs to be done to make it happy and not cause this issue.
I'll have to revert it from 1.6 as well (too bad we released 1.6.6 with
it), and 1.5 (where I backported it today).
Regards,
Willy
