tui: fix use-after-free after UI `stop` event #7922

ui_bridge:ui_bridge_stop() calls ui_detach_impl() last, so the check for
ui_active() in ui:ui_refresh() doesn't help: tui_main() already freed
the `ui` object.

There is a race between ui_bridge_stop (thread T0) and tui_main (thread T1).
UIBridgeData.stopped could be set while ui_bridge_stop() is in the
middle of loop_poll_events(), which may invoke tui_scheduler() on T0.
The pointers in tui_scheduler() may be invalid by then.

Solution(?): Use the `UI.data` field as a "stopped" flag and check it in
tui_scheduler().

ASAN use-after-free report observed in #7908:

    = ==20066==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000000cd0 at pc 0x00000182abed bp 0x7ffe23b07070 sp 0x7ffe23b07068
    = READ of size 8 at 0x611000000cd0 thread T0
    =     0 0x182abec in tui_scheduler /home/travis/build/neovim/neovim/src/nvim/tui/tui.c:393:23
    =     1 0x1876afd in ui_bridge_update_fg /home/travis/build/neovim/neovim/build/src/nvim/auto/ui_events_bridge.generated.h:205:3
    =     2 0x186c130 in ui_resize /home/travis/build/neovim/neovim/src/nvim/ui.c:310:3
    =     3 0x146b9c2 in screen_resize /home/travis/build/neovim/neovim/src/nvim/screen.c:7483:3
    =     4 0x186a6f0 in ui_refresh /home/travis/build/neovim/neovim/src/nvim/ui.c:284:3
    =     5 0x186bbe0 in ui_refresh_event /home/travis/build/neovim/neovim/src/nvim/ui.c:297:3
    =     6 0xa2219a in multiqueue_process_events /home/travis/build/neovim/neovim/src/nvim/event/multiqueue.c:150:7
    =     7 0xa1bd7f in loop_poll_events /home/travis/build/neovim/neovim/src/nvim/event/loop.c:63:3
    =     8 0x1872709 in ui_bridge_stop /home/travis/build/neovim/neovim/src/nvim/ui_bridge.c:121:5
    =     9 0x1864247 in ui_builtin_stop /home/travis/build/neovim/neovim/src/nvim/ui.c:143:3
    =     10 0x1249ec8 in mch_exit /home/travis/build/neovim/neovim/src/nvim/os_unix.c:140:3
    =     11 0xe56ba9 in getout /home/travis/build/neovim/neovim/src/nvim/main.c:671:3
    =     12 0xfc4c8f in preserve_exit /home/travis/build/neovim/neovim/src/nvim/misc1.c:2653:3
    =     13 0x1247c02 in deadly_signal /home/travis/build/neovim/neovim/src/nvim/os/signal.c:137:3
    =     14 0x1247921 in on_signal /home/travis/build/neovim/neovim/src/nvim/os/signal.c:162:9
    =     15 0xa35618 in signal_event /home/travis/build/neovim/neovim/src/nvim/event/signal.c:47:3
    =     16 0xa2219a in multiqueue_process_events /home/travis/build/neovim/neovim/src/nvim/event/multiqueue.c:150:7
    =     17 0xa1bd7f in loop_poll_events /home/travis/build/neovim/neovim/src/nvim/event/loop.c:63:3
    =     18 0x1237bd6 in input_poll /home/travis/build/neovim/neovim/src/nvim/os/input.c:349:3
    =     19 0x123334f in inbuf_poll /home/travis/build/neovim/neovim/src/nvim/os/input.c:372:24
    =     20 0x123316d in os_inchar /home/travis/build/neovim/neovim/src/nvim/os/input.c:110:19
    =     21 0x170d20e in state_enter /home/travis/build/neovim/neovim/src/nvim/state.c:55:13
    =     22 0xbd7441 in command_line_enter /home/travis/build/neovim/neovim/src/nvim/ex_getln.c:384:3
    =     23 0xbd0a60 in getcmdline /home/travis/build/neovim/neovim/src/nvim/ex_getln.c:1920:10
    =     24 0xbdb365 in getexline /home/travis/build/neovim/neovim/src/nvim/ex_getln.c:2100:10
    =     25 0xb00a6b in do_cmdline /home/travis/build/neovim/neovim/src/nvim/ex_docmd.c:528:47
    =     26 0x10a7837 in nv_colon /home/travis/build/neovim/neovim/src/nvim/normal.c:4552:18
    =     27 0x1091e15 in normal_execute /home/travis/build/neovim/neovim/src/nvim/normal.c:1136:3
    =     28 0x170d439 in state_enter /home/travis/build/neovim/neovim/src/nvim/state.c:67:26
    =     29 0x104ee14 in normal_enter /home/travis/build/neovim/neovim/src/nvim/normal.c:466:3
    =     30 0xe4295c in main /home/travis/build/neovim/neovim/src/nvim/main.c:572:3
    =     31 0x2b2ba340bf44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287
    =     32 0x44d24b in _start (/home/travis/build/neovim/neovim/build/bin/nvim+0x44d24b)
    =
    = 0x611000000cd0 is located 16 bytes inside of 240-byte region [0x611000000cc0,0x611000000db0)
    = freed by thread T1 here:
    =     0 0x4ee0e2 in __interceptor_free /local/mnt/workspace/tmp/ubuntu_rel/llvm/utils/release/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:47:3
    =     1 0xf4f6d4 in xfree /home/travis/build/neovim/neovim/src/nvim/memory.c:133:3
    =     2 0x182a963 in tui_main /home/travis/build/neovim/neovim/src/nvim/tui/tui.c:383:3
    =     3 0x18792b0 in ui_thread_run /home/travis/build/neovim/neovim/src/nvim/ui_bridge.c:106:3
    =     4 0x2b2ba2697183 in start_thread /build/eglibc-ripdx6/eglibc-2.19/nptl/pthread_create.c:312
    =
    = previously allocated by thread T0 here:
    =     0 0x4ee61a in calloc /local/mnt/workspace/tmp/ubuntu_rel/llvm/utils/release/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:76:3
    =     1 0xf4f787 in xcalloc /home/travis/build/neovim/neovim/src/nvim/memory.c:147:15
    =     2 0x182000a in tui_start /home/travis/build/neovim/neovim/src/nvim/tui/tui.c:127:12
    =     3 0x1863f7c in ui_builtin_start /home/travis/build/neovim/neovim/src/nvim/ui.c:125:3
    =     4 0xe41bb9 in main /home/travis/build/neovim/neovim/src/nvim/main.c:457:5
    =     5 0x2b2ba340bf44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287
    =
    = Thread T1 created by T0 here:
    =     0 0x4d774d in __interceptor_pthread_create /local/mnt/workspace/tmp/ubuntu_rel/llvm/utils/release/final/llvm.src/projects/compiler-rt/lib/asan/asan_interceptors.cc:317:3
    =     1 0x1aae6b0 in uv_thread_create /home/travis/nvim-deps/build/src/libuv/src/unix/thread.c:75
    =     2 0x18217fa in tui_start /home/travis/build/neovim/neovim/src/nvim/tui/tui.c:159:10
    =     3 0x1863f7c in ui_builtin_start /home/travis/build/neovim/neovim/src/nvim/ui.c:125:3
    =     4 0xe41bb9 in main /home/travis/build/neovim/neovim/src/nvim/main.c:457:5
    =     5 0x2b2ba340bf44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287

---

Alternative attempt:

    commit 6ad9c02491606a0c31e907f38c9931f324327aa5
    Author: Justin M. Keyes <justinkz@gmail.com>
    Date:   Sat Jan 27 15:12:58 2018 +0100

        tui: fix use-after-free: swap in empty scheduler

        This should make life easier for UIs like VimR which implement their own
        in-process bridged UI: they don't need to worry that their `scheduler`
        might receive an invalid pointer.

        To avoid that, ui_bridge_stopped() swaps in an empty scheduler. Note
        that this requires the call to loop_poll_events() to be moved into the
        critical section.

    diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c
    index 779585416f80..491052d19d3b 100644
    --- a/src/nvim/ui_bridge.c
    +++ b/src/nvim/ui_bridge.c
    @@ -93,10 +93,18 @@ UI *ui_bridge_attach(UI *ui, ui_main_fn ui_main, event_scheduler scheduler)
       return &rv->bridge;
     }

    +static void ui_bridge_null_scheduler(Event event, void *d)
    +{
    +  WLOG("ignoring event (bridge stopped)");
    +}
    +
     void ui_bridge_stopped(UIBridgeData *bridge)
     {
       uv_mutex_lock(&bridge->mutex);
       bridge->stopped = true;
    +  // Replace with an empty scheduler, so that the UI internal scheduler does
    +  // not get invoked with an invalid pointer. #7922
    +  bridge->scheduler = ui_bridge_null_scheduler;
       uv_mutex_unlock(&bridge->mutex);
     }

    @@ -111,14 +119,11 @@ static void ui_bridge_stop(UI *b)
       UIBridgeData *bridge = (UIBridgeData *)b;
       bool stopped = bridge->stopped = false;
       UI_BRIDGE_CALL(b, stop, 1, b);
    -  for (;;) {
    +  while (!stopped) {
         uv_mutex_lock(&bridge->mutex);
         stopped = bridge->stopped;
    -    uv_mutex_unlock(&bridge->mutex);
    -    if (stopped) {
    -      break;
    -    }
         loop_poll_events(&main_loop, 10);  // Process one event (at most).
    +    uv_mutex_unlock(&bridge->mutex);
       }
       uv_thread_join(&bridge->ui_thread);
       uv_mutex_destroy(&bridge->mutex);

5 files changed, 26 insertions, 15 deletions

diff --git a/src/nvim/api/ui.c b/src/nvim/api/ui.c
index 35508fde6b..760c95eb5b 100644
--- a/src/nvim/api/ui.c
+++ b/src/nvim/api/ui.c
@@ -43,10 +43,10 @@ void remote_ui_disconnect(uint64_t channel_id)
     return;
   }
   UIData *data = ui->data;
-  // destroy pending screen updates
-  api_free_array(data->buffer);
+  api_free_array(data->buffer);  // Destroy pending screen updates.
   pmap_del(uint64_t)(connected_uis, channel_id);
   xfree(ui->data);
+  ui->data = NULL;  // Flag UI as "stopped".
   ui_detach_impl(ui);
   xfree(ui);
 }
diff --git a/src/nvim/event/loop.c b/src/nvim/event/loop.c
index 55ef0261d9..d92464f17b 100644
--- a/src/nvim/event/loop.c
+++ b/src/nvim/event/loop.c
@@ -33,6 +33,9 @@ void loop_init(Loop *loop, void *data)
   loop->poll_timer.data = xmalloc(sizeof(bool));  // "timeout expired" flag
 }
 
+/// Processes one `Loop.uv` event (at most).
+/// Processes all `Loop.fast_events` events.
+///
 /// @returns true if `ms` timeout was reached
 bool loop_poll_events(Loop *loop, int ms)
 {
diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c
index f3383eb006..2dfe7faa04 100644
--- a/src/nvim/tui/tui.c
+++ b/src/nvim/tui/tui.c
@@ -69,7 +69,6 @@ typedef struct {
 typedef struct {
   UIBridgeData *bridge;
   Loop *loop;
-  bool stop;
   unibi_var_t params[9];
   char buf[OUTBUF_SIZE];
   size_t bufpos;
@@ -124,7 +123,7 @@ static bool cursor_style_enabled = false;
 
 UI *tui_start(void)
 {
-  UI *ui = xcalloc(1, sizeof(UI));
+  UI *ui = xcalloc(1, sizeof(UI));  // Freed by ui_bridge_stop().
   ui->stop = tui_stop;
   ui->rgb = p_tgc;
   ui->resize = tui_resize;
@@ -324,11 +323,11 @@ static void tui_terminal_stop(UI *ui)
 static void tui_stop(UI *ui)
 {
   tui_terminal_stop(ui);
-  TUIData *data = ui->data;
-  data->stop = true;
+  // Flag UI as "stopped". Needed by tui_scheduler (called from main thread).
+  ui->data = NULL;
 }
 
-// Main function of the TUI thread
+/// Main function of the TUI thread.
 static void tui_main(UIBridgeData *bridge, UI *ui)
 {
   Loop tui_loop;
@@ -349,7 +348,6 @@ static void tui_main(UIBridgeData *bridge, UI *ui)
 #endif
   term_input_init(&data->input, &tui_loop);
   tui_terminal_start(ui);
-  data->stop = false;
 
   // Allow main thread to continue, we are ready to handle UI callbacks.
   CONTINUE(bridge);
@@ -358,17 +356,17 @@ static void tui_main(UIBridgeData *bridge, UI *ui)
                          event_create(show_termcap_event, 1, data->ut));
 
   // "Active" loop: first ~100 ms of startup.
-  for (size_t ms = 0; ms < 100 && !data->stop;) {
+  for (size_t ms = 0; ms < 100 && !ui_is_stopped(ui);) {
     ms += (loop_poll_events(&tui_loop, 20) ? 20 : 1);
   }
-  if (!data->stop) {
+  if (!ui_is_stopped(ui)) {
     tui_terminal_after_startup(ui);
     // Tickle `main_loop` with a dummy event, else the initial "focus-gained"
     // terminal response may not get processed until user hits a key.
     loop_schedule_deferred(&main_loop, event_create(tui_dummy_event, 0));
   }
   // "Passive" (I/O-driven) loop: TUI thread "main loop".
-  while (!data->stop) {
+  while (!ui_is_stopped(ui)) {
     loop_poll_events(&tui_loop, -1);  // tui_loop.events is never processed
   }
 
@@ -380,16 +378,19 @@ static void tui_main(UIBridgeData *bridge, UI *ui)
   loop_close(&tui_loop, false);
   kv_destroy(data->invalid_regions);
   xfree(data);
-  xfree(ui);
 }
 
 static void tui_dummy_event(void **argv)
 {
 }
 
+/// Handoff point between the main (ui_bridge) thread and the TUI thread.
 static void tui_scheduler(Event event, void *d)
 {
   UI *ui = d;
+  if (ui_is_stopped(ui)) {
+    return;  // tui_stop was handled, teardown underway.
+  }
   TUIData *data = ui->data;
   loop_schedule(data->loop, event);  // `tui_loop` local to tui_main().
 }
diff --git a/src/nvim/ui.c b/src/nvim/ui.c
index 81da88c54a..8aec923538 100644
--- a/src/nvim/ui.c
+++ b/src/nvim/ui.c
@@ -143,6 +143,12 @@ void ui_builtin_stop(void)
   UI_CALL(stop);
 }
 
+/// Returns true if UI `ui` is stopped.
+bool ui_is_stopped(UI *ui)
+{
+  return ui->data == NULL;
+}
+
 bool ui_rgb_attached(void)
 {
   for (size_t i = 0; i < ui_count; i++) {
@@ -404,7 +410,7 @@ void ui_start_highlight(int attr_code)
 {
   current_attr_code = attr_code;
 
-  if (!ui_count) {
+  if (!ui_active()) {
     return;
   }
 
@@ -415,7 +421,7 @@ void ui_stop_highlight(void)
 {
   current_attr_code = HL_NORMAL;
 
-  if (!ui_count) {
+  if (!ui_active()) {
     return;
   }
 
diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c
index 0a69cf0ecb..16dd42ebaa 100644
--- a/src/nvim/ui_bridge.c
+++ b/src/nvim/ui_bridge.c
@@ -118,11 +118,12 @@ static void ui_bridge_stop(UI *b)
     if (stopped) {
       break;
     }
-    loop_poll_events(&main_loop, 10);
+    loop_poll_events(&main_loop, 10);  // Process one event.
   }
   uv_thread_join(&bridge->ui_thread);
   uv_mutex_destroy(&bridge->mutex);
   uv_cond_destroy(&bridge->cond);
+  xfree(bridge->ui);  // Threads joined, now safe to free UI container. #7922
   ui_detach_impl(b);
   xfree(b);
 }