* [PATCH v3 2/4] rust: print: document safety of _printk FFI calls
@ 2026-07-26 11:59 Cian McGuire
2026-07-27 10:50 ` Gary Guo
2026-07-29 17:53 ` [PATCH v4 " cianM167
0 siblings, 2 replies; 4+ messages in thread
From: Cian McGuire @ 2026-07-26 11:59 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Boqun Feng, Gary Guo, Alice Ryhl, Lyude Paul, Daniel Almeida,
Onur Özkan, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Tamir Duberstein, Alexandre Courbot, FUJITA Tomonori,
Greg Kroah-Hartman, rust-for-linux, linux-kernel, Cian McGuire
Two calls in this file had their SAFETY comments left as "TODO.": the
cast back to `fmt::Arguments` in `rust_fmt_argument`, and the call to
`bindings::_printk` in `call_printk`.
`rust_fmt_argument` is only reachable through the `%pA` format
specifier. Every in-tree Rust caller that uses `%pA` -- `call_printk`,
`device::printk`, `kunit::printk`, and `seq_file::call_printf` -- invokes
it synchronously with a pointer derived from its own live
`fmt::Arguments`, as one call in a `switch` dispatch in
`lib/vsprintf.c`'s pointer formatter that returns its result immediately;
there is no deferred use or storage of the pointer on the C side. So the
`fmt::Arguments` value stays alive on the caller's stack for the whole
time `rust_fmt_argument` is running with `ptr` pointing at it.
The `_printk` call itself is justified by this function's `# Safety`
section, which this patch also tightens: `format_string` must be one of
the non-continuation `format_strings` statics, not
`format_strings::CONT`. `call_printk` always supplies two extra
arguments -- `module_name` for `%s` and `args` for `%pA` -- which only
matches the `"%s: %pA"` shape shared by every non-continuation string;
`CONT`'s format string is `"%pA"` only, so passing it here would make
`_printk` consume `module_name`'s pointer for `%pA` instead, and
`rust_fmt_argument` would then reinterpret that `*const c_char` as a
`*const fmt::Arguments` and dereference it. No caller does this today --
`call_printk` is only reached from `print_macro!`'s non-continuation arm;
the continuation arm calls the separate, safe `call_printk_cont`, which
hardcodes `CONT` together with the single argument it expects -- but the
previous, broader contract permitted it. With `format_string` guaranteed
non-`CONT`, and `module_name` guaranteed null-terminated by both of its
possible origins (`kernel::__LOG_PREFIX` and the one generated by the
`module!` proc macro), the specifiers always match the supplied
arguments.
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-Linux/linux/issues/351
Signed-off-by: Cian McGuire <cianmcguire167@gmail.com>
---
v3:
- Tightened `call_printk`'s `# Safety` contract to require a
non-continuation format string (i.e. not `format_strings::CONT`),
since the function always supplies both a `%s` and a `%pA` argument,
which only matches the non-continuation strings' `"%s: %pA"` shape.
Passing `CONT` under the old, broader contract would make `_printk`
consume `module_name`'s pointer for `%pA` instead of `args`, which
`rust_fmt_argument` would then wrongly reinterpret as a
`*const fmt::Arguments` and dereference. Updated the inline SAFETY
comment to match.
v2:
- Reworded the SAFETY comment on `rust_fmt_argument`'s cast to argue
from the property that makes every `%pA` call site safe (synchronous
invocation with a live `&fmt::Arguments`), instead of enumerating
callers by name, so it doesn't go stale if a new caller is added.
rust/kernel/print.rs | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index 6fd84389a858..5a0ee103b54e 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -29,7 +29,10 @@
use fmt::Write;
// SAFETY: The C contract guarantees that `buf` is valid if it's less than `end`.
let mut w = unsafe { RawFormatter::from_ptrs(buf.cast(), end.cast()) };
- // SAFETY: TODO.
+ // SAFETY: This function is only reachable via the `%pA` format specifier; using `%pA` from
+ // Rust requires the caller to invoke it synchronously with a `ptr` derived from a live
+ // `&fmt::Arguments`, before that reference's scope can end. `ptr` is therefore valid and
+ // properly aligned for a `fmt::Arguments<'_>` for the duration of this call.
let _ = w.write_fmt(unsafe { *ptr.cast::<fmt::Arguments<'_>>() });
w.pos().cast()
}
@@ -96,8 +99,11 @@ pub mod format_strings {
///
/// # Safety
///
-/// The format string must be one of the ones in [`format_strings`], and
-/// the module name must be null-terminated.
+/// The format string must be one of the non-continuation strings in
+/// [`format_strings`] (i.e. not [`format_strings::CONT`]): this function
+/// always supplies both `module_name` and `args` as arguments, which only
+/// matches the `"%s: %pA"` shape used by the non-continuation strings, not
+/// `CONT`'s `"%pA"`-only shape. The module name must be null-terminated.
///
/// [`_printk`]: srctree/include/linux/_printk.h
#[doc(hidden)]
@@ -109,7 +115,11 @@ pub unsafe fn call_printk(
) {
// `_printk` does not seem to fail in any path.
#[cfg(CONFIG_PRINTK)]
- // SAFETY: TODO.
+ // SAFETY: By this function's safety requirements, `format_string` is one of the
+ // non-continuation `format_strings` statics (never `CONT`), so its fixed `"%s: %pA"` shape
+ // matches the two arguments supplied below: `module_name` is null-terminated as `%s`
+ // requires, and the third argument points at `args`, which remains valid for the duration
+ // of this call and is only read back synchronously, through `%pA`, by `rust_fmt_argument`.
unsafe {
bindings::_printk(
format_string.as_ptr(),
--
2.55.0
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3 2/4] rust: print: document safety of _printk FFI calls
2026-07-26 11:59 [PATCH v3 2/4] rust: print: document safety of _printk FFI calls Cian McGuire
@ 2026-07-27 10:50 ` Gary Guo
2026-07-29 17:53 ` [PATCH v4 " cianM167
1 sibling, 0 replies; 4+ messages in thread
From: Gary Guo @ 2026-07-27 10:50 UTC (permalink / raw)
To: Cian McGuire, Miguel Ojeda
Cc: Boqun Feng, Gary Guo, Alice Ryhl, Lyude Paul, Daniel Almeida,
Onur Özkan, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Tamir Duberstein, Alexandre Courbot, FUJITA Tomonori,
Greg Kroah-Hartman, rust-for-linux, linux-kernel
On Sun Jul 26, 2026 at 12:59 PM BST, Cian McGuire wrote:
> Two calls in this file had their SAFETY comments left as "TODO.": the
> cast back to `fmt::Arguments` in `rust_fmt_argument`, and the call to
> `bindings::_printk` in `call_printk`.
>
> `rust_fmt_argument` is only reachable through the `%pA` format
> specifier. Every in-tree Rust caller that uses `%pA` -- `call_printk`,
> `device::printk`, `kunit::printk`, and `seq_file::call_printf` -- invokes
> it synchronously with a pointer derived from its own live
> `fmt::Arguments`, as one call in a `switch` dispatch in
> `lib/vsprintf.c`'s pointer formatter that returns its result immediately;
> there is no deferred use or storage of the pointer on the C side. So the
> `fmt::Arguments` value stays alive on the caller's stack for the whole
> time `rust_fmt_argument` is running with `ptr` pointing at it.
>
> The `_printk` call itself is justified by this function's `# Safety`
> section, which this patch also tightens: `format_string` must be one of
> the non-continuation `format_strings` statics, not
> `format_strings::CONT`. `call_printk` always supplies two extra
> arguments -- `module_name` for `%s` and `args` for `%pA` -- which only
> matches the `"%s: %pA"` shape shared by every non-continuation string;
> `CONT`'s format string is `"%pA"` only, so passing it here would make
> `_printk` consume `module_name`'s pointer for `%pA` instead, and
> `rust_fmt_argument` would then reinterpret that `*const c_char` as a
> `*const fmt::Arguments` and dereference it. No caller does this today --
> `call_printk` is only reached from `print_macro!`'s non-continuation arm;
> the continuation arm calls the separate, safe `call_printk_cont`, which
> hardcodes `CONT` together with the single argument it expects -- but the
> previous, broader contract permitted it. With `format_string` guaranteed
> non-`CONT`, and `module_name` guaranteed null-terminated by both of its
> possible origins (`kernel::__LOG_PREFIX` and the one generated by the
> `module!` proc macro), the specifiers always match the supplied
> arguments.
>
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://github.com/Rust-for-Linux/linux/issues/351
> Signed-off-by: Cian McGuire <cianmcguire167@gmail.com>
> ---
> v3:
> - Tightened `call_printk`'s `# Safety` contract to require a
> non-continuation format string (i.e. not `format_strings::CONT`),
> since the function always supplies both a `%s` and a `%pA` argument,
> which only matches the non-continuation strings' `"%s: %pA"` shape.
> Passing `CONT` under the old, broader contract would make `_printk`
> consume `module_name`'s pointer for `%pA` instead of `args`, which
> `rust_fmt_argument` would then wrongly reinterpret as a
> `*const fmt::Arguments` and dereference. Updated the inline SAFETY
> comment to match.
> v2:
> - Reworded the SAFETY comment on `rust_fmt_argument`'s cast to argue
> from the property that makes every `%pA` call site safe (synchronous
> invocation with a live `&fmt::Arguments`), instead of enumerating
> callers by name, so it doesn't go stale if a new caller is added.
>
> rust/kernel/print.rs | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
> index 6fd84389a858..5a0ee103b54e 100644
> --- a/rust/kernel/print.rs
> +++ b/rust/kernel/print.rs
> @@ -29,7 +29,10 @@
> use fmt::Write;
> // SAFETY: The C contract guarantees that `buf` is valid if it's less than `end`.
> let mut w = unsafe { RawFormatter::from_ptrs(buf.cast(), end.cast()) };
> - // SAFETY: TODO.
> + // SAFETY: This function is only reachable via the `%pA` format specifier; using `%pA` from
> + // Rust requires the caller to invoke it synchronously with a `ptr` derived from a live
> + // `&fmt::Arguments`, before that reference's scope can end. `ptr` is therefore valid and
> + // properly aligned for a `fmt::Arguments<'_>` for the duration of this call.
Bulk of this should be safety precondition of the function, not safety comment.
Best,
Gary
> let _ = w.write_fmt(unsafe { *ptr.cast::<fmt::Arguments<'_>>() });
> w.pos().cast()
> }
> @@ -96,8 +99,11 @@ pub mod format_strings {
> ///
> /// # Safety
> ///
> -/// The format string must be one of the ones in [`format_strings`], and
> -/// the module name must be null-terminated.
> +/// The format string must be one of the non-continuation strings in
> +/// [`format_strings`] (i.e. not [`format_strings::CONT`]): this function
> +/// always supplies both `module_name` and `args` as arguments, which only
> +/// matches the `"%s: %pA"` shape used by the non-continuation strings, not
> +/// `CONT`'s `"%pA"`-only shape. The module name must be null-terminated.
> ///
> /// [`_printk`]: srctree/include/linux/_printk.h
> #[doc(hidden)]
> @@ -109,7 +115,11 @@ pub unsafe fn call_printk(
> ) {
> // `_printk` does not seem to fail in any path.
> #[cfg(CONFIG_PRINTK)]
> - // SAFETY: TODO.
> + // SAFETY: By this function's safety requirements, `format_string` is one of the
> + // non-continuation `format_strings` statics (never `CONT`), so its fixed `"%s: %pA"` shape
> + // matches the two arguments supplied below: `module_name` is null-terminated as `%s`
> + // requires, and the third argument points at `args`, which remains valid for the duration
> + // of this call and is only read back synchronously, through `%pA`, by `rust_fmt_argument`.
> unsafe {
> bindings::_printk(
> format_string.as_ptr(),
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v4 2/4] rust: print: document safety of _printk FFI calls
2026-07-26 11:59 [PATCH v3 2/4] rust: print: document safety of _printk FFI calls Cian McGuire
2026-07-27 10:50 ` Gary Guo
@ 2026-07-29 17:53 ` cianM167
2026-08-03 7:54 ` Miguel Ojeda
1 sibling, 1 reply; 4+ messages in thread
From: cianM167 @ 2026-07-29 17:53 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Boqun Feng, Gary Guo, Alice Ryhl, Lyude Paul, Daniel Almeida,
Onur Özkan, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Tamir Duberstein, Alexandre Courbot, FUJITA Tomonori,
Greg Kroah-Hartman, rust-for-linux, linux-kernel, Cian McGuire
From: Cian McGuire <cianmcguire167@gmail.com>
Two calls in this file had their SAFETY comments left as "TODO.": the
cast back to `fmt::Arguments` in `rust_fmt_argument`, and the call to
`bindings::_printk` in `call_printk`.
`rust_fmt_argument` is only reachable through the `%pA` format
specifier. Every in-tree Rust caller that uses `%pA` -- `call_printk`,
`device::printk`, `kunit::printk`, and `seq_file::call_printf` -- invokes
it synchronously with a pointer derived from its own live
`fmt::Arguments`, as one call in a `switch` dispatch in
`lib/vsprintf.c`'s pointer formatter that returns its result immediately;
there is no deferred use or storage of the pointer on the C side. So the
`fmt::Arguments` value stays alive on the caller's stack for the whole
time `rust_fmt_argument` is running with `ptr` pointing at it.
The `_printk` call itself is justified by this function's `# Safety`
section, which this patch also tightens: `format_string` must be one of
the non-continuation `format_strings` statics, not
`format_strings::CONT`. `call_printk` always supplies two extra
arguments -- `module_name` for `%s` and `args` for `%pA` -- which only
matches the `"%s: %pA"` shape shared by every non-continuation string;
`CONT`'s format string is `"%pA"` only, so passing it here would make
`_printk` consume `module_name`'s pointer for `%pA` instead, and
`rust_fmt_argument` would then reinterpret that `*const c_char` as a
`*const fmt::Arguments` and dereference it. No caller does this today --
`call_printk` is only reached from `print_macro!`'s non-continuation arm;
the continuation arm calls the separate, safe `call_printk_cont`, which
hardcodes `CONT` together with the single argument it expects -- but the
previous, broader contract permitted it. With `format_string` guaranteed
non-`CONT`, and `module_name` guaranteed null-terminated by both of its
possible origins (`kernel::__LOG_PREFIX` and the one generated by the
`module!` proc macro), the specifiers always match the supplied
arguments.
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-Linux/linux/issues/351
Signed-off-by: Cian McGuire <cianmcguire167@gmail.com>
---
v4:
- Trim the `// SAFETY:` comment on the `_printk` call in `call_printk`
down to what's specific to this call site (that `args` stays valid and
is read back synchronously). The shape-matching reasoning it used to
repeat is already covered by the `# Safety` section above it, which is
the right place for it as a precondition rather than a per-call-site
justification. (Gary Guo)
v3:
- Tightened `call_printk`'s `# Safety` contract to require a
non-continuation format string (i.e. not `format_strings::CONT`),
since the function always supplies both a `%s` and a `%pA` argument,
which only matches the non-continuation strings' `"%s: %pA"` shape.
Passing `CONT` under the old, broader contract would make `_printk`
consume `module_name`'s pointer for `%pA` instead of `args`, which
`rust_fmt_argument` would then wrongly reinterpret as a
`*const fmt::Arguments` and dereference. Updated the inline SAFETY
comment to match.
v2:
- Reworded the SAFETY comment on `rust_fmt_argument`'s cast to argue
from the property that makes every `%pA` call site safe (synchronous
invocation with a live `&fmt::Arguments`), instead of enumerating
callers by name, so it doesn't go stale if a new caller is added.
rust/kernel/print.rs | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index 6fd84389a858..3111f8f4c2e1 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -29,7 +29,10 @@
use fmt::Write;
// SAFETY: The C contract guarantees that `buf` is valid if it's less than `end`.
let mut w = unsafe { RawFormatter::from_ptrs(buf.cast(), end.cast()) };
- // SAFETY: TODO.
+ // SAFETY: This function is only reachable via the `%pA` format specifier; using `%pA` from
+ // Rust requires the caller to invoke it synchronously with a `ptr` derived from a live
+ // `&fmt::Arguments`, before that reference's scope can end. `ptr` is therefore valid and
+ // properly aligned for a `fmt::Arguments<'_>` for the duration of this call.
let _ = w.write_fmt(unsafe { *ptr.cast::<fmt::Arguments<'_>>() });
w.pos().cast()
}
@@ -96,8 +99,11 @@ pub mod format_strings {
///
/// # Safety
///
-/// The format string must be one of the ones in [`format_strings`], and
-/// the module name must be null-terminated.
+/// The format string must be one of the non-continuation strings in
+/// [`format_strings`] (i.e. not [`format_strings::CONT`]): this function
+/// always supplies both `module_name` and `args` as arguments, which only
+/// matches the `"%s: %pA"` shape used by the non-continuation strings, not
+/// `CONT`'s `"%pA"`-only shape. The module name must be null-terminated.
///
/// [`_printk`]: srctree/include/linux/_printk.h
#[doc(hidden)]
@@ -109,7 +115,9 @@ pub unsafe fn call_printk(
) {
// `_printk` does not seem to fail in any path.
#[cfg(CONFIG_PRINTK)]
- // SAFETY: TODO.
+ // SAFETY: By this function's safety requirements, `format_string` and `module_name` match
+ // `_printk`'s expectations. `args` remains valid for the duration of this call and is only
+ // read back synchronously, through `%pA`, by `rust_fmt_argument`.
unsafe {
bindings::_printk(
format_string.as_ptr(),
--
2.55.0
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v4 2/4] rust: print: document safety of _printk FFI calls
2026-07-29 17:53 ` [PATCH v4 " cianM167
@ 2026-08-03 7:54 ` Miguel Ojeda
0 siblings, 0 replies; 4+ messages in thread
From: Miguel Ojeda @ 2026-08-03 7:54 UTC (permalink / raw)
To: cianM167
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Alice Ryhl, Lyude Paul,
Daniel Almeida, Onur Özkan, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Tamir Duberstein, Alexandre Courbot, FUJITA Tomonori,
Greg Kroah-Hartman, rust-for-linux, linux-kernel
On Wed, Jul 29, 2026 at 7:53 PM cianM167 <cianmcguire167@gmail.com> wrote:
>
> From: Cian McGuire <cianmcguire167@gmail.com>
>
> Two calls in this file had their SAFETY comments left as "TODO.": the
> cast back to `fmt::Arguments` in `rust_fmt_argument`, and the call to
> `bindings::_printk` in `call_printk`.
>
> `rust_fmt_argument` is only reachable through the `%pA` format
> specifier. Every in-tree Rust caller that uses `%pA` -- `call_printk`,
> `device::printk`, `kunit::printk`, and `seq_file::call_printf` -- invokes
> it synchronously with a pointer derived from its own live
> `fmt::Arguments`, as one call in a `switch` dispatch in
> `lib/vsprintf.c`'s pointer formatter that returns its result immediately;
> there is no deferred use or storage of the pointer on the C side. So the
> `fmt::Arguments` value stays alive on the caller's stack for the whole
> time `rust_fmt_argument` is running with `ptr` pointing at it.
>
> The `_printk` call itself is justified by this function's `# Safety`
> section, which this patch also tightens: `format_string` must be one of
> the non-continuation `format_strings` statics, not
> `format_strings::CONT`. `call_printk` always supplies two extra
> arguments -- `module_name` for `%s` and `args` for `%pA` -- which only
> matches the `"%s: %pA"` shape shared by every non-continuation string;
> `CONT`'s format string is `"%pA"` only, so passing it here would make
> `_printk` consume `module_name`'s pointer for `%pA` instead, and
> `rust_fmt_argument` would then reinterpret that `*const c_char` as a
> `*const fmt::Arguments` and dereference it. No caller does this today --
> `call_printk` is only reached from `print_macro!`'s non-continuation arm;
> the continuation arm calls the separate, safe `call_printk_cont`, which
> hardcodes `CONT` together with the single argument it expects -- but the
> previous, broader contract permitted it. With `format_string` guaranteed
> non-`CONT`, and `module_name` guaranteed null-terminated by both of its
> possible origins (`kernel::__LOG_PREFIX` and the one generated by the
> `module!` proc macro), the specifiers always match the supplied
> arguments.
>
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://github.com/Rust-for-Linux/linux/issues/351
> Signed-off-by: Cian McGuire <cianmcguire167@gmail.com>
Thanks for the patch!
A few notes:
- It seems the "From:" in the email (not the Git commit) changed in
v4. Not a big deal, but I am not sure if it is intended.
- Is there a reason you are sending only 2/4 in the new versions?
If the patches are independent (and especially if they also target
different subsystems), then it may be best to send them separately.
Otherwise, if you are sending a series, then please re-send the entire
series.
(Some developers do send sometimes a single update to a long
series in the form of a patch or a diff, but even that is typically
done replying to the original series.)
For this case, I would suggest resending them separately.
- Just in case: was the commit, or its message and/or changelog
assisted by an LLM? Please see:
https://docs.kernel.org/process/generated-content.html
https://docs.kernel.org/process/coding-assistants.html
Cheers,
Miguel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-03 7:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-26 11:59 [PATCH v3 2/4] rust: print: document safety of _printk FFI calls Cian McGuire
2026-07-27 10:50 ` Gary Guo
2026-07-29 17:53 ` [PATCH v4 " cianM167
2026-08-03 7:54 ` Miguel Ojeda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®