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

[Alice Ryhl]

On Fri, May 29, 2026 at 11:41:53AM +0900, Hyunwoo Kim wrote:
> On Fri, May 29, 2026 at 10:19:27AM +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@xxxxxxxxxxxxxxx
> > Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver")
> > Signed-off-by: Hyunwoo Kim <imv4bel@xxxxxxxxx>
> > ---
> >  drivers/android/binder/allocation.rs | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/android/binder/allocation.rs b/drivers/android/binder/allocation.rs
> > index 0cab959e4b7e..f4ffc57a8cb2 100644
> > --- a/drivers/android/binder/allocation.rs
> > +++ b/drivers/android/binder/allocation.rs
> > @@ -251,7 +251,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)
> >                      }
> > @@ -412,7 +412,7 @@ pub(crate) fn transfer_binder_object(
> >      }
> >  
> >      fn cleanup_object(&self, index_offset: usize) -> Result {
> > -        let offset = self.alloc.read(index_offset)?;
> > +        let offset: usize = self.alloc.read::<u64>(index_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
> > 
> 
> The BC_FREE_BUFFER handling in thread.rs's write() seems to have 
> a similar problem.
> 
> Sashiko's review:
> https://sashiko.dev/#/patchset/ahjpn-3WQTywTdyj@v4bel?part=1

Yeah I think there are a few instances of this. Would be nice to fix
them.

Alice