From f5005d4b8058c92dcd9c6e6fa69f7064b2d0c7f1 Mon Sep 17 00:00:00 2001
From: David Ventura <davidventura27@gmail.com>
Date: Thu, 28 Mar 2024 16:18:25 +0100
Subject: [PATCH] Flush data/instruction cache after modifying vDSO (#4)

* Flush data/instruction cache after modifying vDSO

* sync tasks via mutex

* guard vdso overwrites with a static mutex
---
 Cargo.toml   |  3 ++-
 src/vdso.rs  | 13 +++++++++++++
 tests/pub.rs | 12 +++++++-----
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 0baf60e..f65f57a 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -16,13 +16,14 @@ path = "src/lib.rs"
 [[bin]]
 name = "tpom"
 path = "src/bin.rs"
+
 [dependencies]
+cacheflush-sys = "0.1.0"
 ctor = "0.2.6"
 goblin = "0.6.0"
 libc = "0.2.151"
 
 [dev-dependencies]
-serial_test = "0.9.0"
 
 
 [profile.release]
diff --git a/src/vdso.rs b/src/vdso.rs
index 32eafb7..9c26884 100644
--- a/src/vdso.rs
+++ b/src/vdso.rs
@@ -4,6 +4,10 @@ use goblin::strtab::Strtab;
 use core::slice;
 use std::error::Error;
 use std::fs;
+use cacheflush_sys;
+use std::sync::Mutex;
+
+static vdso_mutex: Mutex<i32> = Mutex::new(0);
 
 #[derive(Debug, PartialEq)]
 pub(crate) struct DynSym {
@@ -106,11 +110,20 @@ impl vDSO {
     /// It is the caller's responsibility to provide the correct amount of data.
     pub(crate) fn overwrite(&self, symbol_address: usize, opcodes: &[u8]) {
         let dst_addr = self.avv.vdso_base + symbol_address;
+
+        let _guard = vdso_mutex.lock().unwrap();
         self.change_mode(true);
         unsafe {
             std::ptr::copy_nonoverlapping(opcodes.as_ptr(), dst_addr as *mut u8, opcodes.len())
         };
+        // https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/caches-and-self-modifying-code
+        // We need to clear the instruction cache, otherwise it's possible that the old
+        // instructions (the trampoline) get executed with the new data (the original vDSO
+        // function)
         self.change_mode(false);
+        unsafe {
+            cacheflush_sys::flush(dst_addr as *const u8, opcodes.len()).unwrap();
+        }
     }
 
     pub fn entry(&self, wanted: Kind) -> Option<impl TVDSOFun + '_> {
diff --git a/tests/pub.rs b/tests/pub.rs
index df379cd..22300cb 100644
--- a/tests/pub.rs
+++ b/tests/pub.rs
@@ -1,9 +1,11 @@
 mod tests {
-    use serial_test::serial;
     use std::time::{SystemTime, Duration};
     use tpom::{vdso, Kind, TVDSOFun, TimeSpec};
     use std::thread;
     use std::hint::black_box;
+    use std::sync::Mutex;
+
+    static tm: Mutex<i32> = Mutex::new(0);
 
     fn myclock(_clockid: i32) -> TimeSpec {
         TimeSpec {
@@ -13,8 +15,8 @@ mod tests {
     }
 
     #[test]
-    #[serial]
     fn regular_clock_produces_different_timestamps() {
+        let _guard = tm.lock().unwrap();
         let time_a = SystemTime::now();
         thread::sleep(std::time::Duration::from_millis(1)); // clock in github actions is coarse
         let time_b = SystemTime::now();
@@ -22,8 +24,8 @@ mod tests {
     }
 
     #[test]
-    #[serial]
     fn it_freezes_system_clock() {
+        let _guard = tm.lock().unwrap();
         let v = vdso::vDSO::read().unwrap();
         let og = v
             .entry(Kind::GetTime)
@@ -39,8 +41,8 @@ mod tests {
     }
 
     #[test]
-    #[serial]
     fn it_works_many_threads() {
+        let _guard = tm.lock().unwrap();
         let v = vdso::vDSO::read().unwrap();
         let og = v
             .entry(Kind::GetTime)
@@ -63,8 +65,8 @@ mod tests {
     }
 
     #[test]
-    #[serial]
     fn it_works_after_setenv() {
+        let _guard = tm.lock().unwrap();
         std::env::set_var("SOMETHING", "VALUE");
         let v = vdso::vDSO::read().unwrap();
         let og = v