63 lines
2.5 KiB
Diff
63 lines
2.5 KiB
Diff
From dde289ad39278a6a18f4141f61a08df9d7020b56 Mon Sep 17 00:00:00 2001
|
|
From: Xavi Hernandez <xhernandez@redhat.com>
|
|
Date: Thu, 19 Dec 2019 11:58:54 +0100
|
|
Subject: [PATCH] timer: fix event destruction race
|
|
|
|
In current timer implementation, each event has an absolute time at which
|
|
it will be fired. When the first timer of the queue has not elapsed yet,
|
|
a pthread_cond_timedwait() is used to wait until the expected time.
|
|
|
|
Apparently that's fine. However the time passed to that function was a
|
|
pointer to the timespec structure contained in the event itself. This is
|
|
problematic because of how pthread_cond_timedwait() works internally.
|
|
|
|
Simplifying a bit, pthread_cond_timedwait() basically queues itself as a
|
|
waiter for the given condition variable and releases the mutex. Then it
|
|
does the timed wait using the passed value.
|
|
|
|
With that in mind, the follwing case is possible:
|
|
|
|
Timer Thread Other Thread
|
|
------------ ------------
|
|
|
|
gf_timer_call_cancel()
|
|
pthread_mutex_lock() |
|
|
+ pthread_mutex_lock()
|
|
event = current_event() |
|
|
pthread_cond_timedwait(&event->at) |
|
|
+ pthread_mutex_unlock() |
|
|
| + remove_event()
|
|
| + destroy_event()
|
|
+ timed_wait(&event->at)
|
|
|
|
As we can see, the time is used after it has been destroyed, which means
|
|
we have a use-after-free problem.
|
|
|
|
This patch fixes the problem by copying the time to a local variable
|
|
before calling pthread_cond_timedwait()
|
|
|
|
Change-Id: I0f4e8eded24fe3a1276dc75c6cf093bae973d26b
|
|
Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
|
|
Fixes: bz#1785208
|
|
---
|
|
libglusterfs/src/timer.c | 3 ++-
|
|
1 file changed, 2 insertions(+), 1 deletion(-)
|
|
|
|
diff --git a/libglusterfs/src/timer.c b/libglusterfs/src/timer.c
|
|
index 1e19ffdff2..66c861b04c 100644
|
|
--- a/libglusterfs/src/timer.c
|
|
+++ b/libglusterfs/src/timer.c
|
|
@@ -137,7 +137,8 @@ gf_timer_proc(void *data)
|
|
timespec_now(&now);
|
|
event = list_first_entry(®->active, gf_timer_t, list);
|
|
if (TS(now) < TS(event->at)) {
|
|
- pthread_cond_timedwait(®->cond, ®->lock, &event->at);
|
|
+ now = event->at;
|
|
+ pthread_cond_timedwait(®->cond, ®->lock, &now);
|
|
} else {
|
|
event->fired = _gf_true;
|
|
list_del_init(&event->list);
|
|
--
|
|
2.33.0
|
|
|