Skip to content

Commit cbced4a

Browse files
committed
Allow constructing MmapRegions for non-seekable fds
vm-memory validates that the file,offset,len combination passed to a MmapRegionBuilder is actually in bounds for the actual size of the file. It uses `lseek(2)` for this, due to #195, however not all file descriptors that can be mmap'd can also be lseek'd. Thus, if lseek is not supported on a fd, fall back to using `fstat(2)` instead of lseek. This allows mmap-ing things like guest_memfd, which does not support seeking. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
1 parent 3a0b31d commit cbced4a

File tree

4 files changed

+73
-4
lines changed

4 files changed

+73
-4
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
### Added
66

77
- \[[#311](https://github.com/rust-vmm/vm-memory/pull/311)\] Allow compiling without the ReadVolatile and WriteVolatile implementations
8+
- \[[#319](https://github.com/rust-vmm/vm-memory/pull/319)\] Allow constructing `MmapRegion`s
9+
mapping file descriptors that cannot be seeked, such as `guest_memfd`.
810

911
### Changed
1012

src/mmap.rs

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,28 @@ pub fn check_file_offset(
8484
let start = file_offset.start();
8585

8686
if let Some(end) = start.checked_add(size as u64) {
87-
let filesize = file
88-
.seek(SeekFrom::End(0))
89-
.map_err(MmapRegionError::SeekEnd)?;
90-
file.rewind().map_err(MmapRegionError::SeekStart)?;
87+
// If f_ops->llseek is not implemented in the kernel for some file, we get -ESPIPE
88+
// In this scenario, fall back to using fstat. We need to prefer lseek in all other
89+
// cases because of https://github.com/rust-vmm/vm-memory/issues/195
90+
// clippy wants us to use file.stream_position() here, but that obscures what we're trying
91+
// to do here.
92+
#[allow(clippy::seek_from_current)]
93+
let seek_unsupported = file
94+
.seek(SeekFrom::Current(0))
95+
.err()
96+
.map(|err| err.raw_os_error() == Some(libc::ESPIPE))
97+
.unwrap_or(true);
98+
99+
let filesize = if seek_unsupported {
100+
file.metadata().map_err(MmapRegionError::Stat)?.len()
101+
} else {
102+
let filesize = file
103+
.seek(SeekFrom::End(0))
104+
.map_err(MmapRegionError::SeekEnd)?;
105+
file.rewind().map_err(MmapRegionError::SeekStart)?;
106+
filesize
107+
};
108+
91109
if filesize < end {
92110
return Err(MmapRegionError::MappingPastEof);
93111
}
@@ -522,6 +540,7 @@ mod tests {
522540

523541
use std::io::Write;
524542
use std::mem;
543+
use std::os::fd::FromRawFd;
525544
#[cfg(feature = "rawfd")]
526545
use std::{fs::File, path::Path};
527546
use vmm_sys_util::tempfile::TempFile;
@@ -1396,4 +1415,46 @@ mod tests {
13961415
.unwrap()
13971416
});
13981417
}
1418+
1419+
#[test]
1420+
#[cfg(unix)]
1421+
#[cfg(not(miri))] /* memfd_secret isn't support by miri */
1422+
#[allow(clippy::seek_from_current)]
1423+
fn test_non_seekable_file() {
1424+
let fd = unsafe { libc::syscall(libc::SYS_memfd_secret, 0) } as libc::c_int;
1425+
if fd < 0 {
1426+
if std::io::Error::last_os_error().raw_os_error() == Some(libc::ENOSYS) {
1427+
// memfd_secret is not enabled by default, so skip this test if that's the case
1428+
return;
1429+
}
1430+
1431+
panic!("Creation of memfd_secret failed!");
1432+
}
1433+
1434+
let r = unsafe { libc::ftruncate(fd, 0x1000) };
1435+
assert_eq!(r, 0);
1436+
1437+
let mut file = unsafe { File::from_raw_fd(fd as _) };
1438+
1439+
// validate the memfd_secret indeed cannot be seeked, but can be mapped
1440+
assert_eq!(
1441+
file.seek(SeekFrom::Current(0))
1442+
.unwrap_err()
1443+
.raw_os_error()
1444+
.unwrap(),
1445+
libc::ESPIPE
1446+
);
1447+
1448+
let file = Arc::new(file);
1449+
let fo = FileOffset::from_arc(Arc::clone(&file), 0);
1450+
1451+
check_file_offset(&fo, 0x1000).unwrap();
1452+
1453+
let fo = FileOffset::from_arc(Arc::clone(&file), 0x1000);
1454+
1455+
assert!(matches!(
1456+
check_file_offset(&fo, 0x1000),
1457+
Err(MmapRegionError::MappingPastEof)
1458+
));
1459+
}
13991460
}

src/mmap_unix.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ pub enum Error {
4747
/// Seeking the start of the file returned an error.
4848
#[error("Error seeking the start of the file: {0}")]
4949
SeekStart(io::Error),
50+
/// Calling [`File::metadata`] returned an error.
51+
#[error("Error determining size of file using fstat: {0}")]
52+
Stat(io::Error),
5053
}
5154

5255
pub type Result<T> = result::Result<T, Error>;

src/mmap_xen.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ pub enum Error {
5050
/// Seeking the start of the file returned an error.
5151
#[error("Error seeking the start of the file: {0}")]
5252
SeekStart(io::Error),
53+
/// Calling [`File::metadata`] returned an error.
54+
#[error("Error determining size of file using fstat: {0}")]
55+
Stat(io::Error),
5356
/// Invalid file offset.
5457
#[error("Invalid file offset")]
5558
InvalidFileOffset,

0 commit comments

Comments
 (0)