Messages in this thread | | | Date | Mon, 3 Apr 2023 19:56:27 +0200 | From | Alice Ryhl <> | Subject | Re: [PATCH v5 10/15] rust: init: add `stack_pin_init!` macro |
| |
On 4/3/23 18:05, Benno Lossin wrote: > The `stack_pin_init!` macro allows pin-initializing a value on the > stack. It accepts a `impl PinInit<T, E>` to initialize a `T`. It allows > propagating any errors via `?` or handling it normally via `match`. > > Signed-off-by: Benno Lossin <y86-dev@protonmail.com> > Cc: Alice Ryhl <aliceryhl@google.com> > Cc: Andreas Hindborg <a.hindborg@samsung.com> > Cc: Gary Guo <gary@garyguo.net> > ---
If you fix the issue below, then you may add Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> + /// Initializes the contents and returns the result. > + #[inline] > + pub fn init<E>(self: Pin<&mut Self>, init: impl PinInit<T, E>) -> Result<Pin<&mut T>, E> { > + // SAFETY: We never move out of `this`. > + let this = unsafe { Pin::into_inner_unchecked(self) }; > + // The value is currently initialized, so it needs to be dropped before we can reuse > + // the memory (this is a safety guarantee of `Pin`). > + if this.1 { > + // SAFETY: `this.1` is true and we set it to false after this. > + unsafe { this.0.assume_init_drop() }; > + this.1 = false; > + }
This would double-free the value if `assume_init_drop` panics. I know that we configure panics to abort the kernel, but someone could copy this into another codebase and then they would have this issue.
You can fix it by setting `this.1` to false *before* calling `assume_init_drop`.
| |