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

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

[Hyunwoo Kim]

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>
---
 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

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

[Hyunwoo Kim]

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@vger.kernel.org
> Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver")
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
>  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


Best regards,
Hyunwoo Kim

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@vger.kernel.org
> > Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver")
> > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> > ---
> >  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

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

[kernel test robot]

Hi Hyunwoo,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on staging/staging-next staging/staging-linus linus/master v7.1-rc5 next-20260528]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Hyunwoo-Kim/rust_binder-use-a-u64-stride-when-cleaning-up-the-offsets-array/20260529-092450
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/ahjpn-3WQTywTdyj%40v4bel
patch subject: [PATCH] rust_binder: use a u64 stride when cleaning up the offsets array
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20260529/202605290823.20jvlxFD-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260529/202605290823.20jvlxFD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605290823.20jvlxFD-lkp@intel.com/

All errors (new ones prefixed by >>):

   PATH=/opt/cross/clang-20/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
   INFO PATH=/opt/cross/rustc-1.88.0-bindgen-0.72.1/cargo/bin:/opt/cross/clang-20/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
   /usr/bin/timeout -k 100 12h /usr/bin/make KCFLAGS=\ -fno-crash-diagnostics\ -Wno-error=return-type\ -Wreturn-type\ -funsigned-char\ -Wundef\ -falign-functions=64 W=1 --keep-going LLVM=1 -j384 -C source O=/kbuild/obj/consumer/x86_64-rhel-9.4-rust ARCH=x86_64 SHELL=/bin/bash rustfmtcheck 
   make: Entering directory '/kbuild/src'
   make[1]: Entering directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
>> Diff in drivers/android/binder/allocation.rs:412:
        }
    
        fn cleanup_object(&self, index_offset: usize) -> Result {
   -        let offset: usize = self.alloc.read::<u64>(index_offset)?.try_into().map_err(|_| EINVAL)?;
   +        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 => {
>> Diff in drivers/android/binder/allocation.rs:412:
        }
    
        fn cleanup_object(&self, index_offset: usize) -> Result {
   -        let offset: usize = self.alloc.read::<u64>(index_offset)?.try_into().map_err(|_| EINVAL)?;
   +        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 => {
   make[1]: Leaving directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
   make: *** [Makefile:248: __sub-make] Error 2
   make: Target 'rustfmtcheck' not remade because of errors.
   make[2]: *** [Makefile:1954: rustfmt] Error 123
   make[2]: Target 'rustfmtcheck' not remade because of errors.
   make: Leaving directory '/kbuild/src'
   make[1]: *** [Makefile:248: __sub-make] Error 2
   make[1]: Target 'rustfmtcheck' not remade because of errors.

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki