Skip to content

Commit 0ad0ac0

Browse files
committed
Add deprecation warning for uncomponentizable modules
Signed-off-by: Lann Martin <[email protected]>
1 parent 5b4879d commit 0ad0ac0

File tree

5 files changed

+164
-1
lines changed

5 files changed

+164
-1
lines changed

Cargo.lock

+3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/componentize/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ rust-version.workspace = true
1010

1111
[dependencies]
1212
anyhow = { workspace = true }
13+
tracing = "0.1"
1314
wasmparser = "0.200.0"
1415
wasm-encoder = "0.200.0"
16+
wasm-metadata = "0.200.0"
1517
wit-component = "0.200.0"
1618
wit-parser = "0.200.0"
1719

@@ -28,3 +30,4 @@ serde = { version = "1.0.197", features = ["derive"] }
2830
tempfile = "3.10.0"
2931
toml = "0.8.10"
3032
serde_json = "1.0"
33+
wat = "1.200.0"

crates/componentize/src/bugs.rs

+118
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
use anyhow::bail;
2+
use wasm_metadata::Producers;
3+
use wasmparser::{Encoding, ExternalKind, Parser, Payload};
4+
5+
/// Represents the detected likelihood of the allocation bug fixed in
6+
/// https://github.com/WebAssembly/wasi-libc/pull/377 being present in a Wasm
7+
/// module.
8+
#[derive(Debug, PartialEq)]
9+
pub enum WasiLibc377Bug {
10+
ProbablySafe,
11+
ProbablyUnsafe,
12+
Unknown,
13+
}
14+
15+
impl WasiLibc377Bug {
16+
pub fn detect(module: &[u8]) -> anyhow::Result<Self> {
17+
for payload in Parser::new(0).parse_all(module) {
18+
match payload? {
19+
Payload::Version { encoding, .. } if encoding != Encoding::Module => {
20+
bail!("detection only applicable to modules");
21+
}
22+
Payload::ExportSection(reader) => {
23+
for export in reader {
24+
let export = export?;
25+
if export.kind == ExternalKind::Func && export.name == "cabi_realloc" {
26+
// `cabi_realloc` is a good signal that this module
27+
// uses wit-bindgen, making it probably-safe.
28+
tracing::debug!("Found cabi_realloc export");
29+
return Ok(Self::ProbablySafe);
30+
}
31+
}
32+
}
33+
Payload::CustomSection(c) if c.name() == "producers" => {
34+
let producers = Producers::from_bytes(c.data(), c.data_offset())?;
35+
if let Some(clang_version) =
36+
producers.get("processed-by").and_then(|f| f.get("clang"))
37+
{
38+
tracing::debug!(clang_version, "Parsed producers.processed-by.clang");
39+
40+
// Clang/LLVM version is a good proxy for wasi-sdk
41+
// version; the allocation bug was fixed in wasi-sdk-18
42+
// and LLVM was updated to 15.0.7 in wasi-sdk-19.
43+
if let Some((major, minor, patch)) = parse_clang_version(clang_version) {
44+
return if (major, minor, patch) >= (15, 0, 7) {
45+
Ok(Self::ProbablySafe)
46+
} else {
47+
Ok(Self::ProbablyUnsafe)
48+
};
49+
} else {
50+
tracing::warn!(
51+
clang_version,
52+
"Unexpected producers.processed-by.clang version"
53+
);
54+
}
55+
}
56+
}
57+
_ => (),
58+
}
59+
}
60+
Ok(Self::Unknown)
61+
}
62+
}
63+
64+
fn parse_clang_version(ver: &str) -> Option<(u16, u16, u16)> {
65+
// Strip optional trailing detail after space
66+
let ver = ver.split(' ').next().unwrap();
67+
let mut parts = ver.split('.');
68+
let major = parts.next()?.parse().ok()?;
69+
let minor = parts.next()?.parse().ok()?;
70+
let patch = parts.next()?.parse().ok()?;
71+
Some((major, minor, patch))
72+
}
73+
74+
#[cfg(test)]
75+
mod tests {
76+
use super::*;
77+
78+
#[test]
79+
fn wasi_libc_377_detect() {
80+
use WasiLibc377Bug::*;
81+
for (wasm, expected) in [
82+
(r#"(module)"#, Unknown),
83+
(
84+
r#"(module (func (export "cabi_realloc") (unreachable)))"#,
85+
ProbablySafe,
86+
),
87+
(
88+
r#"(module (func (export "some_other_function") (unreachable)))"#,
89+
Unknown,
90+
),
91+
(
92+
r#"(module (@producers (processed-by "clang" "16.0.0 extra-stuff")))"#,
93+
ProbablySafe,
94+
),
95+
(
96+
r#"(module (@producers (processed-by "clang" "15.0.7")))"#,
97+
ProbablySafe,
98+
),
99+
(
100+
r#"(module (@producers (processed-by "clang" "15.0.6")))"#,
101+
ProbablyUnsafe,
102+
),
103+
(
104+
r#"(module (@producers (processed-by "clang" "14.0.0")))"#,
105+
ProbablyUnsafe,
106+
),
107+
(
108+
r#"(module (@producers (processed-by "clang" "a.b.c")))"#,
109+
Unknown,
110+
),
111+
] {
112+
eprintln!("WAT: {wasm}");
113+
let module = wat::parse_str(wasm).unwrap();
114+
let detected = WasiLibc377Bug::detect(&module).unwrap();
115+
assert_eq!(detected, expected);
116+
}
117+
}
118+
}

crates/componentize/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ use {
99
wit_component::{metadata, ComponentEncoder},
1010
};
1111

12+
pub mod bugs;
13+
1214
#[cfg(test)]
1315
mod abi_conformance;
1416
mod convert;

crates/trigger/src/loader.rs

+38-1
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
use std::path::PathBuf;
1+
use std::path::{Path, PathBuf};
22

33
use anyhow::{ensure, Context, Result};
44
use async_trait::async_trait;
55
use spin_app::{
66
locked::{LockedApp, LockedComponentSource},
77
AppComponent, Loader,
88
};
9+
use spin_componentize::bugs::WasiLibc377Bug;
910
use spin_core::StoreBuilder;
1011
use tokio::fs;
1112

@@ -135,6 +136,7 @@ impl Loader for TriggerLoader {
135136
.as_ref()
136137
.context("LockedComponentSource missing source field")?;
137138
let path = parse_file_url(source)?;
139+
check_uncomponentizable_module_deprecation(&path);
138140
spin_core::Module::from_file(engine, &path)
139141
.with_context(|| format!("loading module {}", quoted_path(&path)))
140142
}
@@ -167,6 +169,41 @@ impl Loader for TriggerLoader {
167169
}
168170
}
169171

172+
// Check whether the given module is (likely) susceptible to a wasi-libc bug
173+
// that makes it unsafe to componentize. If so, print a deprecation warning;
174+
// this will turn into a hard error in a future release.
175+
fn check_uncomponentizable_module_deprecation(module_path: &Path) {
176+
let module = match std::fs::read(module_path) {
177+
Ok(module) => module,
178+
Err(err) => {
179+
tracing::warn!("Failed to read {module_path:?}: {err:#}");
180+
return;
181+
}
182+
};
183+
match WasiLibc377Bug::detect(&module) {
184+
Ok(WasiLibc377Bug::ProbablySafe) => {}
185+
not_safe @ Ok(WasiLibc377Bug::ProbablyUnsafe | WasiLibc377Bug::Unknown) => {
186+
println!(
187+
"\n!!! DEPRECATION WARNING !!!\n\
188+
The Wasm module at {path}\n\
189+
{verbs} have been compiled with wasi-sdk version <19 and is likely to contain\n\
190+
a critical memory safety bug. Spin has deprecated execution of these modules;\n\
191+
they will stop working in a future release.\n\
192+
For more information, see: https://github.com/fermyon/spin/issues/2552\n",
193+
path = module_path.display(),
194+
verbs = if not_safe.unwrap() == WasiLibc377Bug::ProbablyUnsafe {
195+
"appears to"
196+
} else {
197+
"may"
198+
}
199+
);
200+
}
201+
Err(err) => {
202+
tracing::warn!("Failed to apply wasi-libc bug heuristic on {module_path:?}: {err:#}");
203+
}
204+
}
205+
}
206+
170207
#[cfg(test)]
171208
mod tests {
172209
use super::*;

0 commit comments

Comments
 (0)