linux-kernel - Re: [PATCH RFC 02/20] rust_binder: add binderfs support to Rust binder

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <B4D1A3E2-1AD9-434A-90AC-8D33532D4A1B@kloenk.de>
Date:   Fri, 03 Nov 2023 11:11:22 +0100
From:   Finn Behrens <me@...enk.de>
To:     Alice Ryhl <aliceryhl@...gle.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Arve Hjønnevåg <arve@...roid.com>,
        Todd Kjos <tkjos@...roid.com>,
        Martijn Coenen <maco@...roid.com>,
        Joel Fernandes <joel@...lfernandes.org>,
        Christian Brauner <brauner@...nel.org>,
        Carlos Llamas <cmllamas@...gle.com>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Miguel Ojeda <ojeda@...nel.org>,
        Alex Gaynor <alex.gaynor@...il.com>,
        Wedson Almeida Filho <wedsonaf@...il.com>,
        linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
        Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
        Björn Roy Baron <bjorn3_gh@...tonmail.com>,
        Benno Lossin <benno.lossin@...ton.me>,
        Andreas Hindborg <a.hindborg@...sung.com>,
        Matt Gilbride <mattgilbride@...gle.com>,
        Jeffrey Vander Stoep <jeffv@...gle.com>,
        Matthew Maurer <mmaurer@...gle.com>
Subject: Re: [PATCH RFC 02/20] rust_binder: add binderfs support to Rust
 binder



On 1 Nov 2023, at 19:01, Alice Ryhl wrote:

> Add support for accessing the Rust binder driver via binderfs. The
> actual binderfs implementation is done entirely in C, and the
> `rust_binderfs.c` file is a modified version of `binderfs.c` that is
> adjusted to call into the Rust binder driver rather than the C driver.
>
> We have left the binderfs filesystem component in C. Rewriting it in
> Rust would be a large amount of work and requires a lot of bindings to
> the file system interfaces. Binderfs has not historically had the same
> challenges with security and complexity, so rewriting Binderfs seems to
> have lower value than the rest of Binder.
>
> We also add code on the Rust side for binderfs to call into. Most of
> this is left as stub implementation, with the exception of closing the
> file descriptor and the BINDER_VERSION ioctl.
>
> Co-developed-by: Wedson Almeida Filho <wedsonaf@...il.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@...il.com>
> Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
> ---
>  drivers/android/Kconfig         |  24 ++
>  drivers/android/Makefile        |   1 +
>  drivers/android/context.rs      | 144 +++++++
>  drivers/android/defs.rs         |  39 ++
>  drivers/android/process.rs      | 251 ++++++++++++
>  drivers/android/rust_binder.rs  | 196 ++++++++-
>  drivers/android/rust_binderfs.c | 866 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/rust_binder.h     |  16 +
>  include/uapi/linux/magic.h      |   1 +
>  rust/bindings/bindings_helper.h |   2 +
>  rust/kernel/lib.rs              |   7 +
>  scripts/Makefile.build          |   2 +-
>  12 files changed, 1547 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
> index fcfd25c9a016..82ed6ddabe1a 100644
> --- a/drivers/android/Kconfig
> +++ b/drivers/android/Kconfig
> diff --git a/drivers/android/Makefile b/drivers/android/Makefile
> index 6348f75832ca..5c819011aa77 100644
> --- a/drivers/android/Makefile
> +++ b/drivers/android/Makefile
> diff --git a/drivers/android/context.rs b/drivers/android/context.rs
> new file mode 100644
> index 000000000000..630cb575d3ac
> --- /dev/null
> +++ b/drivers/android/context.rs
> diff --git a/drivers/android/defs.rs b/drivers/android/defs.rs
> new file mode 100644
> index 000000000000..8fdcb856ccad
> --- /dev/null
> +++ b/drivers/android/defs.rs
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use core::ops::{Deref, DerefMut};
> +use kernel::{
> +    bindings,
> +    io_buffer::{ReadableFromBytes, WritableToBytes},
> +};
> +
> +macro_rules! decl_wrapper {
> +    ($newname:ident, $wrapped:ty) => {
> +        #[derive(Copy, Clone, Default)]
> +        #[repr(transparent)]
> +        pub(crate) struct $newname($wrapped);
> +        // SAFETY: This macro is only used with types where this is ok.
Would it make sense so also annotade this safety requirement on the macro itself?
It is only file private, but could help not overlook it, when using for something new in the same file.
> +        unsafe impl ReadableFromBytes for $newname {}
> +        unsafe impl WritableToBytes for $newname {}
> +        impl Deref for $newname {
> +            type Target = $wrapped;
> +            fn deref(&self) -> &Self::Target {
> +                &self.0
> +            }
> +        }
> +        impl DerefMut for $newname {
> +            fn deref_mut(&mut self) -> &mut Self::Target {
> +                &mut self.0
> +            }
> +        }
> +    };
> +}
> +
> +decl_wrapper!(BinderVersion, bindings::binder_version);
> +
> +impl BinderVersion {
> +    pub(crate) fn current() -> Self {
> +        Self(bindings::binder_version {
> +            protocol_version: bindings::BINDER_CURRENT_PROTOCOL_VERSION as _,
> +        })
> +    }
> +}
> diff --git a/drivers/android/process.rs b/drivers/android/process.rs
> new file mode 100644
> index 000000000000..2f16e4cedbf1
> --- /dev/null
> +++ b/drivers/android/process.rs
> diff --git a/drivers/android/rust_binder.rs b/drivers/android/rust_binder.rs
> index 4b3d6676a9cf..6de2f40846fb 100644
> --- a/drivers/android/rust_binder.rs
> +++ b/drivers/android/rust_binder.rs
> diff --git a/drivers/android/rust_binderfs.c b/drivers/android/rust_binderfs.c
> new file mode 100644
> index 000000000000..2c011e26752c
> --- /dev/null
> +++ b/drivers/android/rust_binderfs.c
> diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
> index 6325d1d0e90f..e5a20c1498af 100644
> --- a/include/uapi/linux/magic.h
> +++ b/include/uapi/linux/magic.h
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 00a66666f00a..ffeea312f2fd 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 435d4c2ac5fc..f4d58da9202e 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index da37bfa97211..f78d2e75a795 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> -- 
> 2.42.0.820.g83a721a137-goog