From 205d62b30125b60b225599efb0853da9d740c1f7 Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Sun, 14 Jan 2024 16:17:17 +0100 Subject: [PATCH] Fix memory leak in ReceiveSignals Previously it would only deallocate the signal info allocation if the operation was still running, not it also deallocates if the operation has not been started or already has completed. --- src/signals.rs | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/signals.rs b/src/signals.rs index 7180a3d..f8c5b2d 100644 --- a/src/signals.rs +++ b/src/signals.rs @@ -304,28 +304,31 @@ impl fmt::Debug for ReceiveSignals { impl Drop for ReceiveSignals { fn drop(&mut self) { - if let OpState::Running(op_index) = self.state { - // Only drop the signal `info` field once we know the operation has - // finished, otherwise the kernel might write into memory we have - // deallocated. - // SAFETY: we're in the `Drop` implementation, so `self.info` can't - // be used anymore making it safe to take ownership. - let resource = unsafe { ManuallyDrop::take(&mut self.info) }; - let result = self - .signals - .fd - .sq - .cancel_op(op_index, resource, |submission| unsafe { - submission.cancel_op(op_index); - // We'll get a canceled completion event if we succeeded, which - // is sufficient to cleanup the operation. - submission.no_completion_event(); - }); - if let Err(err) = result { - log::error!( + let signal_info = unsafe { ManuallyDrop::take(&mut self.info) }; + match self.state { + OpState::Running(op_index) => { + // Only drop the signal `info` field once we know the operation has + // finished, otherwise the kernel might write into memory we have + // deallocated. + // SAFETY: we're in the `Drop` implementation, so `self.info` can't + // be used anymore making it safe to take ownership. + let result = + self.signals + .fd + .sq + .cancel_op(op_index, signal_info, |submission| unsafe { + submission.cancel_op(op_index); + // We'll get a canceled completion event if we succeeded, which + // is sufficient to cleanup the operation. + submission.no_completion_event(); + }); + if let Err(err) = result { + log::error!( "dropped a10::ReceiveSignals before canceling it, attempt to cancel failed: {err}" ); + } } + OpState::NotStarted(()) | OpState::Done => drop(signal_info), } } }