public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rust: print: add safety comments for %pA formatting
@ 2026-02-05  4:21 Muchamad Coirul Anwar
  2026-02-05  8:19 ` Alice Ryhl
  2026-02-10 15:00 ` Miguel Ojeda
  0 siblings, 2 replies; 5+ messages in thread
From: Muchamad Coirul Anwar @ 2026-02-05  4:21 UTC (permalink / raw)
  To: ojeda
  Cc: boqun, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross,
	dakr, tamird, gregkh, fujita.tomonori, andrewjballance,
	rust-for-linux, linux-kernel, Muchamad Coirul Anwar

The safety comments in `rust_fmt_argument` and `call_printk` were
previously marked as TODO.

This patch adds the missing safety documentation explaining why
dereferencing the pointers and calling the C `_printk` function
is safe in these contexts. It clarifies the contract between
`lib/vsprintf.c` and the Rust implementation regarding the `%pA`
format specifier.

Signed-off-by: Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>
---
 rust/kernel/print.rs | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index 6fd84389a858..3e6ff8f42d95 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -29,7 +29,11 @@
     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: The C implementation of `vsprintf` (in `lib/vsprintf.c`) specifically
+    // calls this function ONLY when processing the `%pA` format specifier.
+    // On the Rust side (`call_printk`), we guarantee that `%pA` is always paired
+    // with a valid pointer to `fmt::Arguments`.
+    // Therefore, dereferencing `ptr` here is safe.
     let _ = w.write_fmt(unsafe { *ptr.cast::<fmt::Arguments<'_>>() });
     w.pos().cast()
 }
@@ -109,7 +113,9 @@ pub unsafe fn call_printk(
 ) {
     // `_printk` does not seem to fail in any path.
     #[cfg(CONFIG_PRINTK)]
-    // SAFETY: TODO.
+    // SAFETY: The format string is constructed to use `%pA`, which corresponds to the
+    // pointer to `fmt::Arguments` passed as the third argument.
+    // Since `args` is a valid reference, casting it to a pointer is safe.
     unsafe {
         bindings::_printk(
             format_string.as_ptr(),
-- 
2.50.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] rust: print: add safety comments for %pA formatting
  2026-02-05  4:21 [PATCH] rust: print: add safety comments for %pA formatting Muchamad Coirul Anwar
@ 2026-02-05  8:19 ` Alice Ryhl
  2026-02-10 15:00 ` Miguel Ojeda
  1 sibling, 0 replies; 5+ messages in thread
From: Alice Ryhl @ 2026-02-05  8:19 UTC (permalink / raw)
  To: Muchamad Coirul Anwar
  Cc: ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross, dakr,
	tamird, gregkh, fujita.tomonori, andrewjballance, rust-for-linux,
	linux-kernel

On Thu, Feb 05, 2026 at 11:21:32AM +0700, Muchamad Coirul Anwar wrote:
> The safety comments in `rust_fmt_argument` and `call_printk` were
> previously marked as TODO.
> 
> This patch adds the missing safety documentation explaining why
> dereferencing the pointers and calling the C `_printk` function
> is safe in these contexts. It clarifies the contract between
> `lib/vsprintf.c` and the Rust implementation regarding the `%pA`
> format specifier.
> 
> Signed-off-by: Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>
> ---
>  rust/kernel/print.rs | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
> index 6fd84389a858..3e6ff8f42d95 100644
> --- a/rust/kernel/print.rs
> +++ b/rust/kernel/print.rs
> @@ -29,7 +29,11 @@
>      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: The C implementation of `vsprintf` (in `lib/vsprintf.c`) specifically
> +    // calls this function ONLY when processing the `%pA` format specifier.
> +    // On the Rust side (`call_printk`), we guarantee that `%pA` is always paired
> +    // with a valid pointer to `fmt::Arguments`.
> +    // Therefore, dereferencing `ptr` here is safe.

There are multiple places that %pA is passed. For example, there is also
seq_file.rs and probably more.

Perhaps remove the reference to call_printk here?

// SAFETY: The C implementation of `vsprintf` (in `lib/vsprintf.c`) specifically
// calls this function ONLY when processing the `%pA` format specifier.
// On the Rust side, we always pair `%pA` with a valid pointer to
// `fmt::Arguments`.

>      let _ = w.write_fmt(unsafe { *ptr.cast::<fmt::Arguments<'_>>() });
>      w.pos().cast()
>  }
> @@ -109,7 +113,9 @@ pub unsafe fn call_printk(
>  ) {
>      // `_printk` does not seem to fail in any path.
>      #[cfg(CONFIG_PRINTK)]
> -    // SAFETY: TODO.
> +    // SAFETY: The format string is constructed to use `%pA`, which corresponds to the
> +    // pointer to `fmt::Arguments` passed as the third argument.
> +    // Since `args` is a valid reference, casting it to a pointer is safe.
>      unsafe {
>          bindings::_printk(
>              format_string.as_ptr(),
> -- 
> 2.50.0
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] rust: print: add safety comments for %pA formatting
  2026-02-05  4:21 [PATCH] rust: print: add safety comments for %pA formatting Muchamad Coirul Anwar
  2026-02-05  8:19 ` Alice Ryhl
@ 2026-02-10 15:00 ` Miguel Ojeda
       [not found]   ` <CAO26r3TawBQQ6994h+wDTdiobuT8cwU1jGKDs1mh7HdUp=KpLA@mail.gmail.com>
  1 sibling, 1 reply; 5+ messages in thread
From: Miguel Ojeda @ 2026-02-10 15:00 UTC (permalink / raw)
  To: Muchamad Coirul Anwar, Shivendra Sharma
  Cc: ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl,
	tmgross, dakr, tamird, gregkh, fujita.tomonori, andrewjballance,
	rust-for-linux, linux-kernel

On Thu, Feb 5, 2026 at 5:23 AM Muchamad Coirul Anwar
<muchamadcoirulanwar@gmail.com> wrote:
>
> The safety comments in `rust_fmt_argument` and `call_printk` were
> previously marked as TODO.
>
> This patch adds the missing safety documentation explaining why
> dereferencing the pointers and calling the C `_printk` function
> is safe in these contexts. It clarifies the contract between
> `lib/vsprintf.c` and the Rust implementation regarding the `%pA`
> format specifier.
>
> Signed-off-by: Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>

Related:

  https://lore.kernel.org/rust-for-linux/20260128202130.196419-1-shivendra02467@gmail.com/

Cc'ing Shivendra.

Could you (i.e. both) please coordinate the patches, perhaps merging
them into a single series? You may want to use Co-developed-by and/or
have a different author per patch etc.

Thanks!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] rust: print: add safety comments for %pA formatting
       [not found]     ` <CABLcDQBjuZc0edQnoD7SfWj8qEH=ciognT6RPfQPO4r7gcODBA@mail.gmail.com>
@ 2026-02-11 13:44       ` Muchamad Coirul Anwar
  2026-02-11 18:27         ` [PATCH v2] rust: print: add safety documentation for %pA handling Shivendra Sharma
  0 siblings, 1 reply; 5+ messages in thread
From: Muchamad Coirul Anwar @ 2026-02-11 13:44 UTC (permalink / raw)
  To: Shivendra Sharma
  Cc: Miguel Ojeda, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, dakr, tamird, gregkh, fujita.tomonori,
	andrewjballance, rust-for-linux, linux-kernel

Hi Shivendra,

That sounds great, I agree with the plan.

The wording for the SAFETY comment matches exactly what Alice
suggested, so it looks good to me.

Please feel free to include my tags:

Co-developed-by: Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>
Signed-off-by: Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>

Thanks for taking the lead on the v2

Best regards,
Muchamad Coirul Anwar


Pada Rab, 11 Feb 2026 pukul 16.58 Shivendra Sharma
<shivendra02467@gmail.com> menulis:
>
> Hi Muchamad, Miguel,
>
> Thanks for the response! I’m happy to take the lead on the v2 patch.
>
> Regarding the safety comments, I think Alice actually gave us the perfect wording in her reply. She suggested removing the specific reference to call_printk to make it more generic since %pA is used in other places like seq_file.rs.
>
> To keep things simple and ensure we follow her suggestion exactly, I’ll use this text for the SAFETY comment:
>
> // SAFETY: The C implementation of `vsprintf` (in `lib/vsprintf.c`) specifically
>
> // calls this function ONLY when processing the `%pA` format specifier.
>
> // On the Rust side, we always pair `%pA` with a valid pointer to
>
> // `fmt::Arguments`.
>
> Muchamad, if you're okay with that, I'll integrate this into the v2 along with the cleanup from my previous patch and Miguel's earlier suggestions. I'll include you as a Co-author.
>
> Does that sound good to everyone?
>
> Best regards,
>
> Shivendra
>
>
> On Wed, Feb 11, 2026, 3:12 PM Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com> wrote:
>>
>> Hi Miguel, Shivendra,
>>
>> Thanks for the heads-up, Miguel, I'd be happy to collaborate with Shivendra on this. Hi Shivendra, I checked your patch and I really like how you handled the documentation and the C-header part. It’s definitely cleaner than my initial attempt. That said, I just got some feedback from Alice Ryhl regarding the internal '// SAFETY' comments. She pointed out that we should make the safety guarantee generic (covering `seq_file` usage too, not just `printk`). How about we combine our work? You could take the lead on the v2 patch, and I can provide the updated safety comment text that covers Alice's feedback. Let me know what you think.
>>
>> Best regards,
>> Muchamad Coirul Anwar
>>
>> Pada Sel, 10 Feb 2026 pukul 22.00 Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> menulis:
>>>
>>> On Thu, Feb 5, 2026 at 5:23 AM Muchamad Coirul Anwar
>>> <muchamadcoirulanwar@gmail.com> wrote:
>>> >
>>> > The safety comments in `rust_fmt_argument` and `call_printk` were
>>> > previously marked as TODO.
>>> >
>>> > This patch adds the missing safety documentation explaining why
>>> > dereferencing the pointers and calling the C `_printk` function
>>> > is safe in these contexts. It clarifies the contract between
>>> > `lib/vsprintf.c` and the Rust implementation regarding the `%pA`
>>> > format specifier.
>>> >
>>> > Signed-off-by: Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>
>>>
>>> Related:
>>>
>>>   https://lore.kernel.org/rust-for-linux/20260128202130.196419-1-shivendra02467@gmail.com/
>>>
>>> Cc'ing Shivendra.
>>>
>>> Could you (i.e. both) please coordinate the patches, perhaps merging
>>> them into a single series? You may want to use Co-developed-by and/or
>>> have a different author per patch etc.
>>>
>>> Thanks!
>>>
>>> Cheers,
>>> Miguel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] rust: print: add safety documentation for %pA handling
  2026-02-11 13:44       ` Muchamad Coirul Anwar
@ 2026-02-11 18:27         ` Shivendra Sharma
  0 siblings, 0 replies; 5+ messages in thread
From: Shivendra Sharma @ 2026-02-11 18:27 UTC (permalink / raw)
  To: Muchamad Coirul Anwar, Miguel Ojeda
  Cc: Alice Ryhl, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
	Tamir Duberstein, Greg Kroah-Hartman, rust-for-linux,
	linux-kernel, Shivendra Sharma

Add missing safety documentation to `rust_fmt_argument` and `call_printk`.
These comments clarify the safety requirements for dereferencing
pointers and calling the C `_printk` function, specifically
addressing the contract between `lib/vsprintf.c` and Rust regarding
the `%pA` format specifier.

The safety guarantee is made generic to cover all users of `%pA`,
including `seq_file.rs`.

Suggested-by: Alice Ryhl <aliceryhl@google.com>
Co-developed-by: Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>
Signed-off-by: Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>
Signed-off-by: Shivendra Sharma <shivendra02467@gmail.com>
---
v2:
  - Combined work with Muchamad Coirul Anwar.
  - Refined safety comments to be generic per Alice Ryhl's feedback.
  - Added formal "# Safety" section to rust_fmt_argument per Miguel Ojeda's
    previous suggestion.
  - Added documentation to include/linux/sprintf.h.
  - Links to previous versions:
    v1 (Shivendra): https://lore.kernel.org/rust-for-linux/20260128202130.196419-1-shivendra02467@gmail.com/
    v1 (Muchamad): https://lore.kernel.org/rust-for-linux/20260205042132.40772-1-muchamadcoirulanwar@gmail.com/

 include/linux/sprintf.h |  7 ++++++-
 rust/kernel/print.rs    | 19 +++++++++++++++----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/include/linux/sprintf.h b/include/linux/sprintf.h
index f06f7b785091..f915829cbba5 100644
--- a/include/linux/sprintf.h
+++ b/include/linux/sprintf.h
@@ -25,7 +25,12 @@ __scanf(2, 0) int vsscanf(const char *, const char *, va_list);
 extern bool no_hash_pointers;
 void hash_pointers_finalize(bool slub_debug);
 
-/* Used for Rust formatting ('%pA') */
+/**
+ * Used for Rust formatting ('%pA').
+ *
+ * Safety preconditions are documented in the Rust implementation
+ * (rust/kernel/print.rs).
+ */
 char *rust_fmt_argument(char *buf, char *end, const void *ptr);
 
 #endif	/* _LINUX_KERNEL_SPRINTF_H */
diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index 6fd84389a858..15d4039a7646 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -19,7 +19,12 @@
 };
 
 // Called from `vsprintf` with format specifier `%pA`.
-#[expect(clippy::missing_safety_doc)]
+///
+/// # Safety
+///
+/// - `buf` must be valid for writes until `end`.
+/// - `ptr` must point to a valid `fmt::Arguments` instance.
+/// - The `fmt::Arguments` must remain valid for the duration of the call.
 #[export]
 unsafe extern "C" fn rust_fmt_argument(
     buf: *mut c_char,
@@ -27,9 +32,13 @@
     ptr: *const c_void,
 ) -> *mut c_char {
     use fmt::Write;
-    // SAFETY: The C contract guarantees that `buf` is valid if it's less than `end`.
+    // SAFETY: The safety requirements of this function (see above)
+    // guarantee that `buf` and `end` define a valid range.
     let mut w = unsafe { RawFormatter::from_ptrs(buf.cast(), end.cast()) };
-    // SAFETY: TODO.
+    // SAFETY: The C implementation of `vsprintf` (in `lib/vsprintf.c`) specifically
+    // calls this function ONLY when processing the `%pA` format specifier.
+    // On the Rust side, we always pair `%pA` with a valid pointer to
+    // `fmt::Arguments`, satisfying the requirements of this function (see above).
     let _ = w.write_fmt(unsafe { *ptr.cast::<fmt::Arguments<'_>>() });
     w.pos().cast()
 }
@@ -109,7 +118,9 @@ pub unsafe fn call_printk(
 ) {
     // `_printk` does not seem to fail in any path.
     #[cfg(CONFIG_PRINTK)]
-    // SAFETY: TODO.
+    // SAFETY: The format string is constructed to use `%pA`, which corresponds to the
+    // pointer to `fmt::Arguments` passed as the third argument.
+    // Since `args` is a valid reference, casting it to a pointer is safe.
     unsafe {
         bindings::_printk(
             format_string.as_ptr(),
-- 
2.51.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-02-11 18:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-05  4:21 [PATCH] rust: print: add safety comments for %pA formatting Muchamad Coirul Anwar
2026-02-05  8:19 ` Alice Ryhl
2026-02-10 15:00 ` Miguel Ojeda
     [not found]   ` <CAO26r3TawBQQ6994h+wDTdiobuT8cwU1jGKDs1mh7HdUp=KpLA@mail.gmail.com>
     [not found]     ` <CABLcDQBjuZc0edQnoD7SfWj8qEH=ciognT6RPfQPO4r7gcODBA@mail.gmail.com>
2026-02-11 13:44       ` Muchamad Coirul Anwar
2026-02-11 18:27         ` [PATCH v2] rust: print: add safety documentation for %pA handling Shivendra Sharma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox