2017-08-16 Memory Issues and C

I run Bitlbee using the address sanitizer – `./configure --debug=1 --asan=1`. When losing the connection to the Mastodon server, I get the following error:

Bitlbee

==54300==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000046778 at pc 0x00010b688437 bp 0x7fff5460db10 sp 0x7fff5460db08
READ of size 4 at 0x606000046778 thread T0
    #0 0x10b688436 in ssl_disconnect (bitlbee:x86_64+0x100098436)
    #1 0x10b665797 in http_close (bitlbee:x86_64+0x100075797)
    #2 0x10b781e30 in mastodon_http_stream (bitlbee:x86_64+0x100191e30)
    #3 0x10b776856 in mastodon_http_stream_user (bitlbee:x86_64+0x100186856)
    #4 0x10b666984 in http_incoming_data (bitlbee:x86_64+0x100076984)
    #5 0x10b66253d in gaim_io_invoke (bitlbee:x86_64+0x10007253d)
    #6 0x10b9291bc in g_main_context_dispatch (libglib-2.0.0.dylib:x86_64+0x2d1bc)
    #7 0x10b9294bb in g_main_context_iterate (libglib-2.0.0.dylib:x86_64+0x2d4bb)
    #8 0x10b929710 in g_main_loop_run (libglib-2.0.0.dylib:x86_64+0x2d710)
    #9 0x10b66226f in b_main_run (bitlbee:x86_64+0x10007226f)
    #10 0x10b658222 in main unix.c:182
    #11 0x7fffc030d234 in start (libdyld.dylib:x86_64+0x5234)

0x606000046778 is located 24 bytes inside of 56-byte region [0x606000046760,0x606000046798)
freed by thread T0 here:
    #0 0x10bc671c6 in wrap_free (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x531c6)
    #1 0x10b68866d in ssl_disconnect (bitlbee:x86_64+0x10009866d)
    #2 0x10b6665f0 in http_incoming_data (bitlbee:x86_64+0x1000765f0)
    #3 0x10b66253d in gaim_io_invoke (bitlbee:x86_64+0x10007253d)
    #4 0x10b9291bc in g_main_context_dispatch (libglib-2.0.0.dylib:x86_64+0x2d1bc)
    #5 0x10b9294bb in g_main_context_iterate (libglib-2.0.0.dylib:x86_64+0x2d4bb)
    #6 0x10b929710 in g_main_loop_run (libglib-2.0.0.dylib:x86_64+0x2d710)
    #7 0x10b66226f in b_main_run (bitlbee:x86_64+0x10007226f)
    #8 0x10b658222 in main unix.c:182
    #9 0x7fffc030d234 in start (libdyld.dylib:x86_64+0x5234)

previously allocated by thread T0 here:
    #0 0x10bc67560 in wrap_calloc (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x53560)
    #1 0x10b92e28c in g_malloc0 (libglib-2.0.0.dylib:x86_64+0x3228c)
    #2 0x10b687117 in ssl_connect (bitlbee:x86_64+0x100097117)
    #3 0x10b663aef in http_dorequest (bitlbee:x86_64+0x100073aef)
    #4 0x10b7757fd in mastodon_http (bitlbee:x86_64+0x1001857fd)
    #5 0x10b776825 in mastodon_open_stream (bitlbee:x86_64+0x100186825)
    #6 0x10b76ba8f in mastodon_connect (bitlbee:x86_64+0x10017ba8f)
    #7 0x10b771697 in mastodon_login (bitlbee:x86_64+0x100181697)
    #8 0x10b6935aa in account_on (bitlbee:x86_64+0x1000a35aa)
    #9 0x10b642130 in cmd_account root_commands.c:538
    #10 0x10b6404a4 in cmd_identify_finish root_commands.c:211
    #11 0x10b64c2d7 in cmd_identify root_commands.c:194
    #12 0x10b640285 in root_command root_commands.c:68
    #13 0x10b63fe33 in root_command_string root_commands.c:34
    #14 0x10b639451 in root_privmsg irc_user.c:244
    #15 0x10b62286f in control_channel_privmsg irc_channel.c:739
    #16 0x10b628a4e in irc_cmd_privmsg irc_commands.c:485
    #17 0x10b624f5a in irc_exec irc_commands.c:922
    #18 0x10b60aa79 in irc_process irc.c:412
    #19 0x10b5f45a8 in bitlbee_io_current_client_read bitlbee.c:211
    #20 0x10b66253d in gaim_io_invoke (bitlbee:x86_64+0x10007253d)
    #21 0x10b9291bc in g_main_context_dispatch (libglib-2.0.0.dylib:x86_64+0x2d1bc)
    #22 0x10b9294bb in g_main_context_iterate (libglib-2.0.0.dylib:x86_64+0x2d4bb)
    #23 0x10b929710 in g_main_loop_run (libglib-2.0.0.dylib:x86_64+0x2d710)
    #24 0x10b66226f in b_main_run (bitlbee:x86_64+0x10007226f)
    #25 0x10b658222 in main unix.c:182
    #26 0x7fffc030d234 in start (libdyld.dylib:x86_64+0x5234)

What’s going on? In `ssl_connect` Bitlbee is allocating a connection:

void *ssl_connect(char *host, int port, gboolean verify, ssl_input_function func, gpointer data)
{
        struct scd *conn = g_new0(struct scd, 1);
        ...
}

When `http_incoming_data` decides it is done, `ssl_disconnect` is called and the connection is freed:

void ssl_disconnect(void *conn_)
{
        ...
        g_free(conn);
}

When `mastodon_http_stream` closes down, it calls `http_close` which calls `ssl_disconnect` again, which still refers to `conn`, which is wrong.

OK, but how is `http_incoming_data` calling both `ssl_disconnect` and `mastodon_http_stream`?

static gboolean http_incoming_data(gpointer data, int source, b_input_condition cond)
{
        struct http_request *req = data;
        ...
cleanup:
        /* Avoid g_source_remove warnings */
        req->inpa = 0;

        if (req->ssl) {
                ssl_disconnect(req->ssl);
        } else {
                closesocket(req->fd);
        }

        ...

        if (req->func != NULL) {
                req->func(req);
        }
        http_free(req);
        return FALSE;
}

OK, so here’s the problem. The SSL disconnect already happened but `req->ssl` was not set to NULL so there is no way for `http_close` to do the right thing.

static void mastodon_http_stream(struct http_request *req, gboolean from_hashtag)
{
        struct im_connection *ic = req->data;
        struct mastodon_data *md = ic->proto_data;
        ...

        if (!g_slist_find(mastodon_connections, ic)) {
                return;
        }

        if ((req->flags & HTTPC_EOF) || !req->reply_body) {
                md->streams = g_slist_remove (md->streams, req);
                imcb_error(ic, "Stream closed (%s)", req->status_string);
                imc_logout(ic, TRUE);
                http_close(req);
                return;
        }

        ...
}

Sadly, this is not all. When I remove the call to `http_close` in `mastodon_http_stream`, I have the problem that `mastodon_parse_response` calls `imc_logout` which frees the omnipresent `ic` variable, the `struct im_connection`. I have the problem that it calls `mastodon_logout` which goes through `md->streams` and closes those requests, which are already closed. It’s a mess and I don’t see a unifying theme, it’s unclear to me how logout *ought* to work in this asynchronous environment.

What seems clear to me now is that a logout function should *not* free data structures that are needed by all the ongoing requests. As these responses return, the data structures must still be there. Something like this:

1. the logout must mark the connection as dead

2. every response must free the http request and the associated ssl connection if it detects a dead connection

3. it must then remove the request from a list of open requests and free its resources

4. it must then attempt to free the connection, and this may only succeed when all the requests have returned

But actually, what happens is this:

1. user quits

2. `irc_free` → `bee_free` → `imc_logout` → `mastodon_logout` → `imc_free` – and at this point `ic` is free and cannot be referred to, so anything like the following is doomed:static void mastodon_http_callback(struct http_request *req) { struct mastodon_command *mc = req->data; struct im_connection *ic = mc->ic; if (!g_slist_find(mastodon_connections, ic)) { return; } ... }

I think? Or I’m confused? Perhaps this is OK if `ic` is pointing to an address which is no longer in the list of `mastodon_connections` and I should be safe as long as `req` has not been freed?

​#C ​#Bitlbee