Skip to content

Commit

Permalink
rust: macros: fix soundness issue in module! macro
Browse files Browse the repository at this point in the history 
[ Upstream commit 7044dcf ]

The `module!` macro creates glue code that are called by C to initialize
the Rust modules using the `Module::init` function. Part of this glue
code are the local functions `__init` and `__exit` that are used to
initialize/destroy the Rust module.

These functions are safe and also visible to the Rust mod in which the
`module!` macro is invoked. This means that they can be called by other
safe Rust code. But since they contain `unsafe` blocks that rely on only
being called at the right time, this is a soundness issue.

Wrap these generated functions inside of two private modules, this
guarantees that the public functions cannot be called from the outside.
Make the safe functions `unsafe` and add SAFETY comments.

Cc: stable@vger.kernel.org
Reported-by: Björn Roy Baron <bjorn3_gh@protonmail.com>
Closes: Rust-for-Linux/linux#629
Fixes: 1fbde52 ("rust: add `macros` crate")
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Wedson Almeida Filho <walmeida@microsoft.com>
Link: https://lore.kernel.org/r/20240401185222.12015-1-benno.lossin@proton.me
[ Moved `THIS_MODULE` out of the private-in-private modules since it
  should remain public, as Dirk Behme noticed [1]. Capitalized comments,
  avoided newline in non-list SAFETY comments and reworded to add
  Reported-by and newline. ]
Link: https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/x/near/433512583 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
@y86-dev @gregkh
y86-dev authored and gregkh committed May 17, 2024
1 parent 738ea6d commit a4dec33
Showing 1 changed file with 115 additions and 75 deletions.
190 changes: 115 additions & 75 deletions rust/macros/module.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,17 +199,6 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
/// Used by the printing macros, e.g. [`info!`].
const __LOG_PREFIX: &[u8] = b\"{name}\\0\";
/// The \"Rust loadable module\" mark.
//
// This may be best done another way later on, e.g. as a new modinfo
// key or a new section. For the moment, keep it simple.
#[cfg(MODULE)]
#[doc(hidden)]
#[used]
static __IS_RUST_MODULE: () = ();
static mut __MOD: Option<{type_}> = None;
// SAFETY: `__this_module` is constructed by the kernel at load time and will not be
// freed until the module is unloaded.
#[cfg(MODULE)]
Expand All @@ -221,81 +210,132 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
kernel::ThisModule::from_ptr(core::ptr::null_mut())
}};
// Loadable modules need to export the `{{init,cleanup}}_module` identifiers.
/// # Safety
///
/// This function must not be called after module initialization, because it may be
/// freed after that completes.
#[cfg(MODULE)]
#[doc(hidden)]
#[no_mangle]
#[link_section = \".init.text\"]
pub unsafe extern \"C\" fn init_module() -> core::ffi::c_int {{
__init()
}}
#[cfg(MODULE)]
#[doc(hidden)]
#[no_mangle]
pub extern \"C\" fn cleanup_module() {{
__exit()
}}
// Double nested modules, since then nobody can access the public items inside.
mod __module_init {{
mod __module_init {{
use super::super::{type_};
/// The \"Rust loadable module\" mark.
//
// This may be best done another way later on, e.g. as a new modinfo
// key or a new section. For the moment, keep it simple.
#[cfg(MODULE)]
#[doc(hidden)]
#[used]
static __IS_RUST_MODULE: () = ();
static mut __MOD: Option<{type_}> = None;
// Loadable modules need to export the `{{init,cleanup}}_module` identifiers.
/// # Safety
///
/// This function must not be called after module initialization, because it may be
/// freed after that completes.
#[cfg(MODULE)]
#[doc(hidden)]
#[no_mangle]
#[link_section = \".init.text\"]
pub unsafe extern \"C\" fn init_module() -> core::ffi::c_int {{
// SAFETY: This function is inaccessible to the outside due to the double
// module wrapping it. It is called exactly once by the C side via its
// unique name.
unsafe {{ __init() }}
}}
// Built-in modules are initialized through an initcall pointer
// and the identifiers need to be unique.
#[cfg(not(MODULE))]
#[cfg(not(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS))]
#[doc(hidden)]
#[link_section = \"{initcall_section}\"]
#[used]
pub static __{name}_initcall: extern \"C\" fn() -> core::ffi::c_int = __{name}_init;
#[cfg(MODULE)]
#[doc(hidden)]
#[no_mangle]
pub extern \"C\" fn cleanup_module() {{
// SAFETY:
// - This function is inaccessible to the outside due to the double
// module wrapping it. It is called exactly once by the C side via its
// unique name,
// - furthermore it is only called after `init_module` has returned `0`
// (which delegates to `__init`).
unsafe {{ __exit() }}
}}
#[cfg(not(MODULE))]
#[cfg(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)]
core::arch::global_asm!(
r#\".section \"{initcall_section}\", \"a\"
__{name}_initcall:
.long __{name}_init - .
.previous
\"#
);
// Built-in modules are initialized through an initcall pointer
// and the identifiers need to be unique.
#[cfg(not(MODULE))]
#[cfg(not(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS))]
#[doc(hidden)]
#[link_section = \"{initcall_section}\"]
#[used]
pub static __{name}_initcall: extern \"C\" fn() -> core::ffi::c_int = __{name}_init;
#[cfg(not(MODULE))]
#[cfg(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)]
core::arch::global_asm!(
r#\".section \"{initcall_section}\", \"a\"
__{name}_initcall:
.long __{name}_init - .
.previous
\"#
);
#[cfg(not(MODULE))]
#[doc(hidden)]
#[no_mangle]
pub extern \"C\" fn __{name}_init() -> core::ffi::c_int {{
// SAFETY: This function is inaccessible to the outside due to the double
// module wrapping it. It is called exactly once by the C side via its
// placement above in the initcall section.
unsafe {{ __init() }}
}}
#[cfg(not(MODULE))]
#[doc(hidden)]
#[no_mangle]
pub extern \"C\" fn __{name}_init() -> core::ffi::c_int {{
__init()
}}
#[cfg(not(MODULE))]
#[doc(hidden)]
#[no_mangle]
pub extern \"C\" fn __{name}_exit() {{
// SAFETY:
// - This function is inaccessible to the outside due to the double
// module wrapping it. It is called exactly once by the C side via its
// unique name,
// - furthermore it is only called after `__{name}_init` has returned `0`
// (which delegates to `__init`).
unsafe {{ __exit() }}
}}
#[cfg(not(MODULE))]
#[doc(hidden)]
#[no_mangle]
pub extern \"C\" fn __{name}_exit() {{
__exit()
}}
/// # Safety
///
/// This function must only be called once.
unsafe fn __init() -> core::ffi::c_int {{
match <{type_} as kernel::Module>::init(&super::super::THIS_MODULE) {{
Ok(m) => {{
// SAFETY: No data race, since `__MOD` can only be accessed by this
// module and there only `__init` and `__exit` access it. These
// functions are only called once and `__exit` cannot be called
// before or during `__init`.
unsafe {{
__MOD = Some(m);
}}
return 0;
}}
Err(e) => {{
return e.to_errno();
}}
}}
}}
fn __init() -> core::ffi::c_int {{
match <{type_} as kernel::Module>::init(&THIS_MODULE) {{
Ok(m) => {{
/// # Safety
///
/// This function must
/// - only be called once,
/// - be called after `__init` has been called and returned `0`.
unsafe fn __exit() {{
// SAFETY: No data race, since `__MOD` can only be accessed by this module
// and there only `__init` and `__exit` access it. These functions are only
// called once and `__init` was already called.
unsafe {{
__MOD = Some(m);
// Invokes `drop()` on `__MOD`, which should be used for cleanup.
__MOD = None;
}}
return 0;
}}
Err(e) => {{
return e.to_errno();
}}
}}
}}
fn __exit() {{
unsafe {{
// Invokes `drop()` on `__MOD`, which should be used for cleanup.
__MOD = None;
{modinfo}
}}
}}
{modinfo}
",
type_ = info.type_,
name = info.name,
Expand Down

0 comments on commit a4dec33

Please sign in to comment.