Implement dynamic debug logging mask for Rust Binder#3

jahnavi-MN496 · 2026-07-08T07:41:57Z

No description provided.

Darksonn · 2026-07-08T08:15:57Z

Patch 2 (rust_binder: Implement the BINDER_DEBUG_USER_ERROR logging mask for freezer-related operation) looks good!

Darksonn · 2026-07-08T08:18:36Z

Patch 3 (rust_binder: Implement the BINDER_DEBUG_USER_ERROR logging mask for reference counting and death notification operations) looks good, but the title is a bit long. Perhaps we can shorten to:

rust_binder: Implement BINDER_DEBUG_USER_ERROR for refcounting and death notifications

Darksonn

Patch 4 (rust_binder: Implement the BINDER_DEBUG_USER_ERROR logging mask for transaction parsing and protocol validation failures) also looks reasonable. I have one comment below.

However, the title is pretty long. I think "transaction parsing" and "protocol violation" is the same thing. We can just write:

rust_binder: Implement BINDER_DEBUG_USER_ERROR for transaction parsing failures

Darksonn

Some comments on commit rust_binder: Implement the BINDER_DEBUG_FAILED_TRANSACTION logging mask for transaction parsing and routing failures

We can shorten the title to:

rust_binder: Implement BINDER_DEBUG_FAILED_TRANSACTION logging

because I think this includes all cases where we want to print BINDER_DEBUG_FAILED_TRANSACTION.

Darksonn

It looks like the BINDER_DEBUG_DEATH_NOTIFICATION commit is missing most some printlns.

In C Binder, this category prints in these cases:

Death notification is requested.
Death notification is cleared.
Death notification is delivered to userspace.

So I think to implement this category correctly, we should print in the same cases.

Darksonn

rust_binder: Implement the BINDER_DEBUG_DEAD_TRANSACTION logging mask to trace in-flight cancellations during teardown

First, the commit title is a bit long. I think we can shorten to:

rust_binder: Implement BINDER_DEBUG_DEAD_TRANSACTION

Darksonn · 2026-07-10T07:05:39Z

+        kernel::pr_info!($($arg)*);
+    };
+
+    // 2. Rule to explicitly specify a PID (used in deferred flush/release).


It may be a bit clearer to say that it's used from kworkers/the workqueue, since then people can deduce that it's being used there because they don't have a pid.

Suggested change

// 2. Rule to explicitly specify a PID (used in deferred flush/release).

// 2. Rule to explicitly specify a PID (used in kworkers).

Darksonn · 2026-07-10T07:08:32Z

+                binder_debug!(
+                    FailedTransaction,
+                    "{}:{} transaction {} to {}:{} failed {:?}, code {} size {}-{}",
                    info.from_pid,
                    info.from_tid,


Since binder_debug! already includes the pid/tid pair at the beginning, this is printing it double.

Darksonn · 2026-07-10T07:08:50Z

            }
+
        };


The commit rust_binder: Implement BINDER_DEBUG_FAILED_TRANSACTION has a spurious added newline here.

Darksonn · 2026-07-10T07:13:26Z

 pub(crate) struct FreezeMessage {
    cookie: FreezeCookie,
+    pid: i32,
 }


For commit rust_binder: Implement BINDER_DEBUG_DEAD_TRANSACTION, I would add an explanation to the commit message saying that you are adding the pid to some structs so that they can be used when printing in cancel() because the cancel() method is often called from a workqueue (typically from deferred_release).

Darksonn · 2026-07-10T07:15:41Z

            }
-
        };


The commit rust_binder: Implement BINDER_DEBUG_FAILED_TRANSACTION has a spurious added newline here.

When a user-space application sends malformed data or makes a lifecycle mistake, the driver rejects it with a generic error code (like -EINVAL). Without internal logs, the driver acts as a "black box," forcing developers to guess which check failed. In the legacy C Binder driver, this issue is solved using a dynamic debug_mask module parameter that toggles verbose logs for specific subsystems. This series brings the same critical capability to the Rust Binder driver to provide developers with clear, real-time feedback. Instead of rebuilds, reboots, or guessing: - Developers can enable logs instantly on a running device by writing to `/sys/module/rust_binder/parameters/debug_mask`. - It prints the exact reason for failures (such as alignment errors, mismatched call stacks, or invalid handle references) directly into `dmesg`, reducing debugging time from hours to seconds. - It protects system logs by keeping logging off by default and only enabling it when developers are actively troubleshooting. Based on top of: https://lore.kernel.org/rust-for-linux/20260707-upgrade-poll-v6-0-4b8fae7bf1d9@google.com/ # Describe the purpose of this series. The information you put here # will be used by the project maintainer to make a decision whether # your patches should be reviewed, and in what priority order. Please be # very detailed and link to any relevant discussions or sites that the # maintainer can review to better understand your proposed changes. If you # only have a single patch in your series, the contents of the cover # letter will be appended to the "under-the-cut" portion of the patch. # Lines starting with # will be removed from the cover letter. You can # use them to add notes or reminders to yourself. If you want to use # markdown headers in your cover letter, start the line with ">#". # You can add trailers to the cover letter. Any email addresses found in # these trailers will be added to the addresses specified/generated # during the b4 send stage. You can also run "b4 prep --auto-to-cc" to # auto-populate the To: and Cc: trailers based on the code being # modified. To: Greg Kroah-Hartman <gregkh@linuxfoundation.org> To: Arve Hjønnevåg <arve@android.com> To: Todd Kjos <tkjos@android.com> To: Christian Brauner <brauner@kernel.org> To: Carlos Llamas <cmllamas@google.com> To: Alice Ryhl <aliceryhl@google.com> To: Miguel Ojeda <ojeda@kernel.org> To: Boqun Feng <boqun@kernel.org> To: Gary Guo <gary@garyguo.net> To: Björn Roy Baron <bjorn3_gh@protonmail.com> To: Benno Lossin <lossin@kernel.org> To: Andreas Hindborg <a.hindborg@kernel.org> To: Trevor Gross <tmgross@umich.edu> To: Danilo Krummrich <dakr@kernel.org> To: Daniel Almeida <daniel.almeida@collabora.com> To: Tamir Duberstein <tamird@kernel.org> To: Alexandre Courbot <acourbot@nvidia.com> To: Onur Özkan <work@onurozkan.dev> Cc: linux-kernel@vger.kernel.org Cc: rust-for-linux@vger.kernel.org Signed-off-by: Jahnavi MN <jahnavimn@google.com> --- Changes in v3: - EDITME: describe what is new in this series revision. - EDITME: use bulletpoints and terse descriptions. - Link to v2: https://lore.kernel.org/r/20260710-rust_binder_debug_mask-v2-0-2846410e3ae6@google.com Changes in v2: - Defined the debug mask categories using bitflags (impl_flags) to resolve potential Undefined Behavior and match Rust idioms. - Added a tgid helper to rust/kernel/task.rs to expose the task group ID and optimize default "PID:TID" prefixing. - Implemented the BINDER_DEBUG_DEATH_NOTIFICATION mask to log OOM failures, lifecycle updates, and async delivery to user-space. - Added pid fields to ThreadError, DeliverCode, and FreezeMessage to ensure cancellation logs show the correct process PID instead of background kworker PIDs. - Removed duplicate PID printing across workqueue, transaction failure, and stack unwinding logs. - Refactored log formatting (e.g. formatted BinderError with {:?}, used "strong"/"weak" strings, and improved message wording). - Fixed logic bugs (restored missing update_ref block, moved manager lookup warnings). - Cleaned up spurious newlines, corrected indentations, and adjusted all commit titles to be shorter and more meaningful. - Link to v1: https://lore.kernel.org/r/20260703-rust_binder_debug_mask-v1-0-9bdf12b5325c@google.com --- b4-submit-tracking --- # This section is used internally by b4 prep for tracking purposes. { "series": { "revision": 3, "change-id": "20260702-rust_binder_debug_mask-636737015624", "prefixes": [], "history": { "v1": [ "20260703-rust_binder_debug_mask-v1-0-9bdf12b5325c@google.com" ], "v2": [ "20260710-rust_binder_debug_mask-v2-0-2846410e3ae6@google.com" ] } } }

Implement a dynamic debug logging mask (`debug_mask`) for the `rust_binder` module to allow dynamic runtime configuration of log levels. This enables parity with the legacy C driver's debug mask. Since the Rust `module!` macro in the current kernel build does not yet support declaring module parameters directly in Rust, we define the `debug_mask` variable in a C companion file to expose it to the kernel runtime and import it using FFI with volatile reads. To verify the setup, instrument process lifecycle events (open, flush, and release) in `process.rs` under the new `BINDER_DEBUG_OPEN_CLOSE` logging mask. These entry-point events are chosen for initial validation because they represent the start of the Binder lifecycle and occur at low frequency, allowing simple runtime verification of the dynamic toggle without log noise. Signed-off-by: Jahnavi MN <jahnavimn@google.com>

freezer-related operation This adds dynamic debug logs for: - Requesting freeze notifications on invalid references, duplicate cookies, or already active registrations. - Completing freeze notifications that are not pending or not found. - Clearing freeze notifications on invalid references, inactive notifications, or cookie mismatches. Signed-off-by: Jahnavi MN <jahnavimn@google.com>

and death notifications This adds dynamic debug logs for: - Decrementing handle reference counts that are already zero. - Mismatched reference states (calling inc_ref_done with no active inc_refs, or using a weak reference as a strong reference). - Requesting or clearing death notifications on invalid references, already active notifications, or with mismatched cookies. Signed-off-by: Jahnavi MN <jahnavimn@google.com>

transaction parsing failures This adds dynamic debug logs in `thread.rs` for: - File descriptor array (FDA) parent offset and parent buffer address alignment misalignments. - Memory copy, write, and translation failures during transaction serialization (including out-of-bounds pointer fixups). - Incoming transactions or replies that do not match the expected thread calling stack (such as out-of-order replies). Signed-off-by: Jahnavi MN <jahnavimn@google.com>

This adds dynamic debug logs for: - Failed replies, target process deaths, and error code deliveries. - Detailed transaction failure diagnostics (including sender/receiver PIDs, TIDs, transaction IDs, buffer sizes, and error codes). Signed-off-by: Jahnavi MN <jahnavimn@google.com>

This adds dynamic debug logs for: - Memory allocation (OOM) failures when requesting death notifications - Registration and cancellation lifecycle events (BC_REQUEST / BC_CLEAR) - Delivery of death notification events to userspace (BR_DEAD_BINDER) Signed-off-by: Jahnavi MN <jahnavimn@google.com>

This adds dynamic debug logs for: - Releasing active transactions during thread stack unwinding. - Discarded transaction error codes when a thread exits. - Undelivered transaction acknowledgments (TRANSACTION_COMPLETE) upon thread exit. - Undelivered process death and freeze notifications when processes exit or die. - Undelivered transactions canceled due to target process death. We now store the process PID in `ThreadError`, `DeliverCode`, and `FreezeMessage` to ensure the correct PID is logged on cancellation. This is necessary because `cancel()` runs from background `kworkers`, which would otherwise print the wrong PID. Signed-off-by: Jahnavi MN <jahnavimn@google.com>

Darksonn · 2026-07-13T07:20:36Z

+    /// Represents a single debug mask category.
+    #[derive(Debug, Clone, Copy, PartialEq, Eq)]
+    pub enum DebugMask {
+        UserError = kernel::bits::bit_u32(0),


Let's import kernel::bits::bit_u32 and kernel::sync::atomic::Atomic instead of using the full path every time.

Wongi and Jungwoo decoded and reported a non-leader exec() related race which can result in an UAF: sys_timer_delete() exec() posix_cpu_timer_del() // Observes old leader p = pid_task(pid, pid_type); de_thread() switch_leader(); release_task(old_leader) __exit_signal(old_leader) sighand = lock(old_leader, sighand); posix_cpu_timers*_exit(); sighand = lock_task_sighand(p) unhash_task(old_leader); sh = lock(p, sighand) old_leader->sighand = NULL; unlock(sighand); (p->sighand == NULL) unlock(sh) return NULL; // Returns without action if(!sighand) return 0; free_posix_timer(); This is "harmless" unless the deleted timer was armed and enqueued in p->signal because on exec() a TGID targeted timer is inherited. As sys_timer_delete() freed the underlying posix timer object run_posix_cpu_timers() or any timerqueue related add/delete operations on other timers will access the freed object's timerqueue node, which results in an UAF. There is a similar problem vs. posix_cpu_timer_set(). For regular posix timers it just transiently returns -ESRCH to user space, but for the use case in do_cpu_nanosleep() it's the same UAF just that the k_itimer is allocated on the stack. Also posix_cpu_timer_rearm() fails to rearm the timer, which means it stops to expire. While debating solutions Frederic pointed out another problem: posix_cpu_timer_del(tmr) __exit_signal(p) posix_cpu_timers*_exit(p); unhash_task(p); p->sighand = NULL; sh = lock_task_sighand(p) sighand = p->sighand; if (!sighand) return NULL; lock(sighand); if (!sh) WARN_ON_ONCE(timer_queued(tmr)); On weakly ordered architectures it is not guaranteed that posix_cpu_timer_del() will observe the stores in posix_cpu_timers*_exit() when p->sighand is observed as NULL, which means the WARN() can be a false positive. Solve these issues by: 1) Changing the store in __exit_signal() to smp_store_release(). 2) Adding a smp_acquire__after_ctrl_dep() into the !sighand path of lock_task_sighand(). 3) Creating a helper function for looking up the task and locking sighand which does not return when sighand == NULL. Instead it retries the task lookup and only if that fails it gives up. 4) Using that helper in the three affected functions. #1/#2 ensures that the reader side which observes sighand == NULL also observes all preceeding stores, i.e. the stores in posix_cpu_timers*_exit() and the ones in unhash_task(). #3 ensures that the above described non-leader exec() situation is handled gracefully. When the task lookup returns the old leader, but sighand == NULL then it retries. In the non-leader exec() case the subsequent task lookup will observe the new leader due to #1/#2. In normal exit() scenarios the subsequent lookup fails. When the task lookup fails, the function also checks whether the timer is still enqueued and issues a warning if that's the case. Unfortunately there is nothing which can be done about it, but as the task is already not longer visible the timer should not be accessed anymore. This check also requires memory ordering, which is not provided when the first lookup fails. To achieve that the check is preceeded by a smp_rmb() which pairs with the smp_wmb() in write_seqlock() in __exit_signal(). That ensures that the stores in posix_cpu_timers*_exit() are visible. The history of the non-leader exec() issue goes back to the early days of posix CPU timers, which stored a pointer to the group leader task in the timer. That obviously fails when a non-leader exec() switches the leader. commit e0a7021 ("posix-cpu-timers: workaround to suppress the problems with mt exec") added a temporary workaround for that in 2010 which survived about 10 years. The fix for the workaround changed the task pointer to a pid pointer, but failed to see the subtle race described above. So the Fixes tag picks that commit, which seems to be halfways accurate. Thanks to Frederic Weissbecker, Oleg Nesterov and Peter Zijlstra for review, feedback and suggestions and to Wongi and Jungwoo for the excellent bug report and analysis! Fixes: 55e8c8e ("posix-cpu-timers: Store a reference to a pid not a task") Reported-by: Wongi Lee <qw3rtyp0@gmail.com> Reported-by: Jungwoo Lee <jwlee2217@gmail.com> Signed-off-by: Thomas Gleixner <tglx@kernel.org> Reviewed-by: Oleg Nesterov <oleg@redhat.com> Cc: stable@vger.kernel.org