While reviewing the vmm-sys-util Rust crate, it was noticed that on a very rare occasion the memory region of a CString depicting a temporary file name does not get freed after usage resulting in a memory leak situation.
According to the Rust documentation, a call to std::ffi::CString::into_raw(), which transfers ownership of the string to a C caller, must be accompanied by a final call to std::ffi::CString::from_raw() which retakes the ownership again. Failure in doing so results in a memory leak.
In the code snippet shown in the below listing, ownership of a CString is transferred in (1) through calling into_raw() to C and returned in (3) via from_raw() to Rust. In case of an error in mkstemp() in (2), the function is exited without calling from_raw() leading to the memory leak. According to the documentation of mkstemp() this can only happen if there already exists a file which happens to have the same file name as the temporary file.
pub fn new_with_prefix<P: AsRef<OsStr>>(prefix: P) -> Result<TempFile> {
use std::ffi::CString;
use std::os::unix::{ffi::OsStrExt, io::FromRawFd};
let mut os_fname = prefix.as_ref().to_os_string();
os_fname.push("XXXXXX");
let raw_fname = match CString::new(os_fname.as_bytes()) {
(1) Ok(c_string) => c_string.into_raw(),
Err(_) => return Err(Error::new(libc::EINVAL)),
};
// SAFETY: Safe because `raw_fname` originates from CString::into_raw, meaning
// it is a pointer to a nul-terminated sequence of characters.
let fd = unsafe { libc::mkstemp(raw_fname) };
if fd == -1 {
(2) return errno_result();
}
// SAFETY: raw_fname originates from a call to CString::into_raw. The length
// of the string has not changed, as mkstemp returns a valid file name, and
// '\0' cannot be part of a valid filename.
(3) let c_tempname = unsafe { CString::from_raw(raw_fname) };
let os_tempname = OsStr::from_bytes(c_tempname.as_bytes());
// SAFETY: Safe because we checked `fd != -1` above and we uniquely own the file
// descriptor. This `fd` will be freed etc when `File` and thus
// `TempFile` goes out of scope.
let file = unsafe { File::from_raw_fd(fd) };
Ok(TempFile {
path: PathBuf::from(os_tempname),
file: Some(file),
})
}
X41 advises calling std::ffi::CString::from_raw() before returning from the function to avoid memory leaks. This also affects other types such as std::ffi::OsString or std::boxed::Box.
From @y-x41
While reviewing the vmm-sys-util Rust crate, it was noticed that on a very rare occasion the memory region of a CString depicting a temporary file name does not get freed after usage resulting in a memory leak situation.
According to the Rust documentation, a call to std::ffi::CString::into_raw(), which transfers ownership of the string to a C caller, must be accompanied by a final call to std::ffi::CString::from_raw() which retakes the ownership again. Failure in doing so results in a memory leak.
In the code snippet shown in the below listing, ownership of a CString is transferred in (1) through calling into_raw() to C and returned in (3) via from_raw() to Rust. In case of an error in mkstemp() in (2), the function is exited without calling from_raw() leading to the memory leak. According to the documentation of mkstemp() this can only happen if there already exists a file which happens to have the same file name as the temporary file.
X41 advises calling std::ffi::CString::from_raw() before returning from the function to avoid memory leaks. This also affects other types such as std::ffi::OsString or std::boxed::Box.