Skip to content

Commit

Permalink
tracing: use ManuallyDrop instead of mem::forget (#2765)
Browse files Browse the repository at this point in the history
The current code is UB and LLVM could choose to reuse the stack slot causing a UAF.

## Motivation

UB is bad.

## Solution

Don't do that.
  • Loading branch information
Manishearth authored and hawkw committed Oct 19, 2023
1 parent 4b99457 commit 20a1762
Showing 1 changed file with 6 additions and 7 deletions.
13 changes: 6 additions & 7 deletions tracing/src/instrument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
use core::{
future::Future,
marker::Sized,
mem::{self, ManuallyDrop},
mem::ManuallyDrop,
pin::Pin,
task::{Context, Poll},
};
Expand Down Expand Up @@ -359,12 +359,11 @@ impl<T> Instrumented<T> {
///
/// Note that this drops the span.
pub fn into_inner(self) -> T {
// To manually destructure `Instrumented` without `Drop`, we save
// pointers to the fields and use `mem::forget` to leave those pointers
// valid.
let span: *const Span = &self.span;
let inner: *const ManuallyDrop<T> = &self.inner;
mem::forget(self);
// To manually destructure `Instrumented` without `Drop`, we
// move it into a ManuallyDrop and use pointers to its fields
let this = ManuallyDrop::new(self);
let span: *const Span = &this.span;
let inner: *const ManuallyDrop<T> = &this.inner;
// SAFETY: Those pointers are valid for reads, because `Drop` didn't
// run, and properly aligned, because `Instrumented` isn't
// `#[repr(packed)]`.
Expand Down

0 comments on commit 20a1762

Please # to comment.