Re: [PATCH v2] rust_binder: use a u64 stride when cleaning up the offsets array

[Carlos Llamas <cmllamas@google.com>]

On Sun, May 31, 2026 at 10:29:24PM +0900, Hyunwoo Kim wrote:
> Allocation's Drop walks the offsets array (binder_size_t = u64 entries),
> cleaning up the objects, but it used usize instead of u64 for both the
> stride and the per-entry read.
> 
> On 64-bit kernels (usize == u64) this is harmless, but on 32-bit kernels
> it walks the 8-byte entries in 4-byte steps, iterating an N-entry array
> 2N times, and reads the always-zero high word as offset 0, cleaning up
> the object at offset 0 N extra times. As a result the referenced node or
> handle ends up with a lower reference count than it actually has (a
> refcount over-decrement), and binder's reference accounting is corrupted;
> for example, the owner can be notified of a strong reference release
> (BR_RELEASE) even though references still remain.
> 
> Change the stride to u64, and read each entry as a u64, narrowing it to
> usize with try_into().
> 
> On 32-bit ARM, when this over-decrement would drive a count below zero,
> the driver's existing refcount guard refuses it and fires:
> 
>   rust_binder: Failure: refcount underflow!
> 
> Cc: stable@vger.kernel.org
> Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver")
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
> Changes in v2:
> - reformat to satisfy rustfmt, as pointed out by the kernel test robot
> - v1: https://lore.kernel.org/all/ahjpn-3WQTywTdyj@v4bel/
> ---
>  drivers/android/binder/allocation.rs | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/android/binder/allocation.rs b/drivers/android/binder/allocation.rs
> index b7b05e72970a..ea5846e4da16 100644
> --- a/drivers/android/binder/allocation.rs
> +++ b/drivers/android/binder/allocation.rs
> @@ -259,7 +259,7 @@ fn drop(&mut self) {
>  
>              if let Some(offsets) = info.offsets.clone() {
>                  let view = AllocationView::new(self, offsets.start);
> -                for i in offsets.step_by(size_of::<usize>()) {
> +                for i in offsets.step_by(size_of::<u64>()) {
>                      if view.cleanup_object(i).is_err() {
>                          pr_warn!("Error cleaning up object at offset {}\n", i)
>                      }
> @@ -420,7 +420,8 @@ pub(crate) fn transfer_binder_object(
>      }
>  
>      fn cleanup_object(&self, index_offset: usize) -> Result {
> -        let offset = self.alloc.read(index_offset)?;
> +        let offset = self.alloc.read::<u64>(index_offset)?;
> +        let offset: usize = offset.try_into().map_err(|_| EINVAL)?;
>          let header = self.read::<BinderObjectHeader>(offset)?;
>          match header.type_ {
>              BINDER_TYPE_WEAK_BINDER | BINDER_TYPE_BINDER => {
> -- 
> 2.43.0
> 

Hey Alice, have you seen this? This looks correct to me so,

Acked-by: Carlos Llamas <cmllamas@google.com>