[Asterisk-code-review] res_hiredis: REDIS DeviceState and Dialplan functions (asterisk[master])

[Alec Davis]

Attention is currently required from: Sean Bright, N A.
Alec Davis has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18829 )

Change subject: res_hiredis: REDIS DeviceState and Dialplan functions
......................................................................


Patch Set 19:

(1 comment)

Patchset:

PS19: 
Patchset 19:

Now the thread_init doesn't do the incorrect memcpy to some where, it now set's the *data to the new_redisContext. I hope that's correct?.

I'm still getting the double free with debugging on in my hiredis_thread_cleanup().

The double free I'm sure is because redisFree() free's the context see hi_free(c) at end of code below, then ast_unregister_thread also tries to free the same context.

code from hiredis library https://github.com/redis/hiredis/blob/master/hiredis.c line 719

void redisFree(redisContext *c) {
    if (c == NULL)
        return;
    redisNetClose(c);

    sdsfree(c->obuf);
    redisReaderFree(c->reader);
    hi_free(c->tcp.host);
    hi_free(c->tcp.source_addr);
    hi_free(c->unix_sock.path);
    hi_free(c->connect_timeout);
    hi_free(c->command_timeout);
    hi_free(c->saddr);

    if (c->privdata && c->free_privdata)
        c->free_privdata(c->privdata);

    if (c->funcs->free_privctx)
        c->funcs->free_privctx(c->privctx);

    memset(c, 0xff, sizeof(*c));
    hi_free(c);                              <<<<<<<<<< cause of double free <<<<<<<<<<<<
}

How can this double free properly be worked around?

1.)perhaps malloc a dummy redisContext, assign it to threadstorage, then let ast_unregister_thread  free it.
2.) get the hiredis library to have a function that release all resources except for the context.

Alec



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18829
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Ifdf4d33d2a2a5fd044fac13b201c7437de34ba6a
Gerrit-Change-Number: 18829
Gerrit-PatchSet: 19
Gerrit-Owner: Alec Davis <alec at bdt.co.nz>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Sean Bright <sean at seanbright.com>
Gerrit-Attention: Sean Bright <sean at seanbright.com>
Gerrit-Attention: N A <mail at interlinked.x10host.com>
Gerrit-Comment-Date: Wed, 03 Aug 2022 07:29:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220803/5878764d/attachment.html>