mirrors/crosvm
1
0
Fork 0
mirror of https://chromium.googlesource.com/crosvm/crosvm synced 2024-11-28 17:44:10 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

devices: virtio-fs: rework setattr Data::Handle enum

Browse source 
This previously held a raw descriptor (which is just an integer and does
not have an associated lifetime) and an Arc reference to its
corresponding HandleData in an attempt to ensure the HandleData did not
get destroyed while the RawDescriptor was alive. However, newer clippy
versions warn about the unused Arc<HandleDataField>, and this loose
association between the HandleData instance and the descriptor is prone
to being removed by accident since there is no actual lifetime.

To avoid this, keep only a MutexGuard<File> that refers to the file
inside the HandleData, and use a let binding to ensure that the Mutex
itself is alive for the remainder of the function (enforced with actual
Rust lifetimes rather than hopes and dreams).

BUG=b:344974550
TEST=tools/clippy

Change-Id: I4b1f66a2166422a87452fae597c3f08cbdc7dbe5
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5623301
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
This commit is contained in:
Daniel Verkamp 2024-06-11 15:36:42 -07:00 committed by crosvm LUCI
parent 2356c1da35
commit 1ad9fcbab7
1 changed files with 9 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
devices/src/virtio/fs/passthrough.rs
View file

@ -2237,17 +2237,16 @@ impl FileSystem for PassthroughFs {
let _trace = fs_trace!(self.tag, "setattr", inode, handle);
let inode_data = self.find_inode(inode)?;
enum Data {
Handle(Arc<HandleData>, RawDescriptor),
enum Data<'a> {
Handle(MutexGuard<'a, File>),
ProcPath(CString),
}
// If we have a handle then use it otherwise get a new fd from the inode.
let hd;
let data = if let Some(handle) = handle.filter(|&h| h != 0) {
let hd = self.find_handle(handle, inode)?;
let fd = hd.file.lock().as_raw_descriptor();
Data::Handle(hd, fd)
hd = self.find_handle(handle, inode)?;
Data::Handle(hd.file.lock())
} else {
let pathname = CString::new(format!("self/fd/{}", inode_data.as_raw_descriptor()))
.map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?;
@ -2258,7 +2257,7 @@ impl FileSystem for PassthroughFs {
// SAFETY: this doesn't modify any memory and we check the return value.
syscall!(unsafe {
match data {
Data::Handle(_, fd) => libc::fchmod(fd, attr.st_mode),
Data::Handle(ref fd) => libc::fchmod(fd.as_raw_descriptor(), attr.st_mode),
Data::ProcPath(ref p) => {
libc::fchmodat(self.proc.as_raw_descriptor(), p.as_ptr(), attr.st_mode, 0)
}
@ -2298,9 +2297,9 @@ impl FileSystem for PassthroughFs {
if valid.contains(SetattrValid::SIZE) {
syscall!(match data {
Data::Handle(_, fd) => {
Data::Handle(ref fd) => {
// SAFETY: this doesn't modify any memory and we check the return value.
unsafe { libc::ftruncate64(fd, attr.st_size) }
unsafe { libc::ftruncate64(fd.as_raw_descriptor(), attr.st_size) }
}
_ => {
// There is no `ftruncateat` so we need to get a new fd and truncate it.
@ -2340,7 +2339,7 @@ impl FileSystem for PassthroughFs {
// SAFETY: this doesn't modify any memory and we check the return value.
syscall!(unsafe {
match data {
Data::Handle(_, fd) => libc::futimens(fd, tvs.as_ptr()),
Data::Handle(ref fd) => libc::futimens(fd.as_raw_descriptor(), tvs.as_ptr()),
Data::ProcPath(ref p) => {
libc::utimensat(self.proc.as_raw_descriptor(), p.as_ptr(), tvs.as_ptr(), 0)
}