Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add deprecation warning for uncomponentizable modules #2571

Merged
merged 1 commit into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions crates/componentize/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ rust-version.workspace = true

[dependencies]
anyhow = { workspace = true }
tracing = "0.1"
wasmparser = "0.200.0"
wasm-encoder = "0.200.0"
wasm-metadata = "0.200.0"
wit-component = "0.200.0"
wit-parser = "0.200.0"

Expand All @@ -28,3 +30,4 @@ serde = { version = "1.0.197", features = ["derive"] }
tempfile = "3.10.0"
toml = "0.8.10"
serde_json = "1.0"
wat = "1.200.0"
118 changes: 118 additions & 0 deletions crates/componentize/src/bugs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
use anyhow::bail;
use wasm_metadata::Producers;
use wasmparser::{Encoding, ExternalKind, Parser, Payload};

/// Represents the detected likelihood of the allocation bug fixed in
/// https://github.com/WebAssembly/wasi-libc/pull/377 being present in a Wasm
/// module.
#[derive(Debug, PartialEq)]
pub enum WasiLibc377Bug {
ProbablySafe,
ProbablyUnsafe,
Unknown,
}

impl WasiLibc377Bug {
pub fn detect(module: &[u8]) -> anyhow::Result<Self> {
for payload in Parser::new(0).parse_all(module) {
match payload? {
Payload::Version { encoding, .. } if encoding != Encoding::Module => {
bail!("detection only applicable to modules");
}
Payload::ExportSection(reader) => {
for export in reader {
let export = export?;
if export.kind == ExternalKind::Func && export.name == "cabi_realloc" {
// `cabi_realloc` is a good signal that this module
// uses wit-bindgen, making it probably-safe.
tracing::debug!("Found cabi_realloc export");
return Ok(Self::ProbablySafe);
}
}
}
Payload::CustomSection(c) if c.name() == "producers" => {
let producers = Producers::from_bytes(c.data(), c.data_offset())?;
if let Some(clang_version) =
producers.get("processed-by").and_then(|f| f.get("clang"))
{
tracing::debug!(clang_version, "Parsed producers.processed-by.clang");

// Clang/LLVM version is a good proxy for wasi-sdk
// version; the allocation bug was fixed in wasi-sdk-18
// and LLVM was updated to 15.0.7 in wasi-sdk-19.
if let Some((major, minor, patch)) = parse_clang_version(clang_version) {
return if (major, minor, patch) >= (15, 0, 7) {
Ok(Self::ProbablySafe)
} else {
Ok(Self::ProbablyUnsafe)
};
} else {
tracing::warn!(
clang_version,
"Unexpected producers.processed-by.clang version"
);
}
}
}
_ => (),
}
}
Ok(Self::Unknown)
}
}

fn parse_clang_version(ver: &str) -> Option<(u16, u16, u16)> {
// Strip optional trailing detail after space
let ver = ver.split(' ').next().unwrap();
let mut parts = ver.split('.');
let major = parts.next()?.parse().ok()?;
let minor = parts.next()?.parse().ok()?;
let patch = parts.next()?.parse().ok()?;
Some((major, minor, patch))
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn wasi_libc_377_detect() {
use WasiLibc377Bug::*;
for (wasm, expected) in [
(r#"(module)"#, Unknown),
(
r#"(module (func (export "cabi_realloc") (unreachable)))"#,
ProbablySafe,
),
(
r#"(module (func (export "some_other_function") (unreachable)))"#,
Unknown,
),
(
r#"(module (@producers (processed-by "clang" "16.0.0 extra-stuff")))"#,
ProbablySafe,
),
(
r#"(module (@producers (processed-by "clang" "15.0.7")))"#,
ProbablySafe,
),
(
r#"(module (@producers (processed-by "clang" "15.0.6")))"#,
ProbablyUnsafe,
),
(
r#"(module (@producers (processed-by "clang" "14.0.0")))"#,
ProbablyUnsafe,
),
(
r#"(module (@producers (processed-by "clang" "a.b.c")))"#,
Unknown,
),
] {
eprintln!("WAT: {wasm}");
let module = wat::parse_str(wasm).unwrap();
let detected = WasiLibc377Bug::detect(&module).unwrap();
assert_eq!(detected, expected);
}
}
}
2 changes: 2 additions & 0 deletions crates/componentize/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use {
wit_component::{metadata, ComponentEncoder},
};

pub mod bugs;

#[cfg(test)]
mod abi_conformance;
mod convert;
Expand Down
39 changes: 38 additions & 1 deletion crates/trigger/src/loader.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::path::PathBuf;
use std::path::{Path, PathBuf};

use anyhow::{ensure, Context, Result};
use async_trait::async_trait;
use spin_app::{
locked::{LockedApp, LockedComponentSource},
AppComponent, Loader,
};
use spin_componentize::bugs::WasiLibc377Bug;
use spin_core::StoreBuilder;
use tokio::fs;

Expand Down Expand Up @@ -135,6 +136,7 @@ impl Loader for TriggerLoader {
.as_ref()
.context("LockedComponentSource missing source field")?;
let path = parse_file_url(source)?;
check_uncomponentizable_module_deprecation(&path);
spin_core::Module::from_file(engine, &path)
.with_context(|| format!("loading module {}", quoted_path(&path)))
}
Expand Down Expand Up @@ -167,6 +169,41 @@ impl Loader for TriggerLoader {
}
}

// Check whether the given module is (likely) susceptible to a wasi-libc bug
// that makes it unsafe to componentize. If so, print a deprecation warning;
// this will turn into a hard error in a future release.
fn check_uncomponentizable_module_deprecation(module_path: &Path) {
let module = match std::fs::read(module_path) {
Ok(module) => module,
Err(err) => {
tracing::warn!("Failed to read {module_path:?}: {err:#}");
return;
}
};
match WasiLibc377Bug::detect(&module) {
Ok(WasiLibc377Bug::ProbablySafe) => {}
not_safe @ Ok(WasiLibc377Bug::ProbablyUnsafe | WasiLibc377Bug::Unknown) => {
println!(
"\n!!! DEPRECATION WARNING !!!\n\
The Wasm module at {path}\n\
{verbs} have been compiled with wasi-sdk version <19 and is likely to\n\
contain a critical memory safety bug. Spin has deprecated execution of these\n\
modules; they will stop working in a future release.\n\
For more information, see: https://github.com/fermyon/spin/issues/2552\n",
path = module_path.display(),
lann marked this conversation as resolved.
Show resolved Hide resolved
verbs = if not_safe.unwrap() == WasiLibc377Bug::ProbablyUnsafe {
"appears to"
} else {
"may"
}
);
}
Err(err) => {
tracing::warn!("Failed to apply wasi-libc bug heuristic on {module_path:?}: {err:#}");
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
2 changes: 2 additions & 0 deletions examples/spin-timer/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading