From 87194009c0870704f03e26e14d2a06916a52f9f8 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 --- 0005-timer-fix-event-destruction-race.patch | 62 +++++++++++++++++++++ glusterfs.spec | 8 ++- 2 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 0005-timer-fix-event-destruction-race.patch diff --git a/0005-timer-fix-event-destruction-race.patch b/0005-timer-fix-event-destruction-race.patch new file mode 100644 index 0000000..a4783f6 --- /dev/null +++ b/0005-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 f735ea2..acf9ce2 100644 --- a/glusterfs.spec +++ b/glusterfs.spec @@ -3,7 +3,7 @@ Name: glusterfs Version: 7.0 -Release: 10 +Release: 11 License: GPLv2 and LGPLv3+ Summary: Aggregating distributed file system URL: http://docs.gluster.org/ @@ -14,7 +14,8 @@ Patch0: 0000-core-fix-memory-pool-management-races.patch Patch1: 0001-geo-rep-Fix-the-name-of-changelog-archive-file.patch Patch2: 0002-upcall-internal.c-fix-debug-log-message-3651.patch Patch3: 0003-SC2081-can-t-match-globs-Use-or-grep.patch -Patch4: 0004-fuse-Resolve-asan-bug-in-during-receive-event-notifi.patch +Patch4: 0004-fuse-Resolve-asan-bug-in-during-receive-event-notifi.patch +Patch5: 0005-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 @@ -463,6 +464,9 @@ exit 0 %{_mandir}/man8/*gluster*.8* %changelog +* Wed Jul 12 2023 wuguanghao - 7.0-11 +- timer: fix event destruction race + * Thu Mar 9 2023 wuguanghao - 7.0-10 - fix CVE-2023-26253 -- Gitee