Commit 7044dcff authored by Benno Lossin's avatar Benno Lossin Committed by Miguel Ojeda
Browse files

rust: macros: fix soundness issue in `module!` macro 



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: default avatarBjörn Roy Baron <bjorn3_gh@protonmail.com>
Closes: https://github.com/Rust-for-Linux/linux/issues/629


Fixes: 1fbde52b ("rust: add `macros` crate")
Signed-off-by: default avatarBenno Lossin <benno.lossin@proton.me>
Reviewed-by: default avatarWedson 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: default avatarMiguel Ojeda <ojeda@kernel.org>
parent 49ceae68

rust/macros/module.rs

+115 −75
Original line number Diff line number Diff line
@@ -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)]
@@ -221,6 +210,22 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { 
                kernel::ThisModule::from_ptr(core::ptr::null_mut())

            }};



            // 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

                    ///
@@ -231,14 +236,23 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { 
                    #[no_mangle]

                    #[link_section = \".init.text\"]

                    pub unsafe extern \"C\" fn init_module() -> core::ffi::c_int {{

                __init()

                        // 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()

                        // 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
@@ -264,19 +278,35 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { 
                    #[doc(hidden)]

                    #[no_mangle]

                    pub extern \"C\" fn __{name}_init() -> core::ffi::c_int {{

                __init()

                        // 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}_exit() {{

                __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() }}

                    }}



            fn __init() -> core::ffi::c_int {{

                match <{type_} as kernel::Module>::init(&THIS_MODULE) {{

                    /// # 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);

                                }}
@@ -288,7 +318,15 @@ fn __init() -> core::ffi::c_int {{ 
                        }}

                    }}



            fn __exit() {{

                    /// # 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 {{

                            // Invokes `drop()` on `__MOD`, which should be used for cleanup.

                            __MOD = None;
@@ -296,6 +334,8 @@ fn __exit() {{ 
                    }}



                    {modinfo}

                }}

            }}

        ",

        type_ = info.type_,

        name = info.name,