Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date:   Thu, 12 Jan 2023 15:25:38 +0100
Message-Id: <CPQA4LWQZOX1.31XJYNUR2O449@vincent>
To:     "Miguel Ojeda" <ojeda@kernel.org>,
        "Wedson Almeida Filho" <wedsonaf@gmail.com>,
        "Alex Gaynor" <alex.gaynor@gmail.com>,
        "Boqun Feng" <boqun.feng@gmail.com>, "Gary Guo" <gary@garyguo.net>,
        =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>
Cc:     <rust-for-linux@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <patches@lists.linux.dev>,
        "Domen Puncer Kugler" <domen.puncerkugler@nccgroup.com>
Subject: Re: [PATCH] rust: print: avoid evaluating arguments in `pr_*`
 macros in `unsafe` blocks
From:   "Vincenzo Palazzo" <vincenzopalazzodev@gmail.com>
References: <20230109204912.539790-1-ojeda@kernel.org>
In-Reply-To: <20230109204912.539790-1-ojeda@kernel.org>
Precedence: bulk

On Mon Jan 9, 2023 at 9:49 PM CET, Miguel Ojeda wrote:
> At the moment it is possible to perform unsafe operations in
> the arguments of `pr_*` macros since they are evaluated inside
> an `unsafe` block:
>
>     let x =3D &10u32 as *const u32;
>     pr_info!("{}", *x);
>
> In other words, this is a soundness issue.
>
> Fix it so that it requires an explicit `unsafe` block.
>
> Reported-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Reported-by: Domen Puncer Kugler <domen.puncerkugler@nccgroup.com>
> Link: https://github.com/Rust-for-Linux/linux/issues/479
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
> ---
>  rust/kernel/print.rs | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
> index 29bf9c2e8aee..30103325696d 100644
> --- a/rust/kernel/print.rs
> +++ b/rust/kernel/print.rs
> @@ -142,17 +142,24 @@ pub fn call_printk_cont(args: fmt::Arguments<'_>) {
>  macro_rules! print_macro (
>      // The non-continuation cases (most of them, e.g. `INFO`).
>      ($format_string:path, false, $($arg:tt)+) =3D> (
> -        // SAFETY: This hidden macro should only be called by the docume=
nted
> -        // printing macros which ensure the format string is one of the =
fixed
> -        // ones. All `__LOG_PREFIX`s are null-terminated as they are gen=
erated
> -        // by the `module!` proc macro or fixed values defined in a kern=
el
> -        // crate.
> -        unsafe {
> -            $crate::print::call_printk(
> -                &$format_string,
> -                crate::__LOG_PREFIX,
> -                format_args!($($arg)+),
> -            );
> +        // To remain sound, `arg`s must be expanded outside the `unsafe`=
 block.
> +        // Typically one would use a `let` binding for that; however, `f=
ormat_args!`
> +        // takes borrows on the arguments, but does not extend the scope=
 of temporaries.
> +        // Therefore, a `match` expression is used to keep them around, =
since
> +        // the scrutinee is kept until the end of the `match`.
> +        match format_args!($($arg)+) {
> +            // SAFETY: This hidden macro should only be called by the do=
cumented
> +            // printing macros which ensure the format string is one of =
the fixed
> +            // ones. All `__LOG_PREFIX`s are null-terminated as they are=
 generated
> +            // by the `module!` proc macro or fixed values defined in a =
kernel
> +            // crate.
> +            args =3D> unsafe {
> +                $crate::print::call_printk(
> +                    &$format_string,
> +                    crate::__LOG_PREFIX,
> +                    args,
> +                );
> +            }
>          }
>      );
> =20
>
> base-commit: b7bfaa761d760e72a969d116517eaa12e404c262
> --=20
> 2.39.0