From 20a1762b3fd5f1fafead198fd18e469c68683721 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 18 Oct 2023 16:44:33 -0700 Subject: [PATCH] tracing: use ManuallyDrop instead of mem::forget (#2765) 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. --- tracing/src/instrument.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tracing/src/instrument.rs b/tracing/src/instrument.rs index 25edc5e61c..87c74016bb 100644 --- a/tracing/src/instrument.rs +++ b/tracing/src/instrument.rs @@ -5,7 +5,7 @@ use crate::{ use core::{ future::Future, marker::Sized, - mem::{self, ManuallyDrop}, + mem::ManuallyDrop, pin::Pin, task::{Context, Poll}, }; @@ -359,12 +359,11 @@ impl Instrumented { /// /// 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 = &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 = &this.inner; // SAFETY: Those pointers are valid for reads, because `Drop` didn't // run, and properly aligned, because `Instrumented` isn't // `#[repr(packed)]`.