SIGSEGV: when destroying connection

[phiremande]

Bug Report

Describe the bug
There is a double free when destroying connections, if the endpoint in unavailable.

To Reproduce

• Rubular link if applicable:
• Example log message if applicable:

349 ==29485== Invalid write of size 8
350 ==29485== at 0x17479C: __mk_list_del (in /usr/local/bin/fluent-bit)
351 ==29485== by 0x1747D3: mk_list_del (in /usr/local/bin/fluent-bit)
352 ==29485== by 0x1750D3: destroy_conn (in /usr/local/bin/fluent-bit)
353 ==29485== by 0x1756D9: cb_upstream_conn_ka_dropped (in /usr/local/bin/fluent-bit)
354 ==29485== by 0x16E8BD: flb_engine_start (in /usr/local/bin/fluent-bit)
355 ==29485== by 0x1500B3: flb_main (in /usr/local/bin/fluent-bit)
356 ==29485== by 0x150101: main (in /usr/local/bin/fluent-bit)
357 ==29485== Address 0x8 is not stack'd, malloc'd or (recently) free'd
358 ==29485==
359 ==29485==
360 ==29485== Process terminating with default action of signal 6 (SIGABRT)
361 ==29485== at 0x5853F47: raise (raise.c:51)
362 ==29485== by 0x58558B0: abort (abort.c:79)
363 ==29485== by 0x14EE76: flb_signal_handler (in /usr/local/bin/fluent-bit)
364 ==29485== by 0x5853FCF: ??? (in /lib/x86_64-linux-gnu/libc-2.27.so)
365 ==29485== by 0x17479B: __mk_list_del (in /usr/local/bin/fluent-bit)
366 ==29485== by 0x1747D3: mk_list_del (in /usr/local/bin/fluent-bit)
367 ==29485== by 0x1750D3: destroy_conn (in /usr/local/bin/fluent-bit)
368 ==29485== by 0x1756D9: cb_upstream_conn_ka_dropped (in /usr/local/bin/fluent-bit)
369 ==29485== by 0x16E8BD: flb_engine_start (in /usr/local/bin/fluent-bit)
370 ==29485== by 0x1500B3: flb_main (in /usr/local/bin/fluent-bit)
371 ==29485== by 0x150101: main (in /usr/local/bin/fluent-bit)

[engine] caught signal (SIGSEGV)
#0 0x1745f1 in ???() at ???:0
#1 0x174628 in ???() at ???:0
#2 0x174f7b in ???() at ???:0
#3 0x175581 in ???() at ???:0
#4 0x16e712 in ???() at ???:0
#5 0x14ffd3 in ???() at ???:0
#6 0x150021 in ???() at ???:0
#7 0x5836b96 in ???() at ???:0
#8 0x14d999 in divide_look_behind_alternatives() at lib/onigmo/regcomp.c:3296
#9 0x14d999 in setup_look_behind() at lib/onigmo/regcomp.c:3314
#10 0xffffffffffffffff in ???() at ???:0
Aborted (core dumped)

• Steps to reproduce the problem:
  Configure, retry_limit false for the http output plugin and configure an endpoint which is not reachable. Sample configuration given below.

Expected behavior
fluent-bit shouldn't crash.

Your Environment

• Version used: 1.6.0

• Configuration:

  storage.path ./storage
  storage.sync full
  storage.checksum on
  storage.backlog.mem_limit 10M
  storage.metrics on

[INPUT]
Name mem
Tag memory
storage.type filesystem
Mem_Buf_Limit 10M

storage_limit 5K

[OUTPUT]
Name http
Match *
Host 192.168.99.99
Port 80
URI /something
Retry_Limit False

• Environment name and version (e.g. Kubernetes? What version?):
• Server type and version:
• Operating System and version: Ubuntu linux.
• Filters and plugins:

Additional context
Was trying to test the log buffering with storage as filesystem.

[phiremande]

@edsiper , I see that there is double free happening.

34 ^[[1m[^[[0m2020/08/25 11:19:12^[[1m]^[[0m [^[[93mdebug^[[0m] [upstream] KA connection #21 is in a failed state to: 192.168.99.99:80, cleaning up
35 ^[[1m[^[[0m2020/08/25 11:19:50^[[1m]^[[0m [^[[91merror^[[0m] [http_client] broken connection to 192.168.99.99:80 ?
36 ^[[1m[^[[0m2020/08/25 11:19:50^[[1m]^[[0m [^[[91merror^[[0m] [output:http:http.0] could not flush records to 192.168.99.99:80 (http_do=-1)
37 ^[[1m[^[[0m2020/08/25 11:19:50^[[1m]^[[0m [^[[93mdebug^[[0m] [upstream] KA connection #21 to 192.168.99.99:80 is now available
38 ^[[1m[^[[0m2020/08/25 11:19:50^[[1m]^[[0m [^[[93mdebug^[[0m] [upstream] KA connection #21 to 192.168.99.99:80 has been disconnected by the remote service

Hence the destroy_conn is getting called twice.

I think we can add the below fix. Please do let me know if this is sufficient.

+++ b/src/flb_upstream.c
@@ -175,8 +175,9 @@ struct flb_upstream *flb_upstream_create_url(struct flb_config *config,
 static int destroy_conn(struct flb_upstream_conn *u_conn)
 {
     struct flb_upstream *u = u_conn->u;
+    struct mk_list *entry = NULL;

-    flb_trace("[upstream] destroy connection #%i to %s:%i",
+    flb_debug("[upstream] destroy connection #%i to %s:%i",
               u_conn->fd, u->tcp_host, u->tcp_port);

     if (u->flags & FLB_IO_ASYNC) {
@@ -192,10 +193,15 @@ static int destroy_conn(struct flb_upstream_conn *u_conn)

     if (u_conn->fd > 0) {
         flb_socket_close(u_conn->fd);
+        u_conn->fd = -1;
     }

     /* remove connection from the queue */
-    mk_list_del(&u_conn->_head);
-
-    u->n_connections--;
-    flb_free(u_conn);
+    /* TODO: check if already freed. */
+    entry = &u_conn->_head;
+    if (entry && ((entry->next != NULL) || (entry->prev != NULL))) {
+        mk_list_del(&u_conn->_head);
+        u->n_connections--;
+        flb_free(u_conn);
+    }

[PettitWesley]

@phiremande Can you submit a PR with your fix? It'll be easier to review your proposal if it is a pull request.

[phiremande]

@PettitWesley , please review the PR.

[edsiper]

Fixed in #2531