qcow_utils: do not close given fds in `convert_to_*` functions

The `convert_to_*` functions take ownership of the passed FDs even
though they should not according to the function's contract. This change
clones the passed FDs so that the caller can retain ownership of its
FDs.

This change also wraps most of the implementations in catch_unwind so
that panics do not unwind past FFI boundaries, which is undefined
behavior.

BUG=chromium:905799
TEST=in crosh: `vmc export <vm name> <file name>`

Change-Id: I2f65ebff51243675d0854574d8fd02cec1b237a4
Reviewed-on: https://chromium-review.googlesource.com/1338501
Commit-Ready: Zach Reizner <zachr@chromium.org>
Tested-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>

1 files changed, 33 insertions, 7 deletions

diff --git a/qcow_utils/src/qcow_utils.rs b/qcow_utils/src/qcow_utils.rs
index ea84b62..33f20ae 100644
--- a/qcow_utils/src/qcow_utils.rs
+++ b/qcow_utils/src/qcow_utils.rs
@@ -7,11 +7,13 @@
 extern crate libc;
 extern crate qcow;
 
-use libc::{EINVAL, EIO};
+use libc::{EBADFD, EINVAL, EIO};
 use std::ffi::CStr;
 use std::fs::{File, OpenOptions};
+use std::mem::forget;
 use std::os::raw::{c_char, c_int};
 use std::os::unix::io::FromRawFd;
+use std::panic::catch_unwind;
 
 use qcow::{ImageType, QcowFile};
 
@@ -47,23 +49,47 @@ pub unsafe extern "C" fn create_qcow_with_size(path: *const c_char, virtual_size
 #[no_mangle]
 pub unsafe extern "C" fn convert_to_qcow2(src_fd: c_int, dst_fd: c_int) -> c_int {
     // The caller is responsible for passing valid file descriptors.
+    // The caller retains ownership of the file descriptors.
     let src_file = File::from_raw_fd(src_fd);
+    let src_file_owned = src_file.try_clone();
+    forget(src_file);
+
     let dst_file = File::from_raw_fd(dst_fd);
+    let dst_file_owned = dst_file.try_clone();
+    forget(dst_file);
 
-    match qcow::convert(src_file, dst_file, ImageType::Qcow2) {
-        Ok(_) => 0,
-        Err(_) => -EIO,
+    match (src_file_owned, dst_file_owned) {
+        (Ok(src_file), Ok(dst_file)) => {
+            catch_unwind(
+                || match qcow::convert(src_file, dst_file, ImageType::Qcow2) {
+                    Ok(_) => 0,
+                    Err(_) => -EIO,
+                },
+            ).unwrap_or(-EIO)
+        }
+        _ => -EBADFD,
     }
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn convert_to_raw(src_fd: c_int, dst_fd: c_int) -> c_int {
     // The caller is responsible for passing valid file descriptors.
+    // The caller retains ownership of the file descriptors.
     let src_file = File::from_raw_fd(src_fd);
+    let src_file_owned = src_file.try_clone();
+    forget(src_file);
+
     let dst_file = File::from_raw_fd(dst_fd);
+    let dst_file_owned = dst_file.try_clone();
+    forget(dst_file);
 
-    match qcow::convert(src_file, dst_file, ImageType::Raw) {
-        Ok(_) => 0,
-        Err(_) => -EIO,
+    match (src_file_owned, dst_file_owned) {
+        (Ok(src_file), Ok(dst_file)) => {
+            catch_unwind(|| match qcow::convert(src_file, dst_file, ImageType::Raw) {
+                Ok(_) => 0,
+                Err(_) => -EIO,
+            }).unwrap_or(-EIO)
+        }
+        _ => -EBADFD,
     }
 }