From b8626063f066fad6eb1e518aaebd11db0be9b577 Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Mon, 7 Dec 2020 15:39:36 +0200 Subject: [PATCH 1/4] update Rust toolchain to version 1.46.0 Signed-off-by: Adrian Catangiu --- tools/devctr/Dockerfile.aarch64 | 2 +- tools/devctr/Dockerfile.x86_64 | 7 +++---- tools/devtool | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/tools/devctr/Dockerfile.aarch64 b/tools/devctr/Dockerfile.aarch64 index 8faae953c6c..a91017089f0 100644 --- a/tools/devctr/Dockerfile.aarch64 +++ b/tools/devctr/Dockerfile.aarch64 @@ -4,7 +4,7 @@ FROM ubuntu:18.04 # The Rust toolchain layer will get updated most frequently, but we could keep the system # dependencies layer intact for much longer. -ARG RUST_TOOLCHAIN="1.43.1" +ARG RUST_TOOLCHAIN="1.46.0" ARG TINI_VERSION_TAG="v0.18.0" ARG TMP_BUILD_DIR=/tmp/build ARG FIRECRACKER_SRC_DIR="/firecracker" diff --git a/tools/devctr/Dockerfile.x86_64 b/tools/devctr/Dockerfile.x86_64 index f092811740d..1f2db669def 100644 --- a/tools/devctr/Dockerfile.x86_64 +++ b/tools/devctr/Dockerfile.x86_64 @@ -4,8 +4,7 @@ FROM ubuntu:18.04 # The Rust toolchain layer will get updated most frequently, but we could keep the system # dependencies layer intact for much longer. -ARG RUST_TOOLCHAIN="1.43.1" -ARG RUST_TOOLCHAIN_AUDIT="1.47.0" +ARG RUST_TOOLCHAIN="1.46.0" ARG TINI_VERSION_TAG="v0.18.0" ARG TMP_BUILD_DIR=/tmp/build ARG FIRECRACKER_SRC_DIR="/firecracker" @@ -78,10 +77,10 @@ RUN mkdir "$TMP_BUILD_DIR" \ && rustup target add x86_64-unknown-linux-musl \ && rustup component add rustfmt \ && rustup component add clippy-preview \ - && rustup install "$RUST_TOOLCHAIN_AUDIT" \ + && rustup install "stable" \ && cd "$TMP_BUILD_DIR" \ && cargo install cargo-kcov \ - && cargo +"$RUST_TOOLCHAIN_AUDIT" install cargo-audit \ + && cargo +"stable" install cargo-audit \ && cargo kcov --print-install-kcov-sh | sh \ && rm -rf "$CARGO_HOME/registry" \ && ln -s "$CARGO_REGISTRY_DIR" "$CARGO_HOME/registry" \ diff --git a/tools/devtool b/tools/devtool index 7d8313a0d5e..d619111b6d5 100755 --- a/tools/devtool +++ b/tools/devtool @@ -73,7 +73,7 @@ # Development container image (name:tag) # This should be updated whenever we upgrade the development container. # (Yet another step on our way to reproducible builds.) -DEVCTR_IMAGE="fcuvm/dev:v23" +DEVCTR_IMAGE="fcuvm/dev:v24" # Naming things is hard MY_NAME="Firecracker $(basename "$0")" From b7659e58f5e351b33c181ff7fea9f2caf55072a1 Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Mon, 7 Dec 2020 15:40:30 +0200 Subject: [PATCH 2/4] fix various clippy lints Signed-off-by: Adrian Catangiu --- src/arch/src/aarch64/fdt.rs | 2 +- src/arch/src/x86_64/msr.rs | 5 +- src/devices/src/virtio/balloon/utils.rs | 7 +- src/devices/src/virtio/block/request.rs | 80 +++++++++++----------- src/devices/src/virtio/net/device.rs | 3 +- src/devices/src/virtio/vsock/unix/muxer.rs | 8 +-- src/dumbo/src/tcp/connection.rs | 10 +-- src/kernel/src/cmdline/mod.rs | 5 +- src/utils/src/arg_parser.rs | 5 +- src/vmm/src/default_syscalls/filters.rs | 2 +- 10 files changed, 53 insertions(+), 74 deletions(-) diff --git a/src/arch/src/aarch64/fdt.rs b/src/arch/src/aarch64/fdt.rs index 6eef3888bdf..fdea867164c 100644 --- a/src/arch/src/aarch64/fdt.rs +++ b/src/arch/src/aarch64/fdt.rs @@ -519,7 +519,7 @@ fn create_devices_node false, - _ => true, - }; + let should = !matches!(msr, MSR_IA32_FEATURE_CONTROL | MSR_IA32_MCG_CTL); assert_eq!(msr_should_serialize(msr), should); } } diff --git a/src/devices/src/virtio/balloon/utils.rs b/src/devices/src/virtio/balloon/utils.rs index 16a302bc305..0cbd5092872 100644 --- a/src/devices/src/virtio/balloon/utils.rs +++ b/src/devices/src/virtio/balloon/utils.rs @@ -18,7 +18,7 @@ pub(crate) fn compact_page_frame_numbers(v: &mut Vec) -> Vec<(u32, u32)> { // received at once from a single descriptor is `MAX_PAGES_IN_DESC`, // this sort does not change the complexity of handling // an inflation. - v.sort(); + v.sort_unstable(); // Since there are at most `MAX_PAGES_IN_DESC` pages, setting the // capacity of `result` to this makes sense. @@ -108,10 +108,7 @@ mod tests { /// This asserts that $lhs matches $rhs. macro_rules! assert_match { ($lhs:expr, $rhs:pat) => {{ - assert!(match $lhs { - $rhs => true, - _ => false, - }) + assert!(matches!($lhs, $rhs)) }}; } diff --git a/src/devices/src/virtio/block/request.rs b/src/devices/src/virtio/block/request.rs index 32b3e9b095b..5f878724aed 100644 --- a/src/devices/src/virtio/block/request.rs +++ b/src/devices/src/virtio/block/request.rs @@ -344,20 +344,20 @@ mod tests { let request_header = RequestHeader::new(VIRTIO_BLK_T_OUT, 114); m.write_obj::(request_header, GuestAddress(0x1000)) .unwrap(); - assert!(match Request::parse(&q.pop(m).unwrap(), m) { - Err(Error::UnexpectedWriteOnlyDescriptor) => true, - _ => false, - }); + assert!(matches!( + Request::parse(&q.pop(m).unwrap(), m), + Err(Error::UnexpectedWriteOnlyDescriptor) + )); } { let mut q = vq.create_queue(); // Chain too short: no data_descriptor. vq.dtable[request_type_descriptor].flags.set(0); - assert!(match Request::parse(&q.pop(m).unwrap(), m) { - Err(Error::DescriptorChainTooShort) => true, - _ => false, - }); + assert!(matches!( + Request::parse(&q.pop(m).unwrap(), m), + Err(Error::DescriptorChainTooShort) + )); } { @@ -367,10 +367,10 @@ mod tests { .flags .set(VIRTQ_DESC_F_NEXT); vq.dtable[data_descriptor].set(0x2000, 0x1000, 0, 2); - assert!(match Request::parse(&q.pop(m).unwrap(), m) { - Err(Error::DescriptorChainTooShort) => true, - _ => false, - }); + assert!(matches!( + Request::parse(&q.pop(m).unwrap(), m), + Err(Error::DescriptorChainTooShort) + )); } { @@ -380,10 +380,10 @@ mod tests { .flags .set(VIRTQ_DESC_F_NEXT | VIRTQ_DESC_F_WRITE); vq.dtable[status_descriptor].set(0x3000, 0, 0, 0); - assert!(match Request::parse(&q.pop(m).unwrap(), m) { - Err(Error::UnexpectedWriteOnlyDescriptor) => true, - _ => false, - }); + assert!(matches!( + Request::parse(&q.pop(m).unwrap(), m), + Err(Error::UnexpectedWriteOnlyDescriptor) + )); } { @@ -392,10 +392,10 @@ mod tests { m.write_obj::(VIRTIO_BLK_T_GET_ID, GuestAddress(0x1000)) .unwrap(); vq.dtable[data_descriptor].flags.set(VIRTQ_DESC_F_NEXT); - assert!(match Request::parse(&q.pop(m).unwrap(), m) { - Err(Error::UnexpectedReadOnlyDescriptor) => true, - _ => false, - }); + assert!(matches!( + Request::parse(&q.pop(m).unwrap(), m), + Err(Error::UnexpectedReadOnlyDescriptor) + )); } { @@ -404,10 +404,10 @@ mod tests { m.write_obj::(VIRTIO_BLK_T_IN, GuestAddress(0x1000)) .unwrap(); vq.dtable[data_descriptor].flags.set(VIRTQ_DESC_F_NEXT); - assert!(match Request::parse(&q.pop(m).unwrap(), m) { - Err(Error::UnexpectedReadOnlyDescriptor) => true, - _ => false, - }); + assert!(matches!( + Request::parse(&q.pop(m).unwrap(), m), + Err(Error::UnexpectedReadOnlyDescriptor) + )); } { @@ -416,20 +416,20 @@ mod tests { vq.dtable[data_descriptor] .flags .set(VIRTQ_DESC_F_NEXT | VIRTQ_DESC_F_WRITE); - assert!(match Request::parse(&q.pop(m).unwrap(), m) { - Err(Error::UnexpectedReadOnlyDescriptor) => true, - _ => false, - }); + assert!(matches!( + Request::parse(&q.pop(m).unwrap(), m), + Err(Error::UnexpectedReadOnlyDescriptor) + )); } { let mut q = vq.create_queue(); // Status descriptor too small. vq.dtable[status_descriptor].flags.set(VIRTQ_DESC_F_WRITE); - assert!(match Request::parse(&q.pop(m).unwrap(), m) { - Err(Error::DescriptorLengthTooSmall) => true, - _ => false, - }); + assert!(matches!( + Request::parse(&q.pop(m).unwrap(), m), + Err(Error::DescriptorLengthTooSmall) + )); } { @@ -440,10 +440,10 @@ mod tests { vq.dtable[status_descriptor] .addr .set(m.last_addr().raw_value()); - assert!(match Request::parse(&q.pop(m).unwrap(), m) { - Err(Error::GuestMemory(GuestMemoryError::InvalidGuestAddress(_))) => true, - _ => false, - }); + assert!(matches!( + Request::parse(&q.pop(m).unwrap(), m), + Err(Error::GuestMemory(GuestMemoryError::InvalidGuestAddress(_))) + )); } { @@ -454,10 +454,10 @@ mod tests { vq.dtable[data_descriptor] .addr .set(m.last_addr().raw_value()); - assert!(match Request::parse(&q.pop(m).unwrap(), m) { - Err(Error::GuestMemory(GuestMemoryError::InvalidGuestAddress(_))) => true, - _ => false, - }); + assert!(matches!( + Request::parse(&q.pop(m).unwrap(), m), + Err(Error::GuestMemory(GuestMemoryError::InvalidGuestAddress(_))) + )); } { diff --git a/src/devices/src/virtio/net/device.rs b/src/devices/src/virtio/net/device.rs index f69b95a6a0c..dd0c6ec4225 100644 --- a/src/devices/src/virtio/net/device.rs +++ b/src/devices/src/virtio/net/device.rs @@ -404,11 +404,10 @@ impl Net { // Check for guest MAC spoofing. if let Some(mac) = guest_mac { - let _ = EthernetFrame::from_bytes(checked_frame(frame_buf)?).and_then(|eth_frame| { + let _ = EthernetFrame::from_bytes(checked_frame(frame_buf)?).map(|eth_frame| { if mac != eth_frame.src_mac() { METRICS.net.tx_spoofed_mac_count.inc(); } - Ok(()) }); } diff --git a/src/devices/src/virtio/vsock/unix/muxer.rs b/src/devices/src/virtio/vsock/unix/muxer.rs index e813a5a6a1c..bb37ac56fb9 100644 --- a/src/devices/src/virtio/vsock/unix/muxer.rs +++ b/src/devices/src/virtio/vsock/unix/muxer.rs @@ -391,7 +391,7 @@ impl VsockMuxer { Some(EpollListener::LocalStream(_)) => { if let Some(EpollListener::LocalStream(mut stream)) = self.remove_listener(fd) { Self::read_local_stream_port(&mut stream) - .and_then(|peer_port| Ok((self.allocate_local_port(), peer_port))) + .map(|peer_port| (self.allocate_local_port(), peer_port)) .and_then(|(local_port, peer_port)| { self.add_connection( ConnMapKey { @@ -487,7 +487,7 @@ impl VsockMuxer { evset: conn.get_polled_evset(), }, ) - .and_then(|_| { + .map(|_| { if conn.has_pending_rx() { // We can safely ignore any error in adding a connection RX indication. Worst // case scenario, the RX queue will get desynchronized, but we'll handle that @@ -496,7 +496,6 @@ impl VsockMuxer { } self.conn_map.insert(key, conn); METRICS.vsock.conns_added.inc(); - Ok(()) }) } @@ -540,9 +539,8 @@ impl VsockMuxer { self.epoll .ctl(ControlOperation::Add, fd, EpollEvent::new(evset, fd as u64)) - .and_then(|_| { + .map(|_| { self.listener_map.insert(fd, listener); - Ok(()) }) .map_err(Error::EpollAdd)?; diff --git a/src/dumbo/src/tcp/connection.rs b/src/dumbo/src/tcp/connection.rs index b3b0cb74871..b702c8b958e 100644 --- a/src/dumbo/src/tcp/connection.rs +++ b/src/dumbo/src/tcp/connection.rs @@ -310,10 +310,7 @@ impl Connection { return false; } - match parse_mss_option(segment) { - Ok(mss) if mss == self.mss => true, - _ => false, - } + matches!(parse_mss_option(segment), Ok(mss) if mss == self.mss) } fn reset_for_segment(&mut self, s: &TcpSegment) { @@ -334,10 +331,7 @@ impl Connection { // has been ACKed by the other endpoint, and no FIN has been previously sent. fn can_send_first_fin(&self) -> bool { !self.fin_sent() - && match self.send_fin { - Some(fin_seq) if fin_seq == self.highest_ack_received => true, - _ => false, - } + && matches!(self.send_fin, Some(fin_seq) if fin_seq == self.highest_ack_received) } // Returns the window size which should be written to an outgoing segment. This is going to be diff --git a/src/kernel/src/cmdline/mod.rs b/src/kernel/src/cmdline/mod.rs index daf767022af..0f1dfd7a0a8 100644 --- a/src/kernel/src/cmdline/mod.rs +++ b/src/kernel/src/cmdline/mod.rs @@ -49,10 +49,7 @@ impl fmt::Display for Error { pub type Result = result::Result; fn valid_char(c: char) -> bool { - match c { - ' '..='~' => true, - _ => false, - } + matches!(c, ' '..='~') } fn valid_str(s: &str) -> Result<()> { diff --git a/src/utils/src/arg_parser.rs b/src/utils/src/arg_parser.rs index b8cc7b29c34..cb9e233203a 100644 --- a/src/utils/src/arg_parser.rs +++ b/src/utils/src/arg_parser.rs @@ -248,10 +248,7 @@ impl Value { } fn as_flag(&self) -> bool { - match self { - Value::Flag => true, - _ => false, - } + matches!(self, Value::Flag) } fn as_multiple(&self) -> Option<&[String]> { diff --git a/src/vmm/src/default_syscalls/filters.rs b/src/vmm/src/default_syscalls/filters.rs index a66a88037cb..511531f5b4a 100644 --- a/src/vmm/src/default_syscalls/filters.rs +++ b/src/vmm/src/default_syscalls/filters.rs @@ -177,7 +177,7 @@ pub fn get_seccomp_filter(seccomp_level: SeccompLevel) -> Result Ok(vec![]), SeccompLevel::Basic => default_filter() - .and_then(|filter| Ok(filter.allow_all())) + .map(|filter| filter.allow_all()) .and_then(|filter| filter.try_into()) .map_err(SeccompError::SeccompFilter), SeccompLevel::Advanced => default_filter() From 26d5bf43665557469abd5602472dfa72a0d6e416 Mon Sep 17 00:00:00 2001 From: Diana Popa Date: Wed, 9 Dec 2020 14:54:13 +0200 Subject: [PATCH 3/4] adjust coverage and binary size Signed-off-by: Diana Popa --- tests/integration_tests/build/test_binary_size.py | 4 ++-- tests/integration_tests/build/test_coverage.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests/build/test_binary_size.py b/tests/integration_tests/build/test_binary_size.py index 182e95bcce2..f30a88243ca 100644 --- a/tests/integration_tests/build/test_binary_size.py +++ b/tests/integration_tests/build/test_binary_size.py @@ -13,9 +13,9 @@ SIZES_DICT = { "x86_64": { - "FC_BINARY_SIZE_TARGET": 2212120, + "FC_BINARY_SIZE_TARGET": 2067944, "JAILER_BINARY_SIZE_TARGET": 1439512, - "FC_BINARY_SIZE_LIMIT": 2322726, + "FC_BINARY_SIZE_LIMIT": 2171516, "JAILER_BINARY_SIZE_LIMIT": 1511488, }, "aarch64": { diff --git a/tests/integration_tests/build/test_coverage.py b/tests/integration_tests/build/test_coverage.py index 732c7e8dc8e..a01930c32d5 100644 --- a/tests/integration_tests/build/test_coverage.py +++ b/tests/integration_tests/build/test_coverage.py @@ -23,7 +23,7 @@ # this contains the frequency while on AMD it does not. # Checkout the cpuid crate. In the future other # differences may appear. -COVERAGE_DICT = {"Intel": 85.25, "AMD": 84.50, "ARM": 82.72} +COVERAGE_DICT = {"Intel": 85.04, "AMD": 84.27, "ARM": 82.98} PROC_MODEL = proc.proc_type() COVERAGE_MAX_DELTA = 0.05 From 7bfd66a9b0494c0a1d2e4677551674e429ddcbee Mon Sep 17 00:00:00 2001 From: Diana Popa Date: Wed, 9 Dec 2020 15:58:46 +0200 Subject: [PATCH 4/4] CI: add mprotect to the whitelist syscalls Upon exec, a mprotect call is triggered. Since this syscall is not whitelisted the CI would fail with "Bad System Call". This is needed only in the CI. Signed-off-by: Diana Popa --- .../security/demo_seccomp/src/bin/demo_malicious.rs | 2 +- .../security/demo_seccomp/src/bin/seccomp_rules/mod.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration_tests/security/demo_seccomp/src/bin/demo_malicious.rs b/tests/integration_tests/security/demo_seccomp/src/bin/demo_malicious.rs index 06d8f78adc9..14ef85e8253 100644 --- a/tests/integration_tests/security/demo_seccomp/src/bin/demo_malicious.rs +++ b/tests/integration_tests/security/demo_seccomp/src/bin/demo_malicious.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 fn main() { unsafe { - // In this example, the malicious component is outputing to standard input. + // In this example, the malicious component is outputting to standard input. libc::syscall(libc::SYS_write, libc::STDIN_FILENO, "Hello, world!\n", 14); } } diff --git a/tests/integration_tests/security/demo_seccomp/src/bin/seccomp_rules/mod.rs b/tests/integration_tests/security/demo_seccomp/src/bin/seccomp_rules/mod.rs index 9fa97e998fe..9ec79bd8fd6 100644 --- a/tests/integration_tests/security/demo_seccomp/src/bin/seccomp_rules/mod.rs +++ b/tests/integration_tests/security/demo_seccomp/src/bin/seccomp_rules/mod.rs @@ -18,6 +18,7 @@ pub fn jailer_required_rules() -> Vec { allow_syscall(libc::SYS_rt_sigaction), allow_syscall(libc::SYS_execve), allow_syscall(libc::SYS_mmap), + allow_syscall(libc::SYS_mprotect), #[cfg(target_arch = "x86_64")] allow_syscall(libc::SYS_arch_prctl), allow_syscall(libc::SYS_set_tid_address),