qcow: check return value of alloc_zeroed and add safety comments

Function alloc_zeroed can fail. Check its return in read and write
functions. Its return value in is_valid_alignment is not checked because
handling error in that case does not give us much benefit. Instead, an
assertion is added.

Add safety comments to all `unsafe`s.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
This commit is contained in:
Wei Liu 2021-11-24 20:53:51 +00:00 committed by Sebastien Boeuf
parent 60b44141b4
commit 9b9015d907

View File

@ -28,8 +28,11 @@ const BLK_ALIGNMENTS: [usize; 2] = [512, 4096];
fn is_valid_alignment(fd: RawFd, alignment: usize) -> bool { fn is_valid_alignment(fd: RawFd, alignment: usize) -> bool {
let layout = Layout::from_size_align(alignment, alignment).unwrap(); let layout = Layout::from_size_align(alignment, alignment).unwrap();
// SAFETY: layout has non-zero size
let ptr = unsafe { alloc_zeroed(layout) }; let ptr = unsafe { alloc_zeroed(layout) };
assert!(!ptr.is_null());
// SAFETY: FFI call
let ret = unsafe { let ret = unsafe {
::libc::pread( ::libc::pread(
fd, fd,
@ -39,6 +42,7 @@ fn is_valid_alignment(fd: RawFd, alignment: usize) -> bool {
) )
}; };
// SAFETY: ptr was allocated by alloc_zeroed with layout
unsafe { dealloc(ptr, layout) }; unsafe { dealloc(ptr, layout) };
ret >= 0 ret >= 0
@ -141,11 +145,18 @@ impl Read for RawFile {
.unwrap(); .unwrap();
let layout = Layout::from_size_align(rounded_len, self.alignment).unwrap(); let layout = Layout::from_size_align(rounded_len, self.alignment).unwrap();
// SAFETY: layout has non-zero size
let tmp_ptr = unsafe { alloc_zeroed(layout) }; let tmp_ptr = unsafe { alloc_zeroed(layout) };
if tmp_ptr.is_null() {
return Err(io::Error::last_os_error());
}
// SAFETY: tmp_ptr is valid and at least rounded_len long
let tmp_buf = unsafe { slice::from_raw_parts_mut(tmp_ptr, rounded_len) }; let tmp_buf = unsafe { slice::from_raw_parts_mut(tmp_ptr, rounded_len) };
// This can eventually replaced with read_at once its interface // This can eventually replaced with read_at once its interface
// has been stabilized. // has been stabilized.
// SAFETY: FFI call. All parameters are valid.
let ret = unsafe { let ret = unsafe {
::libc::pread64( ::libc::pread64(
self.file.as_raw_fd(), self.file.as_raw_fd(),
@ -155,12 +166,14 @@ impl Read for RawFile {
) )
}; };
if ret < 0 { if ret < 0 {
// SAFETY: tmp_ptr was allocated by alloc_zeroed with layout
unsafe { dealloc(tmp_ptr, layout) }; unsafe { dealloc(tmp_ptr, layout) };
return Err(io::Error::last_os_error()); return Err(io::Error::last_os_error());
} }
let read: usize = ret.try_into().unwrap(); let read: usize = ret.try_into().unwrap();
if read < file_offset { if read < file_offset {
// SAFETY: tmp_ptr was allocated by alloc_zeroed with layout
unsafe { dealloc(tmp_ptr, layout) }; unsafe { dealloc(tmp_ptr, layout) };
return Ok(0); return Ok(0);
} }
@ -171,6 +184,7 @@ impl Read for RawFile {
} }
buf.copy_from_slice(&tmp_buf[file_offset..(file_offset + buf_len)]); buf.copy_from_slice(&tmp_buf[file_offset..(file_offset + buf_len)]);
// SAFETY: tmp_ptr was allocated by alloc_zeroed with layout
unsafe { dealloc(tmp_ptr, layout) }; unsafe { dealloc(tmp_ptr, layout) };
self.seek(SeekFrom::Current(to_copy.try_into().unwrap())) self.seek(SeekFrom::Current(to_copy.try_into().unwrap()))
@ -211,11 +225,17 @@ impl Write for RawFile {
.unwrap(); .unwrap();
let layout = Layout::from_size_align(rounded_len, self.alignment).unwrap(); let layout = Layout::from_size_align(rounded_len, self.alignment).unwrap();
// SAFETY: layout has non-zero size
let tmp_ptr = unsafe { alloc_zeroed(layout) }; let tmp_ptr = unsafe { alloc_zeroed(layout) };
if tmp_ptr.is_null() {
return Err(io::Error::last_os_error());
}
let tmp_buf = unsafe { slice::from_raw_parts_mut(tmp_ptr, rounded_len) }; let tmp_buf = unsafe { slice::from_raw_parts_mut(tmp_ptr, rounded_len) };
// This can eventually replaced with read_at once its interface // This can eventually replaced with read_at once its interface
// has been stabilized. // has been stabilized.
// SAFETY: FFI call
let ret = unsafe { let ret = unsafe {
::libc::pread64( ::libc::pread64(
self.file.as_raw_fd(), self.file.as_raw_fd(),
@ -225,6 +245,7 @@ impl Write for RawFile {
) )
}; };
if ret < 0 { if ret < 0 {
// SAFETY: tmp_ptr was allocated by alloc_zeroed with layout
unsafe { dealloc(tmp_ptr, layout) }; unsafe { dealloc(tmp_ptr, layout) };
return Err(io::Error::last_os_error()); return Err(io::Error::last_os_error());
}; };
@ -233,6 +254,7 @@ impl Write for RawFile {
// This can eventually replaced with write_at once its interface // This can eventually replaced with write_at once its interface
// has been stabilized. // has been stabilized.
// SAFETY: FFI call
let ret = unsafe { let ret = unsafe {
::libc::pwrite64( ::libc::pwrite64(
self.file.as_raw_fd(), self.file.as_raw_fd(),
@ -242,6 +264,7 @@ impl Write for RawFile {
) )
}; };
// SAFETY: tmp_ptr was allocated by alloc_zeroed with layout
unsafe { dealloc(tmp_ptr, layout) }; unsafe { dealloc(tmp_ptr, layout) };
if ret < 0 { if ret < 0 {