On Mon, Apr 01, 2024 at 06:52:50PM +0000, Benno Lossin wrote:
[...]
> + // 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;
> +
> + // SAFETY: `__this_module` is constructed by the kernel at load time and will not be
> + // freed until the module is unloaded.
> + #[cfg(MODULE)]
> + static THIS_MODULE: kernel::ThisModule = unsafe {{
> + kernel::ThisModule::from_ptr(&kernel::bindings::__this_module as *const _ as *mut _)
While we're at it, probably we want the following as well? I.e. using
`Opaque` and extern block, because __this_module is certainly something
interior mutable and !Unpin.
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 293beca0a583..8aa4eed6578c 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -219,7 +219,11 @@ mod __module_init {{
// freed until the module is unloaded.
#[cfg(MODULE)]
static THIS_MODULE: kernel::ThisModule = unsafe {{
- kernel::ThisModule::from_ptr(&kernel::bindings::__this_module as *const _ as *mut _)
+ extern \"C\" {{
+ static __this_module: kernel::types::Opaque<kernel::bindings::module>;
+ }}
+
+ kernel::ThisModule::from_ptr(__this_module.get())
}};
#[cfg(not(MODULE))]
static THIS_MODULE: kernel::ThisModule = unsafe {{
Thoughts?
Note this requires `Opaque::get` to be `const`, which I will send out
shortly, I think it's a good change regardless of the usage here.
Regards,
Boqun
> + }};
> + #[cfg(not(MODULE))]
> + static THIS_MODULE: kernel::ThisModule = unsafe {{
> + 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 {{
> + // 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() }}
> + }}
>
> - #[cfg(MODULE)]
> - #[doc(hidden)]
> - #[no_mangle]
> - pub extern \"C\" fn cleanup_module() {{
> - __exit()
> - }}
> + #[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() }}
> + }}
>
> - // 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
> - \"#
> - );
> + // 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(&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,
>
> base-commit: 9ffe2a730313f27cebd0859ea856247ac59c576c
> --
> 2.44.0
>
>
