From da52468b34ceacf8a3d74d191f09cf9d8099a0a6 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Thu, 13 Jun 2019 19:36:08 -0700 Subject: [PATCH] tempfile: reimplement using libc::mkdtemp POSIX provides a standard mkdtemp() function to securely create a temporary directory; use it rather than reinventing the wheel. This also drops the dependency of tempfile on rand_ish, which will allow easier use of the tempfile implementation outside of crosvm. BUG=chromium:974059 TEST=cargo test -p tempfile Change-Id: I34a226b046dc6f272106988a78d121a24a377f44 Signed-off-by: Daniel Verkamp Reviewed-on: https://chromium-review.googlesource.com/1659971 Tested-by: kokoro Legacy-Commit-Queue: Commit Bot Reviewed-by: Dylan Reid --- Cargo.lock | 2 +- tempfile/Cargo.toml | 2 +- tempfile/src/lib.rs | 47 +++++++++++++++++++++++++++++---------------- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 844653264c..beaf37b496 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -483,7 +483,7 @@ version = "0.1.0" name = "tempfile" version = "3.0.7" dependencies = [ - "rand_ish 0.1.0", + "libc 0.2.44 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] diff --git a/tempfile/Cargo.toml b/tempfile/Cargo.toml index b12e976119..b7ac1c17a3 100644 --- a/tempfile/Cargo.toml +++ b/tempfile/Cargo.toml @@ -5,4 +5,4 @@ authors = ["The Chromium OS Authors"] edition = "2018" [dependencies] -rand_ish = { path = "../rand_ish" } +libc = "*" diff --git a/tempfile/src/lib.rs b/tempfile/src/lib.rs index 14f235b036..0b9e39bef5 100644 --- a/tempfile/src/lib.rs +++ b/tempfile/src/lib.rs @@ -5,8 +5,9 @@ //! Simplified tempfile which doesn't depend on the `rand` crate, instead using //! /dev/urandom as a source of entropy -use rand_ish::urandom_str; +use libc::mkdtemp; use std::env; +use std::ffi::CString; use std::fs; use std::io::{Error, ErrorKind, Result}; use std::path::{Path, PathBuf}; @@ -35,21 +36,35 @@ impl Builder { /// dropped. /// If the directory can not be created, `Err` is returned. pub fn tempdir(&self) -> Result { - for _ in 0..NUM_RETRIES { - let suffix = urandom_str(12)?; - let path = env::temp_dir().join(format!("{}.{}", self.prefix, suffix)); - - match fs::create_dir(&path) { - Ok(_) => return Ok(TempDir { path }), - Err(ref e) if e.kind() == ErrorKind::AlreadyExists => {} - Err(e) => return Err(e), + // mkdtemp() requires the template to end in 6 X chars, which will be replaced + // with random characters to make the path unique. + let path_template = env::temp_dir().join(format!("{}.XXXXXX", self.prefix)); + let template = match path_template.to_str() { + Some(s) => CString::new(s)?, + None => { + return Err(Error::new( + ErrorKind::InvalidData, + "Path to string conversion failed", + )) } - } - - Err(Error::new( - ErrorKind::AlreadyExists, - "too many tempdirs exist", - )) + }; + let ptr = template.into_raw(); + // Safe because ownership of the buffer is handed off to mkdtemp() only + // until it returns, and ownership is reclaimed by calling CString::from_raw() + // on the same pointer returned by into_raw(). + let path = unsafe { + let ret = mkdtemp(ptr); + let path = CString::from_raw(ptr); + if ret.is_null() { + return Err(Error::last_os_error()); + } + path + }; + Ok(TempDir { + path: PathBuf::from(path.to_str().map_err(|_| { + Error::new(ErrorKind::InvalidData, "Path to string conversion failed") + })?), + }) } } @@ -57,8 +72,6 @@ pub struct TempDir { path: PathBuf, } -const NUM_RETRIES: u32 = 4; - impl TempDir { /// Accesses the tempdir's [`Path`]. ///