diff --git a/.cargo/config.toml b/.cargo/config.toml index 1f84c3bbfe..455cc2d715 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -16,6 +16,7 @@ rustflags = [ "-Aclippy::needless_bool", "-Aclippy::new-ret-no-self", "-Aclippy::or_fun_call", + "-Aclippy::result_large_err", "-Aclippy::result-unit-err", "-Aclippy::should_implement_trait", "-Aclippy::single_char_pattern", diff --git a/Cargo.lock b/Cargo.lock index de3d7603f8..86d57f76e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -70,6 +70,12 @@ dependencies = [ "thiserror", ] +[[package]] +name = "android_log-sys" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ecc8056bf6ab9892dcd53216c83d1597487d7dacac16c8df6b877d127df9937" + [[package]] name = "anti_tamper" version = "0.1.0" @@ -266,6 +272,7 @@ dependencies = [ name = "base" version = "0.1.0" dependencies = [ + "android_log-sys", "audio_streams", "base_event_token_derive", "cfg-if", diff --git a/Cargo.toml b/Cargo.toml index 038ba8b6cf..a2ab327698 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -435,6 +435,25 @@ all-mingw64 = [ ## All features that are compiled and tested for msvc64 all-msvc64 = [ "all-mingw64" ] +## All features that are compiled and tested for android builds +all-android = [ + "android-sparse", + "audio", + "audio_aaudio", + "balloon", + "config-file", + "gdb", + "gdbstub", + "gdbstub_arch", + "geniezone", + "gunyah", + "libaaudio_stub", + "net", + "qcow", + "usb", + "composite-disk", +] + [dependencies] anyhow = "1.0.32" arch = { path = "arch" } @@ -464,7 +483,7 @@ kernel_cmdline = { path = "kernel_cmdline" } kernel_loader = { path = "kernel_loader" } kvm = { path = "kvm", optional = true } kvm_sys = { path = "kvm_sys", optional = true } -libc = "0.2.93" +libc = "0.2.153" libcras = "*" # Compile out trace statements in release builds log = { version = "0", features = ["release_max_level_debug"]} diff --git a/base/Cargo.toml b/base/Cargo.toml index 646d0d7b1b..f813e7d198 100644 --- a/base/Cargo.toml +++ b/base/Cargo.toml @@ -38,3 +38,6 @@ protobuf = "3.2" rand = "0.8" winapi = "0.3" win_util = { path = "../win_util"} + +[target.'cfg(target_os = "android")'.dependencies] +android_log-sys = "0.3.1" diff --git a/base/src/mmap.rs b/base/src/mmap.rs index 8095135eb8..7e32bde6a5 100644 --- a/base/src/mmap.rs +++ b/base/src/mmap.rs @@ -30,6 +30,12 @@ use crate::VolatileMemoryError; use crate::VolatileMemoryResult; use crate::VolatileSlice; +// TODO: Remove once available in libc bindings +#[cfg(target_os = "android")] +const _SC_LEVEL1_DCACHE_LINESIZE: i32 = 0x0094; +#[cfg(target_os = "linux")] +use libc::_SC_LEVEL1_DCACHE_LINESIZE; + static CACHELINE_SIZE: OnceLock = OnceLock::new(); #[allow(unused_assignments)] @@ -39,7 +45,7 @@ fn get_cacheline_size_once() -> usize { if #[cfg(all(any(target_os = "android", target_os = "linux"), not(target_env = "musl")))] { // SAFETY: // Safe because we check the return value for errors or unsupported requests - let linesize = unsafe { libc::sysconf(libc::_SC_LEVEL1_DCACHE_LINESIZE) }; + let linesize = unsafe { libc::sysconf(_SC_LEVEL1_DCACHE_LINESIZE) }; if linesize > 0 { return linesize as usize; } else { diff --git a/base/src/sys/linux/android/syslog.rs b/base/src/sys/linux/android/syslog.rs index eddc8a26cb..6db5e79238 100644 --- a/base/src/sys/linux/android/syslog.rs +++ b/base/src/sys/linux/android/syslog.rs @@ -4,8 +4,6 @@ //! Implementation of the Syslog trait as a wrapper around Android's logging library, liblog. -extern crate android_log_sys; - use std::ffi::CString; use std::ffi::NulError; use std::mem::size_of; @@ -30,7 +28,7 @@ pub struct PlatformSyslog { impl Syslog for PlatformSyslog { fn new( proc_name: String, - facility: Facility, + _facility: Facility, ) -> Result<(Option>, Option), Error> { Ok((Some(Box::new(Self { proc_name })), None)) } @@ -56,7 +54,7 @@ impl Log for PlatformSyslog { ); } - fn enabled(&self, metadata: &log::Metadata) -> bool { + fn enabled(&self, _metadata: &log::Metadata) -> bool { true } @@ -82,6 +80,7 @@ fn android_log( ) -> Result<(), NulError> { let tag = CString::new(tag)?; let default_pri = LogPriority::VERBOSE; + // SAFETY: `tag` is guaranteed to be valid for duration of the call if unsafe { __android_log_is_loggable(priority as i32, tag.as_ptr(), default_pri as i32) } != 0 { let c_file_name = match file { @@ -99,6 +98,7 @@ fn android_log( line, message: message.as_ptr(), }; + // SAFETY: `log_message` is guaranteed to be valid for duration of the call unsafe { __android_log_write_log_message(&mut log_message) }; } Ok(()) diff --git a/base/src/sys/linux/sched.rs b/base/src/sys/linux/sched.rs index 054faf6b83..9f0e459325 100644 --- a/base/src/sys/linux/sched.rs +++ b/base/src/sys/linux/sched.rs @@ -35,6 +35,7 @@ impl CpuSet { CpuSet(cpuset) } + #[allow(clippy::unnecessary_cast)] pub fn to_cpus(&self) -> Vec { let mut cpus = Vec::new(); for i in 0..(CPU_SETSIZE as usize) { @@ -70,6 +71,7 @@ impl FromIterator for CpuSet { /// # use base::linux::set_cpu_affinity; /// set_cpu_affinity(vec![0, 1, 5, 6]).unwrap(); /// ``` +#[allow(clippy::unnecessary_cast)] pub fn set_cpu_affinity>(cpus: I) -> Result<()> { let CpuSet(cpuset) = cpus .into_iter() diff --git a/devices/src/vfio.rs b/devices/src/vfio.rs index e1fca79f46..1b66f84e36 100644 --- a/devices/src/vfio.rs +++ b/devices/src/vfio.rs @@ -225,7 +225,7 @@ impl KvmVfioPviommu { if ret < 0 { Err(VfioError::KvmSetDeviceAttr(get_error())) } else { - // Safe as we verify the return value. + // SAFETY: Safe as we verify the return value. Ok(unsafe { File::from_raw_descriptor(ret) }) } } diff --git a/devices/src/virtio/vhost/user/device/fs/sys/linux.rs b/devices/src/virtio/vhost/user/device/fs/sys/linux.rs index 7236bb1b86..f32726f994 100644 --- a/devices/src/virtio/vhost/user/device/fs/sys/linux.rs +++ b/devices/src/virtio/vhost/user/device/fs/sys/linux.rs @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -use std::io; use std::path::Path; use std::path::PathBuf; @@ -30,6 +29,7 @@ fn default_gidmap() -> String { format!("{} {} 1", egid, egid) } +#[allow(clippy::unnecessary_cast)] fn jail_and_fork( mut keep_rds: Vec, dir_path: PathBuf, @@ -122,26 +122,26 @@ pub fn start_device(opts: Options) -> anyhow::Result<()> { return Ok(()); } - // We need to set the no setuid fixup secure bit so that we don't drop capabilities when - // changing the thread uid/gid. Without this, creating new entries can fail in some corner - // cases. - const SECBIT_NO_SETUID_FIXUP: i32 = 1 << 2; - // TODO(crbug.com/1199487): Remove this once libc provides the wrapper for all targets. #[cfg(target_os = "linux")] { + // We need to set the no setuid fixup secure bit so that we don't drop capabilities when + // changing the thread uid/gid. Without this, creating new entries can fail in some corner + // cases. + const SECBIT_NO_SETUID_FIXUP: i32 = 1 << 2; + // SAFETY: // Safe because this doesn't modify any memory and we check the return value. let mut securebits = unsafe { libc::prctl(libc::PR_GET_SECUREBITS) }; if securebits < 0 { - bail!(io::Error::last_os_error()); + bail!(std::io::Error::last_os_error()); } securebits |= SECBIT_NO_SETUID_FIXUP; // SAFETY: // Safe because this doesn't modify any memory and we check the return value. let ret = unsafe { libc::prctl(libc::PR_SET_SECUREBITS, securebits) }; if ret < 0 { - bail!(io::Error::last_os_error()); + bail!(std::io::Error::last_os_error()); } } diff --git a/devices/src/virtio/wl.rs b/devices/src/virtio/wl.rs index dc542fe80e..b92d710340 100644 --- a/devices/src/virtio/wl.rs +++ b/devices/src/virtio/wl.rs @@ -72,7 +72,6 @@ use base::Error; use base::Event; use base::EventToken; use base::EventType; -use base::FromRawDescriptor; #[cfg(feature = "gpu")] use base::IntoRawDescriptor; #[cfg(feature = "minigbm")] @@ -464,7 +463,7 @@ struct VmRequester { fn to_safe_descriptor(r: RutabagaDescriptor) -> SafeDescriptor { // SAFETY: // Safe because we own the SafeDescriptor at this point. - unsafe { SafeDescriptor::from_raw_descriptor(r.into_raw_descriptor()) } + unsafe { base::FromRawDescriptor::from_raw_descriptor(r.into_raw_descriptor()) } } impl VmRequester { @@ -1494,7 +1493,9 @@ impl WlState { *descriptor = dup.into_raw_descriptor(); // SAFETY: // Safe because the fd comes from a valid SafeDescriptor. - let file = unsafe { File::from_raw_descriptor(*descriptor) }; + let file: File = unsafe { + base::FromRawDescriptor::from_raw_descriptor(*descriptor) + }; bridged_files.push(file); } Err(_) => return Ok(WlResp::InvalidId), diff --git a/jail/src/helpers.rs b/jail/src/helpers.rs index 8d7be5c507..e5c7f885d0 100644 --- a/jail/src/helpers.rs +++ b/jail/src/helpers.rs @@ -110,6 +110,7 @@ impl Drop for ScopedMinijail { /// /// * `root` - The root path to be changed to by minijail. /// * `max_open_files` - The maximum number of file descriptors to allow a jailed process to open. +#[allow(clippy::unnecessary_cast)] pub fn create_base_minijail(root: &Path, max_open_files: u64) -> Result { // Validate new root directory. Path::is_dir() also checks the existence. if !root.is_dir() { diff --git a/src/crosvm/sys/linux/android.rs b/src/crosvm/sys/linux/android.rs index d1eefa527e..8b93106734 100644 --- a/src/crosvm/sys/linux/android.rs +++ b/src/crosvm/sys/linux/android.rs @@ -4,7 +4,6 @@ #![cfg(target_os = "android")] -use std::ffi::CStr; use std::ffi::CString; use anyhow::anyhow; @@ -22,8 +21,8 @@ extern "C" { } // Apply the listed task profiles to all tasks (current and future) in this process. -pub fn set_process_profiles(profiles: &Vec) -> Result<()> { - if (profiles.is_empty()) { +pub fn set_process_profiles(profiles: &[String]) -> Result<()> { + if profiles.is_empty() { return Ok(()); } let owned: Vec = profiles @@ -34,8 +33,7 @@ pub fn set_process_profiles(profiles: &Vec) -> Result<()> { // SAFETY: the ownership of the array of string is not passed. The function copies it // internally. unsafe { - if (android_set_process_profiles(libc::getuid(), libc::getpid(), ptrs.len(), ptrs.as_ptr())) - { + if android_set_process_profiles(libc::getuid(), libc::getpid(), ptrs.len(), ptrs.as_ptr()) { Ok(()) } else { Err(anyhow!("failed to set task profiles")) diff --git a/src/crosvm/sys/linux/device_helpers.rs b/src/crosvm/sys/linux/device_helpers.rs index 7bba9a8285..cdb9193aa2 100644 --- a/src/crosvm/sys/linux/device_helpers.rs +++ b/src/crosvm/sys/linux/device_helpers.rs @@ -409,7 +409,7 @@ pub fn create_virtio_snd_device( Backend::Sys(virtio::snd::sys::StreamSourceBackend::AAUDIO) => "snd_aaudio_device", #[cfg(feature = "audio_cras")] Backend::Sys(virtio::snd::sys::StreamSourceBackend::CRAS) => "snd_cras_device", - #[cfg(not(feature = "audio_cras"))] + #[cfg(not(any(feature = "audio_cras", feature = "audio_aaudio")))] _ => unreachable!(), }; diff --git a/src/crosvm/sys/linux/vcpu.rs b/src/crosvm/sys/linux/vcpu.rs index c39a72ce6e..3ab3a7daaa 100644 --- a/src/crosvm/sys/linux/vcpu.rs +++ b/src/crosvm/sys/linux/vcpu.rs @@ -37,8 +37,6 @@ use hypervisor::IoParams; use hypervisor::VcpuExit; use hypervisor::VcpuSignalHandle; use libc::c_int; -use libc::SCHED_FLAG_KEEP_ALL; -use libc::SCHED_FLAG_RESET_ON_FORK; use metrics_events::MetricEventType; #[cfg(target_arch = "riscv64")] use riscv64::Riscv64 as Arch; @@ -55,8 +53,12 @@ use super::ExitState; use crate::crosvm::ratelimit::Ratelimit; // TODO(davidai): Import libc constant when updated +const SCHED_FLAG_RESET_ON_FORK: u64 = 0x1; +const SCHED_FLAG_KEEP_POLICY: u64 = 0x08; +const SCHED_FLAG_KEEP_PARAMS: u64 = 0x10; const SCHED_FLAG_UTIL_CLAMP_MIN: u64 = 0x20; const SCHED_SCALE_CAPACITY: u32 = 1024; +const SCHED_FLAG_KEEP_ALL: u64 = SCHED_FLAG_KEEP_POLICY | SCHED_FLAG_KEEP_PARAMS; fn bus_io_handler(bus: &Bus) -> impl FnMut(IoParams) -> Option<[u8; 8]> + '_ { |IoParams { @@ -89,6 +91,7 @@ fn bus_io_handler(bus: &Bus) -> impl FnMut(IoParams) -> Option<[u8; 8]> + '_ { /// Set the VCPU thread affinity and other per-thread scheduler properties. /// This function will be called from each VCPU thread at startup. +#[allow(clippy::unnecessary_cast)] pub fn set_vcpu_thread_scheduling( vcpu_affinity: CpuSet, core_scheduling: bool, diff --git a/third_party/minijail b/third_party/minijail index 13be56d797..76b25b89fc 160000 --- a/third_party/minijail +++ b/third_party/minijail @@ -1 +1 @@ -Subproject commit 13be56d79718425a60173f61f8174669d9cc8930 +Subproject commit 76b25b89fc9035f39df32b59c870cd8111ad5348 diff --git a/tools/clippy b/tools/clippy index 49d88d1413..a65495989a 100755 --- a/tools/clippy +++ b/tools/clippy @@ -30,7 +30,7 @@ def main( chdir(CROSVM_ROOT) # Note: Clippy checks are configured in .cargo/config.toml - common_args = [ + args = [ "--message-format=json" if json else None, "--locked" if locked else None, "--all-targets", @@ -38,22 +38,23 @@ def main( "-Dwarnings", ] if fix: - common_args = [ + args = [ "--fix", "--allow-no-vcs", "--allow-dirty", "--allow-staged", - *common_args, + *args, ] + # For experimental builds, don't clippy the whole workspace, just what's enabled by features. + if triple in (Triple.from_shorthand("riscv64"), Triple.from_shorthand("android")): + args = ["--no-default-features", *args] + else: + args = ["--workspace", *args] + print("Clippy crosvm workspace") - exclude_args = [] - if triple == Triple.from_shorthand("riscv64"): - exclude_args = ["--exclude=" + s for s in DO_NOT_BUILD_RISCV64] clippy( - "--workspace", f"--features={triple.feature_flag}", - *exclude_args, - *common_args, + *args, ).with_envs(triple.get_cargo_env()).fg() diff --git a/tools/custom_checks b/tools/custom_checks index d8bdd0f8ed..17b2efdced 100755 --- a/tools/custom_checks +++ b/tools/custom_checks @@ -128,6 +128,7 @@ def check_rust_features(*files: str): *collect_features("all-mingw64"), *collect_features("all-msvc64"), *collect_features("all-riscv64"), + *collect_features("all-android"), ) ) disabled_features = [ diff --git a/tools/impl/util.py b/tools/impl/util.py index 05d1da3263..2ed5d46b71 100644 --- a/tools/impl/util.py +++ b/tools/impl/util.py @@ -210,6 +210,7 @@ SHORTHANDS = { "aarch64": "aarch64-unknown-linux-gnu", "riscv64": "riscv64gc-unknown-linux-gnu", "x86_64": "x86_64-unknown-linux-gnu", + "android": "aarch64-linux-android", } @@ -285,10 +286,22 @@ class Triple(NamedTuple): env["CARGO_BUILD_TARGET"] = cargo_target env["CARGO_TARGET_DIR"] = str(self.target_dir) env["CROSVM_TARGET_DIR"] = str(crosvm_target_dir()) + # Android builds are not fully supported and can only be used to run clippy. + # Underlying libraries (e.g. minijail) will be built for linux instead + # TODO(denniskempin): This could be better done with [env] in Cargo.toml if it supported + # per-target configuration. See https://github.com/rust-lang/cargo/issues/10273 + if str(self).endswith("-linux-android"): + env["MINIJAIL_DO_NOT_BUILD"] = "true" + env["MINIJAIL_BINDGEN_TARGET"] = f"{self.arch}-unknown-linux-gnu" return env def __str__(self): - return f"{self.arch}-{self.vendor}-{self.sys}-{self.abi}" + parts = [self.arch, self.vendor] + if self.sys: + parts = [*parts, self.sys] + if self.abi: + parts = [*parts, self.abi] + return "-".join(parts) def download_file(url: str, filename: Path, attempts: int = 3): diff --git a/tools/presubmit b/tools/presubmit index 04541a2545..33be5c0acf 100755 --- a/tools/presubmit +++ b/tools/presubmit @@ -5,7 +5,7 @@ import os import typing -from typing import Generator, List, Literal, Optional, Tuple +from typing import Generator, List, Literal, Optional, Tuple, Union from impl.common import ( CROSVM_ROOT, @@ -28,8 +28,11 @@ lucicfg = cmd("third_party/depot_tools/lucicfg") Platform = Literal["x86_64", "aarch64", "mingw64", "armhf"] PLATFORMS: Tuple[Platform, ...] = typing.get_args(Platform) +ClippyOnlyPlatform = Literal["android"] +CLIPPY_ONLY_PLATFORMS: Tuple[ClippyOnlyPlatform, ...] = typing.get_args(ClippyOnlyPlatform) -def platform_is_supported(platform: Platform): + +def platform_is_supported(platform: Union[Platform, ClippyOnlyPlatform]): "Returns true if the platform is available as a target in rustup." triple = Triple.from_shorthand(platform) installed_toolchains = cmd("rustup target list --installed").lines() @@ -158,7 +161,7 @@ def check_crosvm_build( return check -def check_clippy(platform: Platform): +def check_clippy(platform: Union[Platform, ClippyOnlyPlatform]): def check(context: CheckContext): if not platform_is_supported(platform): return None @@ -294,7 +297,7 @@ CHECKS: List[Check] = [ can_fix=True, priority=True, ) - for platform in PLATFORMS + for platform in (*PLATFORMS, *CLIPPY_ONLY_PLATFORMS) ), Check( custom_check("check-copyright-header"), @@ -373,13 +376,14 @@ GROUPS: List[Group] = [ checks=[ "health_checks", *(f"linux_{platform}" for platform in PLATFORMS), + *(f"clippy_{platform}" for platform in CLIPPY_ONLY_PLATFORMS), ], ), # Convenience groups for local usage: Group( name="clippy", doc="Runs clippy for all platforms", - checks=[f"clippy_{platform}" for platform in PLATFORMS], + checks=[f"clippy_{platform}" for platform in PLATFORMS + CLIPPY_ONLY_PLATFORMS], ), Group( name="unit_tests", diff --git a/vm_control/src/lib.rs b/vm_control/src/lib.rs index 67760515cb..5ce6a6f01e 100644 --- a/vm_control/src/lib.rs +++ b/vm_control/src/lib.rs @@ -1628,6 +1628,7 @@ impl VmRequest { /// This does not return a result, instead encapsulating the success or failure in a /// `VmResponse` with the intended purpose of sending the response back over the socket that /// received this `VmRequest`. + #[allow(unused_variables)] pub fn execute( &self, vm: &impl Vm,