The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Jahnavi MN via B4 Relay <devnull+jahnavimn.google.com@kernel.org>
To: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Arve Hjønnevåg" <arve@android.com>,
	"Todd Kjos" <tkjos@android.com>,
	"Christian Brauner" <brauner@kernel.org>,
	"Carlos Llamas" <cmllamas@google.com>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Onur Özkan" <work@onurozkan.dev>
Cc: linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	 Jahnavi MN <jahnavimn@google.com>
Subject: [PATCH 4/7] rust_binder: Implement the BINDER_DEBUG_USER_ERROR logging mask for transaction parsing and protocol validation failures
Date: Fri, 03 Jul 2026 15:29:25 +0000	[thread overview]
Message-ID: <20260703-rust_binder_debug_mask-v1-4-9bdf12b5325c@google.com> (raw)
In-Reply-To: <20260703-rust_binder_debug_mask-v1-0-9bdf12b5325c@google.com>

From: Jahnavi MN <jahnavimn@google.com>

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>
---
 drivers/android/binder/thread.rs | 88 +++++++++++++++++++++++++++++++++-------
 1 file changed, 73 insertions(+), 15 deletions(-)

diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs
index c908dde5796a..ecf05cebcb3e 100644
--- a/drivers/android/binder/thread.rs
+++ b/drivers/android/binder/thread.rs
@@ -721,11 +721,12 @@ fn translate_object(
                 let alloc_offset = match sg_state.unused_buffer_space.claim_next(obj_length) {
                     Ok(alloc_offset) => alloc_offset,
                     Err(err) => {
-                        pr_warn!(
-                            "Failed to claim space for a BINDER_TYPE_PTR. (offset: {}, limit: {}, size: {})",
+                        binder_debug!(
+                            crate::debug::BINDER_DEBUG_USER_ERROR,
+                            "failed to claim space for a BINDER_TYPE_PTR (offset: {}, limit: {}, size: {})",
                             sg_state.unused_buffer_space.offset,
                             sg_state.unused_buffer_space.limit,
-                            obj_length,
+                            obj_length
                         );
                         return Err(err.into());
                     }
@@ -804,6 +805,10 @@ fn translate_object(
                 let fds_len = num_fds.checked_mul(size_of::<u32>()).ok_or(EINVAL)?;
 
                 if !is_aligned(parent_offset, size_of::<u32>()) {
+                    binder_debug!(
+                        crate::debug::BINDER_DEBUG_USER_ERROR,
+                        "FDA parent offset not aligned correctly"
+                    );
                     return Err(EINVAL.into());
                 }
 
@@ -822,6 +827,10 @@ fn translate_object(
                 };
 
                 if !is_aligned(parent_entry.sender_uaddr, size_of::<u32>()) {
+                    binder_debug!(
+                        crate::debug::BINDER_DEBUG_USER_ERROR,
+                        "FDA parent buffer not aligned correctly"
+                    );
                     return Err(EINVAL.into());
                 }
 
@@ -905,8 +914,9 @@ fn apply_sg(&self, alloc: &mut Allocation, sg_state: &mut ScatterGatherState) ->
 
                 let target_offset_end = fixup_offset.checked_add(fixup_len).ok_or(EINVAL)?;
                 if fixup_offset < end_of_previous_fixup || offset_end < target_offset_end {
-                    pr_warn!(
-                        "Fixups oob {} {} {} {}",
+                    binder_debug!(
+                        crate::debug::BINDER_DEBUG_USER_ERROR,
+                        "fixups oob {} {} {} {}",
                         fixup_offset,
                         end_of_previous_fixup,
                         offset_end,
@@ -918,18 +928,31 @@ fn apply_sg(&self, alloc: &mut Allocation, sg_state: &mut ScatterGatherState) ->
                 let copy_off = end_of_previous_fixup;
                 let copy_len = fixup_offset - end_of_previous_fixup;
                 if let Err(err) = alloc.copy_into(&mut reader, copy_off, copy_len) {
-                    pr_warn!("Failed copying into alloc: {:?}", err);
+                    binder_debug!(
+                        crate::debug::BINDER_DEBUG_USER_ERROR,
+                        "failed copying into alloc: {:?}",
+                        err
+                    );
                     return Err(err.into());
                 }
                 if let PointerFixupEntry::Fixup { pointer_value, .. } = fixup {
                     let res = alloc.write::<u64>(fixup_offset, pointer_value);
                     if let Err(err) = res {
-                        pr_warn!("Failed copying ptr into alloc: {:?}", err);
+                        binder_debug!(
+                            crate::debug::BINDER_DEBUG_USER_ERROR,
+                            "failed copying ptr into alloc: {:?}",
+                            err
+                        );
                         return Err(err.into());
                     }
                 }
                 if let Err(err) = reader.skip(fixup_len) {
-                    pr_warn!("Failed skipping {} from reader: {:?}", fixup_len, err);
+                    binder_debug!(
+                        crate::debug::BINDER_DEBUG_USER_ERROR,
+                        "failed skipping {} from reader: {:?}",
+                        fixup_len,
+                        err
+                    );
                     return Err(err.into());
                 }
                 end_of_previous_fixup = target_offset_end;
@@ -937,7 +960,11 @@ fn apply_sg(&self, alloc: &mut Allocation, sg_state: &mut ScatterGatherState) ->
             let copy_off = end_of_previous_fixup;
             let copy_len = offset_end - end_of_previous_fixup;
             if let Err(err) = alloc.copy_into(&mut reader, copy_off, copy_len) {
-                pr_warn!("Failed copying remainder into alloc: {:?}", err);
+                binder_debug!(
+                    crate::debug::BINDER_DEBUG_USER_ERROR,
+                    "failed copying remainder into alloc: {:?}",
+                    err
+                );
                 return Err(err.into());
             }
         }
@@ -1041,7 +1068,10 @@ pub(crate) fn copy_transaction_data(
                 let offset: usize = offset.try_into().map_err(|_| EINVAL)?;
 
                 if offset < end_of_previous_object || !is_aligned(offset, size_of::<u32>()) {
-                    pr_warn!("Got transaction with invalid offset.");
+                    binder_debug!(
+                        crate::debug::BINDER_DEBUG_USER_ERROR,
+                        "got transaction with invalid offset"
+                    );
                     return Err(EINVAL.into());
                 }
 
@@ -1066,7 +1096,11 @@ pub(crate) fn copy_transaction_data(
                 ) {
                     Ok(()) => end_of_previous_object = offset + object.size(),
                     Err(err) => {
-                        pr_warn!("Error while translating object.");
+                        binder_debug!(
+                            crate::debug::BINDER_DEBUG_USER_ERROR,
+                            "error while translating object: {:?}",
+                            err
+                        );
                         return Err(err);
                     }
                 }
@@ -1087,14 +1121,22 @@ pub(crate) fn copy_transaction_data(
 
         if let Some(sg_state) = sg_state.as_mut() {
             if let Err(err) = self.apply_sg(&mut alloc, sg_state) {
-                pr_warn!("Failure in apply_sg: {:?}", err);
+                binder_debug!(
+                    crate::debug::BINDER_DEBUG_USER_ERROR,
+                    "failure in apply_sg: {:?}",
+                    err
+                );
                 return Err(err);
             }
         }
 
         if let Some((off_out, secctx)) = secctx.as_mut() {
             if let Err(err) = alloc.write(secctx_off, secctx.as_bytes()) {
-                pr_warn!("Failed to write security context: {:?}", err);
+                binder_debug!(
+                    crate::debug::BINDER_DEBUG_USER_ERROR,
+                    "failed to write security context: {:?}",
+                    err
+                );
                 return Err(err.into());
             }
             **off_out = secctx_off;
@@ -1283,7 +1325,10 @@ fn transaction_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> BinderResu
         {
             let mut inner = self.inner.lock();
             if !transaction.is_stacked_on(&inner.current_transaction) {
-                pr_warn!("Transaction stack changed during transaction!");
+                binder_debug!(
+                    crate::debug::BINDER_DEBUG_USER_ERROR,
+                    "got new transaction with bad transaction stack"
+                );
                 return Err(EINVAL.into());
             }
             inner.current_transaction = Some(transaction.clone_arc());
@@ -1306,8 +1351,21 @@ fn transaction_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> BinderResu
     }
 
     fn reply_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> BinderResult {
-        let orig = self.inner.lock().pop_transaction_to_reply(self)?;
+        let orig = match self.inner.lock().pop_transaction_to_reply(self) {
+            Ok(orig) => orig,
+            Err(err) => {
+                binder_debug!(
+                    crate::debug::BINDER_DEBUG_USER_ERROR,
+                    "got reply transaction with no transaction stack"
+                );
+                return Err(err.into());
+            }
+        };
         if !orig.from.is_current_transaction(&orig) {
+            binder_debug!(
+                crate::debug::BINDER_DEBUG_USER_ERROR,
+                "got reply transaction with bad transaction stack"
+            );
             return Err(EINVAL.into());
         }
 

-- 
2.55.0.rc0.799.gd6f94ed593-goog



  parent reply	other threads:[~2026-07-03 15:29 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-03 15:29 [PATCH 0/7] rust_binder : Implement dynamic debug logging mask Jahnavi MN via B4 Relay
2026-07-03 15:29 ` [PATCH 1/7] rust_binder: Add " Jahnavi MN via B4 Relay
2026-07-03 16:23   ` Gary Guo
2026-07-04 21:03   ` Alice Ryhl
2026-07-03 15:29 ` [PATCH 2/7] rust_binder: Implement the BINDER_DEBUG_USER_ERROR logging mask for freezer-related operation Jahnavi MN via B4 Relay
2026-07-03 15:29 ` [PATCH 3/7] rust_binder: Implement the BINDER_DEBUG_USER_ERROR logging mask for reference counting and death notification operations Jahnavi MN via B4 Relay
2026-07-04 21:14   ` Alice Ryhl
2026-07-03 15:29 ` Jahnavi MN via B4 Relay [this message]
2026-07-03 15:29 ` [PATCH 5/7] rust_binder: Implement the BINDER_DEBUG_FAILED_TRANSACTION logging mask for transaction parsing and routing failures Jahnavi MN via B4 Relay
2026-07-03 15:29 ` [PATCH 6/7] rust_binder: Implement BINDER_DEBUG_FAILED_TRANSACTION logging for death notification allocation failures Jahnavi MN via B4 Relay
2026-07-04 21:17   ` Alice Ryhl
2026-07-03 15:29 ` [PATCH 7/7] rust_binder: Implement the BINDER_DEBUG_DEAD_TRANSACTION logging mask to trace in-flight cancellations during teardown Jahnavi MN via B4 Relay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260703-rust_binder_debug_mask-v1-4-9bdf12b5325c@google.com \
    --to=devnull+jahnavimn.google.com@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=arve@android.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=brauner@kernel.org \
    --cc=cmllamas@google.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jahnavimn@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@kernel.org \
    --cc=tkjos@android.com \
    --cc=tmgross@umich.edu \
    --cc=work@onurozkan.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox