From ce52b565b42817ebb7208a794399da548902cf6c Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 21 Oct 2020 13:41:15 -0700 Subject: [PATCH 1/4] net: fix use-after-free in slab compaction An off-by-one bug results in freeing the incorrect page. Fixes: 3014 --- .github/workflows/ci.yml | 15 ++++++++++++++ tokio/src/util/slab.rs | 43 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d2bcc47d486..c7bc75f8178 100644 --- a/.github/workflows/ci.yml +++ b/.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 +nightly 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 diff --git a/tokio/src/util/slab.rs b/tokio/src/util/slab.rs index 0ab46adcd29..8ba548bf0da 100644 --- a/tokio/src/util/slab.rs +++ b/tokio/src/util/slab.rs @@ -272,7 +272,9 @@ impl Slab { 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 (n, page) in (&self.pages[1..]).iter().enumerate() { + let idx = n + 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 +304,8 @@ impl Slab { // 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()); + // Clear cache self.cached[idx].slots = ptr::null(); self.cached[idx].init = 0; @@ -791,4 +795,41 @@ mod test { } } } + + #[test] + fn issue_3014() { + let mut slab = Slab::::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)); + } + } + } } From 01b2c14cb89876414406072306d008c4453aa744 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 21 Oct 2020 13:56:10 -0700 Subject: [PATCH 2/4] expand debug_assert --- tokio/src/util/slab.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tokio/src/util/slab.rs b/tokio/src/util/slab.rs index 8ba548bf0da..d233ca158ff 100644 --- a/tokio/src/util/slab.rs +++ b/tokio/src/util/slab.rs @@ -304,7 +304,10 @@ impl Slab { // 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()); + 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(); @@ -492,8 +495,11 @@ impl CachedPage { /// Refresh the cache fn refresh(&mut self, page: &Page) { 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 From 9a678ea906f4d66faabb6f89bd13716d2726fd0d Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 21 Oct 2020 14:00:57 -0700 Subject: [PATCH 3/4] style --- tokio/src/util/slab.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tokio/src/util/slab.rs b/tokio/src/util/slab.rs index d233ca158ff..efc72e13365 100644 --- a/tokio/src/util/slab.rs +++ b/tokio/src/util/slab.rs @@ -272,9 +272,7 @@ impl Slab { pub(crate) fn compact(&mut self) { // Iterate each page except the very first one. The very first page is // never freed. - for (n, page) in (&self.pages[1..]).iter().enumerate() { - let idx = n + 1; - + 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. @@ -306,7 +304,9 @@ impl Slab { debug_assert!( self.cached[idx].slots.is_null() || self.cached[idx].slots == vec.as_ptr(), - "cached = {:?}; actual = {:?}", self.cached[idx].slots, vec.as_ptr(), + "cached = {:?}; actual = {:?}", + self.cached[idx].slots, + vec.as_ptr(), ); // Clear cache From c871a0c4eb1e6cc6d9ea993ab4e5bbcada3f32e9 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 21 Oct 2020 14:03:56 -0700 Subject: [PATCH 4/4] try fixing asan ci run --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c7bc75f8178..975ed5c4df7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -122,7 +122,7 @@ jobs: toolchain: ${{ env.nightly }} override: true - name: asan - run: cargo +nightly test --all-features --target x86_64-unknown-linux-gnu --lib -- --test-threads 1 + run: cargo test --all-features --target x86_64-unknown-linux-gnu --lib -- --test-threads 1 working-directory: tokio env: RUSTFLAGS: -Z sanitizer=address