linux-kernel - Re: [PATCH v9 06/27] rust: add C helpers

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: <202208171240.8B10053B9D@keescook>
Date:   Wed, 17 Aug 2022 12:44:41 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Miguel Ojeda <ojeda@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, patches@...ts.linux.dev,
        Jarkko Sakkinen <jarkko@...nel.org>,
        Alex Gaynor <alex.gaynor@...il.com>,
        Geoffrey Thomas <geofft@...reload.com>,
        Wedson Almeida Filho <wedsonaf@...gle.com>,
        Sven Van Asbroeck <thesven73@...il.com>,
        Gary Guo <gary@...yguo.net>, Boqun Feng <boqun.feng@...il.com>,
        Maciej Falkowski <m.falkowski@...sung.com>,
        Wei Liu <wei.liu@...nel.org>,
        Björn Roy Baron <bjorn3_gh@...tonmail.com>
Subject: Re: [PATCH v9 06/27] rust: add C helpers

On Fri, Aug 05, 2022 at 05:41:51PM +0200, Miguel Ojeda wrote:
> This source file contains forwarders to C macros and inlined
> functions.

Perhaps:

"Introduce the source file that will contain forwarders to common C
macros as inlined Rust functions. Initially this only contains type
size asserts, but will gain more helpers in subsequent patches."

> 
> Co-developed-by: Alex Gaynor <alex.gaynor@...il.com>
> Signed-off-by: Alex Gaynor <alex.gaynor@...il.com>
> Co-developed-by: Geoffrey Thomas <geofft@...reload.com>
> Signed-off-by: Geoffrey Thomas <geofft@...reload.com>
> Co-developed-by: Wedson Almeida Filho <wedsonaf@...gle.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@...gle.com>
> Co-developed-by: Sven Van Asbroeck <thesven73@...il.com>
> Signed-off-by: Sven Van Asbroeck <thesven73@...il.com>
> Co-developed-by: Gary Guo <gary@...yguo.net>
> Signed-off-by: Gary Guo <gary@...yguo.net>
> Co-developed-by: Boqun Feng <boqun.feng@...il.com>
> Signed-off-by: Boqun Feng <boqun.feng@...il.com>
> Co-developed-by: Maciej Falkowski <m.falkowski@...sung.com>
> Signed-off-by: Maciej Falkowski <m.falkowski@...sung.com>
> Co-developed-by: Wei Liu <wei.liu@...nel.org>
> Signed-off-by: Wei Liu <wei.liu@...nel.org>
> Signed-off-by: Miguel Ojeda <ojeda@...nel.org>
> ---
>  rust/helpers.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 rust/helpers.c
> 
> diff --git a/rust/helpers.c b/rust/helpers.c
> new file mode 100644
> index 000000000000..b4f15eee2ffd
> --- /dev/null
> +++ b/rust/helpers.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Non-trivial C macros cannot be used in Rust. Similarly, inlined C functions
> + * cannot be called either. This file explicitly creates functions ("helpers")
> + * that wrap those so that they can be called from Rust.
> + *
> + * Even though Rust kernel modules should never use directly the bindings, some
> + * of these helpers need to be exported because Rust generics and inlined
> + * functions may not get their code generated in the crate where they are
> + * defined. Other helpers, called from non-inline functions, may not be
> + * exported, in principle. However, in general, the Rust compiler does not
> + * guarantee codegen will be performed for a non-inline function either.
> + * Therefore, this file exports all the helpers. In the future, this may be
> + * revisited to reduce the number of exports after the compiler is informed
> + * about the places codegen is required.
> + *
> + * All symbols are exported as GPL-only to guarantee no GPL-only feature is
> + * accidentally exposed.
> + */
> +
> +#include <linux/bug.h>
> +#include <linux/build_bug.h>
> +
> +__noreturn void rust_helper_BUG(void)
> +{
> +	BUG();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_BUG);

Given the distaste for ever using BUG()[1], why does this helper exist?

> +
> +/*
> + * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> + * as the Rust `usize` type, so we can use it in contexts where Rust
> + * expects a `usize` like slice (array) indices. `usize` is defined to be
> + * the same as C's `uintptr_t` type (can hold any pointer) but not
> + * necessarily the same as `size_t` (can hold the size of any single
> + * object). Most modern platforms use the same concrete integer type for
> + * both of them, but in case we find ourselves on a platform where
> + * that's not true, fail early instead of risking ABI or
> + * integer-overflow issues.
> + *
> + * If your platform fails this assertion, it means that you are in
> + * danger of integer-overflow bugs (even if you attempt to remove
> + * `--size_t-is-usize`). It may be easiest to change the kernel ABI on
> + * your platform such that `size_t` matches `uintptr_t` (i.e., to increase
> + * `size_t`, because `uintptr_t` has to be at least as big as `size_t`).
> + */
> +static_assert(
> +	sizeof(size_t) == sizeof(uintptr_t) &&
> +	__alignof__(size_t) == __alignof__(uintptr_t),
> +	"Rust code expects C `size_t` to match Rust `usize`"
> +);

-Kees

[1] https://docs.kernel.org/process/deprecated.html#bug-and-bug-on

-- 
Kees Cook