use_after_free_crash.patch

   1 From 16f63a3c8fa227625bade5a9edea22354b347d18 Mon Sep 17 00:00:00 2001
   2 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= <pobrn@protonmail.com>
   3 Date: Fri, 18 Feb 2022 18:36:36 +0100
   4 Subject: [PATCH 1/2] Revert "loop: remove destroy list"
   5
   6 This reverts commit c474846c42967c44db069a23b76a29da6f496f33.
   7 In addition, `s->loop` is also checked before dispatching a source.
   8
   9 The destroy list is needed in the presence of threads. The
  10 issue is that a source may be destroyed between `epoll_wait()`
  11 returning and thread loop lock being acquired. If this
  12 source is active, then a use-after-free will be triggered
  13 when the thread loop acquires the lock and starts dispatching
  14 the sources.
  15
  16   thread 1                       thread 2
  17  ----------                     ----------
  18                                 loop_iterate
  19                                   spa_loop_control_hook_before
  20                                     // release lock
  21
  22  pw_thread_loop_lock
  23
  24                                   spa_system_pollfd_wait
  25                                     // assume it returns with source A
  26
  27  pw_loop_destroy_source(..., A)
  28   // frees storage of A
  29
  30  pw_thread_loop_unlock
  31                                   spa_loop_control_hook_after
  32                                     // acquire the lock
  33
  34                                   for (...) {
  35                                     struct spa_source *s = ep[i].data;
  36                                     s->rmask = ep[i].events;
  37                                       // use-after-free if `s` refers to
  38                                       // the previously freed `A`
  39
  40 Fixes #2147
  41 ---
  42  spa/plugins/support/loop.c | 19 +++++++++++++++++--
  43  1 file changed, 17 insertions(+), 2 deletions(-)
  44
  45 diff --git a/spa/plugins/support/loop.c b/spa/plugins/support/loop.c
  46 index 0588ce770..04739eb2a 100644
  47 --- a/spa/plugins/support/loop.c
  48 +++ b/spa/plugins/support/loop.c
  49 @@ -75,6 +75,7 @@ struct impl {
  50          struct spa_system *system;
  51
  52         struct spa_list source_list;
  53 +       struct spa_list destroy_list;
  54         struct spa_hook_list hooks_list;
  55
  56         int poll_fd;
  57 @@ -325,6 +326,14 @@ static void loop_leave(void *object)
  58                 impl->thread = 0;
  59  }
  60
  61 +static inline void process_destroy(struct impl *impl)
  62 +{
  63 +       struct source_impl *source, *tmp;
  64 +       spa_list_for_each_safe(source, tmp, &impl->destroy_list, link)
  65 +               free(source);
  66 +       spa_list_init(&impl->destroy_list);
  67 +}
  68 +
  69  static int loop_iterate(void *object, int timeout)
  70  {
  71         struct impl *impl = object;
  72 @@ -354,11 +363,14 @@ static int loop_iterate(void *object, int timeout)
  73         }
  74         for (i = 0; i < nfds; i++) {
  75                 struct spa_source *s = ep[i].data;
  76 -               if (SPA_LIKELY(s && s->rmask)) {
  77 +               if (SPA_LIKELY(s && s->rmask && s->loop)) {
  78                         s->priv = NULL;
  79                         s->func(s);
  80                 }
  81         }
  82 +       if (SPA_UNLIKELY(!spa_list_is_empty(&impl->destroy_list)))
  83 +               process_destroy(impl);
  84 +
  85         return nfds;
  86  }
  87
  88 @@ -712,7 +724,7 @@ static void loop_destroy_source(void *object, struct spa_source *source)
  89                 spa_system_close(impl->impl->system, source->fd);
  90                 source->fd = -1;
  91         }
  92 -       free(source);
  93 +       spa_list_insert(&impl->impl->destroy_list, &impl->link);
  94  }
  95
  96  static const struct spa_loop_methods impl_loop = {
  97 @@ -783,6 +795,8 @@ static int impl_clear(struct spa_handle *handle)
  98         spa_list_consume(source, &impl->source_list, link)
  99                 loop_destroy_source(impl, &source->source);
 100
 101 +       process_destroy(impl);
 102 +
 103         spa_system_close(impl->system, impl->ack_fd);
 104         spa_system_close(impl->system, impl->poll_fd);
 105
 106 @@ -844,6 +858,7 @@ impl_init(const struct spa_handle_factory *factory,
 107         impl->poll_fd = res;
 108
 109         spa_list_init(&impl->source_list);
 110 +       spa_list_init(&impl->destroy_list);
 111         spa_hook_list_init(&impl->hooks_list);
 112
 113         impl->buffer_data = SPA_PTR_ALIGN(impl->buffer_mem, MAX_ALIGN, uint8_t);
 114 --
 115 GitLab
 116
 117
 118 From d1f7e96f821089224ddcacf8e8f506f99c54eb5c Mon Sep 17 00:00:00 2001
 119 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= <pobrn@protonmail.com>
 120 Date: Fri, 18 Feb 2022 19:27:13 +0100
 121 Subject: [PATCH 2/2] test: loop: add test for destroying source of thread loop
 122
 123 Add test which tries to destroy an active source precisely
 124 after the loop has returned from polling but has not yet
 125 acquired the thread loop lock.
 126 ---
 127  test/test-loop.c | 34 ++++++++++++++++++++++++++++++++++
 128  1 file changed, 34 insertions(+)
 129
 130 diff --git a/test/test-loop.c b/test/test-loop.c
 131 index 98b2add09..81f7a117c 100644
 132 --- a/test/test-loop.c
 133 +++ b/test/test-loop.c
 134 @@ -227,11 +227,45 @@ PWTEST(pwtest_loop_recurse2)
 135         return PWTEST_PASS;
 136  }
 137
 138 +PWTEST(thread_loop_destroy_between_poll_and_lock)
 139 +{
 140 +       pw_init(NULL, NULL);
 141 +
 142 +       struct pw_thread_loop *thread_loop = pw_thread_loop_new("uaf", NULL);
 143 +       pwtest_ptr_notnull(thread_loop);
 144 +
 145 +       struct pw_loop *loop = pw_thread_loop_get_loop(thread_loop);
 146 +       pwtest_ptr_notnull(loop);
 147 +
 148 +       int evfd = eventfd(0, 0);
 149 +       pwtest_errno_ok(evfd);
 150 +
 151 +       struct spa_source *source = pw_loop_add_io(loop, evfd, SPA_IO_IN, true, NULL, NULL);
 152 +       pwtest_ptr_notnull(source);
 153 +
 154 +       pw_thread_loop_start(thread_loop);
 155 +
 156 +       pw_thread_loop_lock(thread_loop);
 157 +       {
 158 +               write(evfd, &(uint64_t){1}, sizeof(uint64_t));
 159 +               sleep(1);
 160 +               pw_loop_destroy_source(loop, source);
 161 +       }
 162 +       pw_thread_loop_unlock(thread_loop);
 163 +
 164 +       pw_thread_loop_destroy(thread_loop);
 165 +
 166 +       pw_deinit();
 167 +
 168 +       return PWTEST_PASS;
 169 +}
 170 +
 171  PWTEST_SUITE(support)
 172  {
 173         pwtest_add(pwtest_loop_destroy2, PWTEST_NOARG);
 174         pwtest_add(pwtest_loop_recurse1, PWTEST_NOARG);
 175         pwtest_add(pwtest_loop_recurse2, PWTEST_NOARG);
 176 +       pwtest_add(thread_loop_destroy_between_poll_and_lock, PWTEST_NOARG);
 177
 178         return PWTEST_PASS;
 179  }
 180 --
 181 GitLab
 182