On Tue, 17 Jul 2018 10:10:58 +0200
Willy Tarreau <[email protected]> wrote:
> Hi again Nenad,
>
> On Tue, Jul 17, 2018 at 05:18:45AM +0200, Willy Tarreau wrote:
> > Hi Nenad,
> >
> > On Tue, Jul 17, 2018 at 03:37:37AM +0200, Nenad Merdanovic wrote:
> > > Ugh, this was a long time ago. [FROM MEMORY] The element should not be
> > > duplicated as far as I can remember. The references are stored in an
> > > ebtree
> > > in order to prevent duplication and to provide consistent view when
> > > updated
> > > dynamically.
> >
> > OK. Then maybe the elements are freed after scanning the ebtree as well,
> > and we're meeting the node again after it was freed. I'll run a test
> > with the memory debugging options to see if it crashes on first dereference.
>
> OK I found it, the same keys_ref is indeed assigned to multiple bind_confs :
>
> static int bind_parse_tls_ticket_keys(char **args, int cur_arg, struct proxy
> *px, struct bind_conf *conf, char **err)
> {
> ...
> keys_ref = tlskeys_ref_lookup(args[cur_arg + 1]);
> if(keys_ref) {
> conf->keys_ref = keys_ref;
> return 0;
> }
>
> So we clearly end up with a normal double-free. I'm checking what's the best
> solution to fix this now, as we don't have flags nor any such thing in the
> bind_conf to indicate that it must or must not be freed. We can't duplicate
> the allocation either because it's used for the updates. I think the cleanest
> solution will be to add a refcount to the tls_keys_ref entry.
>
> Then I'm proposing the attached patch which works for me and is obvious
> enough to make valgrind happy as well.
>
> Could you guys please give it a try to confirm ? I'll then merge it.
i, the patch works for me with backport to the 1.8 version. It is on
productio stage.
Thanks,
Thierry
> Thanks,
> Willy
--
Thierry Fournier
Web Performance & Security Expert
m: +33 6 68 69 21 85 | e: [email protected]
w: http://www.ozon.io/ | b: http://blog.ozon.io/
