* Re: [PATCH] rust: print: avoid evaluating arguments in `pr_*` macros in `unsafe` blocks
2023-01-09 20:49 [PATCH] rust: print: avoid evaluating arguments in `pr_*` macros in `unsafe` blocks Miguel Ojeda
@ 2023-01-09 21:19 ` Boqun Feng
2023-01-10 14:47 ` Gary Guo
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Boqun Feng @ 2023-01-09 21:19 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Wedson Almeida Filho, Alex Gaynor, Gary Guo, Björn Roy Baron,
rust-for-linux, linux-kernel, patches, Domen Puncer Kugler
On Mon, Jan 09, 2023 at 09:49:12PM +0100, 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 = &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: Boqun Feng <boqun.feng@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)+) => (
> - // SAFETY: This hidden macro should only be called by the documented
> - // 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.
> - 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, `format_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 documented
> + // 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 => unsafe {
> + $crate::print::call_printk(
> + &$format_string,
> + crate::__LOG_PREFIX,
> + args,
> + );
> + }
> }
> );
>
>
> base-commit: b7bfaa761d760e72a969d116517eaa12e404c262
> --
> 2.39.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] rust: print: avoid evaluating arguments in `pr_*` macros in `unsafe` blocks
2023-01-09 20:49 [PATCH] rust: print: avoid evaluating arguments in `pr_*` macros in `unsafe` blocks Miguel Ojeda
2023-01-09 21:19 ` Boqun Feng
@ 2023-01-10 14:47 ` Gary Guo
2023-01-11 10:43 ` Björn Roy Baron
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Gary Guo @ 2023-01-10 14:47 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Björn Roy Baron, rust-for-linux, linux-kernel, patches,
Domen Puncer Kugler
On Mon, 9 Jan 2023 21:49:12 +0100
Miguel Ojeda <ojeda@kernel.org> 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 = &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: Gary Guo <gary@garyguo.net>
> ---
> 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)+) => (
> - // SAFETY: This hidden macro should only be called by the documented
> - // 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.
> - 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, `format_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 documented
> + // 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 => unsafe {
> + $crate::print::call_printk(
> + &$format_string,
> + crate::__LOG_PREFIX,
> + args,
> + );
> + }
> }
> );
>
>
> base-commit: b7bfaa761d760e72a969d116517eaa12e404c262
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] rust: print: avoid evaluating arguments in `pr_*` macros in `unsafe` blocks
2023-01-09 20:49 [PATCH] rust: print: avoid evaluating arguments in `pr_*` macros in `unsafe` blocks Miguel Ojeda
2023-01-09 21:19 ` Boqun Feng
2023-01-10 14:47 ` Gary Guo
@ 2023-01-11 10:43 ` Björn Roy Baron
2023-01-12 14:25 ` Vincenzo Palazzo
2023-01-16 0:03 ` Miguel Ojeda
4 siblings, 0 replies; 6+ messages in thread
From: Björn Roy Baron @ 2023-01-11 10:43 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Wedson Almeida Filho, Alex Gaynor, Boqun Feng, Gary Guo,
rust-for-linux, linux-kernel, patches, Domen Puncer Kugler
On Monday, January 9th, 2023 at 21:49, Miguel Ojeda <ojeda@kernel.org> 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 = &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: Björn Roy Baron <bjorn3_gh@protonmail.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)+) => (
> - // SAFETY: This hidden macro should only be called by the documented
> - // 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.
> - 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, `format_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 documented
> + // 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 => unsafe {
> + $crate::print::call_printk(
> + &$format_string,
> + crate::__LOG_PREFIX,
> + args,
> + );
> + }
> }
> );
>
>
> base-commit: b7bfaa761d760e72a969d116517eaa12e404c262
> --
> 2.39.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] rust: print: avoid evaluating arguments in `pr_*` macros in `unsafe` blocks
2023-01-09 20:49 [PATCH] rust: print: avoid evaluating arguments in `pr_*` macros in `unsafe` blocks Miguel Ojeda
` (2 preceding siblings ...)
2023-01-11 10:43 ` Björn Roy Baron
@ 2023-01-12 14:25 ` Vincenzo Palazzo
2023-01-16 0:03 ` Miguel Ojeda
4 siblings, 0 replies; 6+ messages in thread
From: Vincenzo Palazzo @ 2023-01-12 14:25 UTC (permalink / raw)
To: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron
Cc: rust-for-linux, linux-kernel, patches, Domen Puncer Kugler
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 = &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)+) => (
> - // SAFETY: This hidden macro should only be called by the documented
> - // 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.
> - 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, `format_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 documented
> + // 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 => unsafe {
> + $crate::print::call_printk(
> + &$format_string,
> + crate::__LOG_PREFIX,
> + args,
> + );
> + }
> }
> );
>
>
> base-commit: b7bfaa761d760e72a969d116517eaa12e404c262
> --
> 2.39.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] rust: print: avoid evaluating arguments in `pr_*` macros in `unsafe` blocks
2023-01-09 20:49 [PATCH] rust: print: avoid evaluating arguments in `pr_*` macros in `unsafe` blocks Miguel Ojeda
` (3 preceding siblings ...)
2023-01-12 14:25 ` Vincenzo Palazzo
@ 2023-01-16 0:03 ` Miguel Ojeda
4 siblings, 0 replies; 6+ messages in thread
From: Miguel Ojeda @ 2023-01-16 0:03 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Wedson Almeida Filho, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, rust-for-linux, linux-kernel, patches,
Domen Puncer Kugler
On Mon, Jan 9, 2023 at 9:49 PM Miguel Ojeda <ojeda@kernel.org> 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 = &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>
Applied to rust-fixes, thanks all!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 6+ messages in thread