From 78c41ab8189fa70f102ee2981cbcb1d08100ba42 Mon Sep 17 00:00:00 2001 From: wguanghao Date: Wed, 12 Jul 2023 15:36:19 +0800 Subject: [PATCH] timer: fix event destruction race --- 0002-timer-fix-event-destruction-race.patch | 62 +++++++++++++++++++++ glusterfs.spec | 4 ++ 2 files changed, 66 insertions(+) create mode 100644 0002-timer-fix-event-destruction-race.patch diff --git a/0002-timer-fix-event-destruction-race.patch b/0002-timer-fix-event-destruction-race.patch new file mode 100644 index 0000000..a4783f6 --- /dev/null +++ b/0002-timer-fix-event-destruction-race.patch @@ -0,0 +1,62 @@ +From dde289ad39278a6a18f4141f61a08df9d7020b56 Mon Sep 17 00:00:00 2001 +From: Xavi Hernandez +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 +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 + diff --git a/glusterfs.spec b/glusterfs.spec index cd52f3a..d9baa7b 100644 --- a/glusterfs.spec +++ b/glusterfs.spec @@ -12,6 +12,7 @@ Source7: glusterfsd.service Patch0: 0000-core-fix-memory-pool-management-races.patch Patch1: 0001-geo-rep-Fix-the-name-of-changelog-archive-file.patch +Patch2: 0002-timer-fix-event-destruction-race.patch BuildRequires: systemd bison flex gcc make libtool ncurses-devel readline-devel libattr-devel BuildRequires: libxml2-devel openssl-devel libaio-devel libacl-devel python3-devel git perl @@ -460,6 +461,9 @@ exit 0 %{_mandir}/man8/*gluster*.8* %changelog +* Wed Jun 12 2023 wuguanghao - 7.0-8 +- timer: fix event destruction race + * Tue Jun 8 2021 yanglongkang - 7.0-7 - geo-rep fix the name of changelog archive file -- Gitee