Skip to content

Commit c188444

Browse files
authored
Forbid unsafe code in most module (#1018)
This change adds the `forbid(unsafe_code)` directive to most modules, ensuring that these modules are "safe". There are three modules this cannot apply to: - `duplex` -- due to the implementation of `BufMut` for `CopyBuf`; and - `proxy::transport` -- due to socket option interactions. - `system` -- for system-level counter access Furthermore, this change adds `deny(warnings)` directives to a few modules that were missing it.
1 parent 94dec91 commit c188444

File tree

69 files changed

+253
-122
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

69 files changed

+253
-122
lines changed

Cargo.lock

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,6 @@ dependencies = [
645645
"hyper",
646646
"indexmap",
647647
"ipnet",
648-
"libc",
649648
"linkerd-addr",
650649
"linkerd-cache",
651650
"linkerd-concurrency-limit",
@@ -681,14 +680,14 @@ dependencies = [
681680
"linkerd-stack",
682681
"linkerd-stack-metrics",
683682
"linkerd-stack-tracing",
683+
"linkerd-system",
684684
"linkerd-timeout",
685685
"linkerd-tls",
686686
"linkerd-trace-context",
687687
"linkerd-tracing",
688688
"linkerd-transport-header",
689689
"linkerd2-proxy-api",
690690
"pin-project",
691-
"procinfo",
692691
"prost-types",
693692
"regex",
694693
"thiserror",
@@ -1344,6 +1343,15 @@ dependencies = [
13441343
"tracing",
13451344
]
13461345

1346+
[[package]]
1347+
name = "linkerd-system"
1348+
version = "0.1.0"
1349+
dependencies = [
1350+
"libc",
1351+
"procinfo",
1352+
"tracing",
1353+
]
1354+
13471355
[[package]]
13481356
name = "linkerd-timeout"
13491357
version = "0.1.0"

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ members = [
5151
"linkerd/stack",
5252
"linkerd/stack/metrics",
5353
"linkerd/stack/tracing",
54+
"linkerd/system",
5455
"linkerd/timeout",
5556
"linkerd/tls",
5657
"linkerd/tracing",

hyper-balance/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![deny(warnings, rust_2018_idioms)]
2+
#![forbid(unsafe_code)]
23
#![allow(clippy::inconsistent_struct_constructor)]
34

45
use hyper::body::HttpBody;

linkerd/addr/fuzz/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ tracing = "0.1.23"
1717
# Prevent this from interfering with workspaces
1818
[workspace]
1919
members = ["."]
20+
resolver = "2"
2021

2122
[[bin]]
2223
name = "fuzz_target_1"

linkerd/addr/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![deny(warnings, rust_2018_idioms)]
2+
#![forbid(unsafe_code)]
23
#![allow(clippy::inconsistent_struct_constructor)]
34
use linkerd_dns_name::Name;
45
use std::{

linkerd/app/core/Cargo.toml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,7 @@ features = [
8181
]
8282

8383
[target.'cfg(target_os = "linux")'.dependencies]
84-
libc = "0.2"
85-
procinfo = "0.4.2"
84+
linkerd-system = { path = "../../system" }
8685

8786
[dev-dependencies]
8887
linkerd2-proxy-api = { git = "https://github.com/linkerd/linkerd2-proxy-api", tag = "v0.1.18", features = ["arbitrary"] }

linkerd/app/core/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
//! - Metric labeling
99
1010
#![deny(warnings, rust_2018_idioms)]
11+
#![forbid(unsafe_code)]
1112
#![allow(clippy::inconsistent_struct_constructor)]
1213

1314
pub use linkerd_addr::{self as addr, Addr, NameAddr};
Lines changed: 55 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
1-
use self::system::System;
21
use linkerd_metrics::{metrics, FmtMetrics, Gauge};
32
use std::fmt;
43
use std::sync::Arc;
54
use std::time::{SystemTime, UNIX_EPOCH};
6-
use tracing::debug;
75

86
metrics! {
97
process_start_time_seconds: Gauge {
108
"Time that the process started (in seconds since the UNIX epoch)"
119
}
1210
}
1311

14-
#[derive(Clone, Debug, Default)]
12+
#[derive(Clone, Debug)]
1513
pub struct Report {
1614
start_time: Arc<Gauge>,
17-
system: Option<System>,
15+
16+
#[cfg(target_os = "linux")]
17+
system: linux::System,
1818
}
1919

2020
impl Report {
@@ -24,16 +24,13 @@ impl Report {
2424
.expect("process start time")
2525
.as_secs();
2626

27-
let system = match System::new() {
28-
Ok(s) => Some(s),
29-
Err(err) => {
30-
debug!("failed to load system stats: {}", err);
31-
None
32-
}
33-
};
27+
#[cfg(not(target_os = "linux"))]
28+
info!("System-level metrics are only supported on Linux");
3429
Self {
3530
start_time: Arc::new(t0.into()),
36-
system,
31+
32+
#[cfg(target_os = "linux")]
33+
system: linux::System::new(),
3734
}
3835
}
3936
}
@@ -43,22 +40,19 @@ impl FmtMetrics for Report {
4340
process_start_time_seconds.fmt_help(f)?;
4441
process_start_time_seconds.fmt_metric(f, self.start_time.as_ref())?;
4542

46-
if let Some(ref sys) = self.system {
47-
sys.fmt_metrics(f)?;
48-
}
43+
#[cfg(target_os = "linux")]
44+
self.system.fmt_metrics(f)?;
4945

5046
Ok(())
5147
}
5248
}
5349

5450
#[cfg(target_os = "linux")]
55-
mod system {
56-
use libc::{self, pid_t};
51+
mod linux {
5752
use linkerd_metrics::{metrics, Counter, FmtMetrics, Gauge, MillisAsSeconds};
58-
use procinfo::pid;
53+
use linkerd_system as sys;
5954
use std::fmt;
60-
use std::{fs, io};
61-
use tracing::{error, warn};
55+
use tracing::warn;
6256

6357
metrics! {
6458
process_cpu_seconds_total: Counter<MillisAsSeconds> {
@@ -74,131 +68,86 @@ mod system {
7468
}
7569
}
7670

77-
#[derive(Clone, Debug)]
71+
#[derive(Clone, Debug, Default)]
7872
pub(super) struct System {
79-
page_size: u64,
80-
ms_per_tick: u64,
73+
page_size: Option<u64>,
74+
ms_per_tick: Option<u64>,
8175
}
8276

8377
impl System {
84-
pub fn new() -> io::Result<Self> {
85-
let page_size = Self::sysconf(libc::_SC_PAGESIZE, "page size")?;
86-
87-
// On Linux, CLK_TCK is ~always `100`, so pure integer division
88-
// works. This is probably not suitable if we encounter other
89-
// values.
90-
let clock_ticks_per_sec = Self::sysconf(libc::_SC_CLK_TCK, "clock ticks per second")?;
91-
let ms_per_tick = 1_000 / clock_ticks_per_sec;
92-
if clock_ticks_per_sec != 100 {
93-
warn!(
94-
clock_ticks_per_sec,
95-
ms_per_tick, "Unexpected value; process_cpu_seconds_total may be inaccurate."
96-
);
97-
}
98-
99-
Ok(Self {
100-
page_size,
101-
ms_per_tick,
102-
})
103-
}
104-
105-
fn open_fds(pid: pid_t) -> io::Result<Gauge> {
106-
let mut open = 0;
107-
for f in fs::read_dir(format!("/proc/{}/fd", pid))? {
108-
if !f?.file_type()?.is_dir() {
109-
open += 1;
78+
pub fn new() -> Self {
79+
let page_size = match sys::page_size() {
80+
Ok(ps) => Some(ps),
81+
Err(err) => {
82+
warn!("Failed to load page size: {}", err);
83+
None
11084
}
111-
}
112-
Ok(Gauge::from(open))
113-
}
114-
115-
fn max_fds() -> io::Result<Option<Gauge>> {
116-
let limit = pid::limits_self()?.max_open_files;
117-
let max_fds = limit.soft.or(limit.hard).map(|max| Gauge::from(max as u64));
118-
Ok(max_fds)
119-
}
120-
121-
fn sysconf(num: libc::c_int, name: &'static str) -> Result<u64, io::Error> {
122-
match unsafe { libc::sysconf(num) } {
123-
e if e <= 0 => {
124-
let error = io::Error::last_os_error();
125-
error!("error getting {}: {:?}", name, error);
126-
Err(error)
85+
};
86+
let ms_per_tick = match sys::ms_per_tick() {
87+
Ok(mpt) => Some(mpt),
88+
Err(err) => {
89+
warn!("Failed to load cpu clock speed: {}", err);
90+
None
12791
}
128-
val => Ok(val as u64),
92+
};
93+
Self {
94+
page_size,
95+
ms_per_tick,
12996
}
13097
}
13198
}
13299

133100
impl FmtMetrics for System {
134101
fn fmt_metrics(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
135-
// XXX potentially blocking call
136-
let stat = match pid::stat_self() {
102+
let stat = match sys::blocking_stat() {
137103
Ok(stat) => stat,
138104
Err(err) => {
139105
warn!("failed to read process stats: {}", err);
140106
return Ok(());
141107
}
142108
};
143109

144-
let clock_ticks = stat.utime as u64 + stat.stime as u64;
145-
let cpu_ms = clock_ticks * self.ms_per_tick;
146-
process_cpu_seconds_total.fmt_help(f)?;
147-
process_cpu_seconds_total.fmt_metric(f, &Counter::from(cpu_ms))?;
110+
if let Some(mpt) = self.ms_per_tick {
111+
let clock_ticks = stat.utime as u64 + stat.stime as u64;
112+
let cpu_ms = clock_ticks * mpt;
113+
process_cpu_seconds_total.fmt_help(f)?;
114+
process_cpu_seconds_total.fmt_metric(f, &Counter::from(cpu_ms))?;
115+
} else {
116+
warn!("Could not determine process_cpu_seconds_total");
117+
}
148118

149119
process_virtual_memory_bytes.fmt_help(f)?;
150120
process_virtual_memory_bytes.fmt_metric(f, &Gauge::from(stat.vsize as u64))?;
151121

152-
process_resident_memory_bytes.fmt_help(f)?;
153-
process_resident_memory_bytes
154-
.fmt_metric(f, &Gauge::from(stat.rss as u64 * self.page_size))?;
122+
if let Some(ps) = self.page_size {
123+
process_resident_memory_bytes.fmt_help(f)?;
124+
process_resident_memory_bytes.fmt_metric(f, &Gauge::from(stat.rss as u64 * ps))?;
125+
} else {
126+
warn!("Could not determine process_resident_memory_bytes");
127+
}
155128

156-
match Self::open_fds(stat.pid) {
129+
match sys::open_fds(stat.pid) {
157130
Ok(open_fds) => {
158131
process_open_fds.fmt_help(f)?;
159-
process_open_fds.fmt_metric(f, &open_fds)?;
132+
process_open_fds.fmt_metric(f, &open_fds.into())?;
160133
}
161134
Err(err) => {
162-
warn!("could not determine process_open_fds: {}", err);
135+
warn!("Could not determine process_open_fds: {}", err);
163136
}
164137
}
165138

166-
match Self::max_fds() {
139+
match sys::max_fds() {
167140
Ok(None) => {}
168-
Ok(Some(ref max_fds)) => {
141+
Ok(Some(max_fds)) => {
169142
process_max_fds.fmt_help(f)?;
170-
process_max_fds.fmt_metric(f, max_fds)?;
143+
process_max_fds.fmt_metric(f, &max_fds.into())?;
171144
}
172145
Err(err) => {
173-
warn!("could not determine process_max_fds: {}", err);
146+
warn!("Could not determine process_max_fds: {}", err);
174147
}
175148
}
176149

177150
Ok(())
178151
}
179152
}
180153
}
181-
182-
#[cfg(not(target_os = "linux"))]
183-
mod system {
184-
use crate::metrics::FmtMetrics;
185-
use std::{fmt, io};
186-
187-
#[derive(Clone, Debug)]
188-
pub(super) struct System {}
189-
190-
impl System {
191-
pub fn new() -> io::Result<Self> {
192-
Err(io::Error::new(
193-
io::ErrorKind::Other,
194-
"procinfo not supported on this operating system",
195-
))
196-
}
197-
}
198-
199-
impl FmtMetrics for System {
200-
fn fmt_metrics(&self, _: &mut fmt::Formatter<'_>) -> fmt::Result {
201-
Ok(())
202-
}
203-
}
204-
}

linkerd/app/gateway/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![deny(warnings, rust_2018_idioms)]
2+
#![forbid(unsafe_code)]
23
#![allow(clippy::inconsistent_struct_constructor)]
34

45
mod gateway;

linkerd/app/inbound/fuzz/Cargo.lock

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,6 @@ dependencies = [
575575
"hyper",
576576
"indexmap",
577577
"ipnet",
578-
"libc",
579578
"linkerd-addr",
580579
"linkerd-cache",
581580
"linkerd-concurrency-limit",
@@ -611,14 +610,14 @@ dependencies = [
611610
"linkerd-stack",
612611
"linkerd-stack-metrics",
613612
"linkerd-stack-tracing",
613+
"linkerd-system",
614614
"linkerd-timeout",
615615
"linkerd-tls",
616616
"linkerd-trace-context",
617617
"linkerd-tracing",
618618
"linkerd-transport-header",
619619
"linkerd2-proxy-api",
620620
"pin-project",
621-
"procinfo",
622621
"regex",
623622
"thiserror",
624623
"tokio",
@@ -1189,6 +1188,15 @@ dependencies = [
11891188
"tracing",
11901189
]
11911190

1191+
[[package]]
1192+
name = "linkerd-system"
1193+
version = "0.1.0"
1194+
dependencies = [
1195+
"libc",
1196+
"procinfo",
1197+
"tracing",
1198+
]
1199+
11921200
[[package]]
11931201
name = "linkerd-timeout"
11941202
version = "0.1.0"

0 commit comments

Comments
 (0)