net: fix use-after-free in slab compaction (#3019)

An off-by-one bug results in freeing the incorrect page. This
also adds an `asan` CI job.

Fixes: 3014

.github/workflows/ci.yml

@@ -112,6 +112,21 @@ jobs:
       - name: miri
         run: cargo miri test --features rt,rt-multi-thread,sync task
         working-directory: tokio
+  san:
+    name: san
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          toolchain: ${{ env.nightly }}
+          override: true
+      - name: asan
+        run: cargo test --all-features --target x86_64-unknown-linux-gnu --lib -- --test-threads 1
+        working-directory: tokio
+        env:
+          RUSTFLAGS: -Z sanitizer=address
+          ASAN_OPTIONS: detect_leaks=0
 
   cross:
     name: cross

tokio/src/util/slab.rs

@@ -272,7 +272,7 @@ impl<T> Slab<T> {
     pub(crate) fn compact(&mut self) {
         // Iterate each page except the very first one. The very first page is
         // never freed.
-        for (idx, page) in (&self.pages[1..]).iter().enumerate() {
+        for (idx, page) in self.pages.iter().enumerate().skip(1) {
             if page.used.load(Relaxed) != 0 || !page.allocated.load(Relaxed) {
                 // If the page has slots in use or the memory has not been
                 // allocated then it cannot be compacted.
@@ -302,6 +302,13 @@ impl<T> Slab<T> {
             // Drop the lock so we can drop the vector outside the lock below.
             drop(slots);
 
+            debug_assert!(
+                self.cached[idx].slots.is_null() || self.cached[idx].slots == vec.as_ptr(),
+                "cached = {:?}; actual = {:?}",
+                self.cached[idx].slots,
+                vec.as_ptr(),
+            );
+
             // Clear cache
             self.cached[idx].slots = ptr::null();
             self.cached[idx].init = 0;
@@ -488,8 +495,11 @@ impl<T> CachedPage<T> {
     /// Refresh the cache
     fn refresh(&mut self, page: &Page<T>) {
         let slots = page.slots.lock();
-        self.slots = slots.slots.as_ptr();
-        self.init = slots.slots.len();
+
+        if !slots.slots.is_empty() {
+            self.slots = slots.slots.as_ptr();
+            self.init = slots.slots.len();
+        }
     }
 
     // Get a value by index
@@ -791,4 +801,41 @@ mod test {
             }
         }
     }
+
+    #[test]
+    fn issue_3014() {
+        let mut slab = Slab::<Foo>::new();
+        let alloc = slab.allocator();
+        let mut entries = vec![];
+
+        for _ in 0..5 {
+            entries.clear();
+
+            // Allocate a few pages + 1
+            for i in 0..(32 + 64 + 128 + 1) {
+                let (addr, val) = alloc.allocate().unwrap();
+                val.id.store(i, SeqCst);
+
+                entries.push((addr, val, i));
+            }
+
+            for (addr, val, i) in &entries {
+                assert_eq!(*i, val.id.load(SeqCst));
+                assert_eq!(*i, slab.get(*addr).unwrap().id.load(SeqCst));
+            }
+
+            // Release the last entry
+            entries.pop();
+
+            // Compact
+            slab.compact();
+
+            // Check all the addresses
+
+            for (addr, val, i) in &entries {
+                assert_eq!(*i, val.id.load(SeqCst));
+                assert_eq!(*i, slab.get(*addr).unwrap().id.load(SeqCst));
+            }
+        }
+    }
 }