When sending a transaction, its offsets array is first copied into the
target proc's vma, and then the values are read back from there. This is
normally fine because the vma is a read-only mapping, so the target
process cannot change the value under us.
However, if the target process somehow gains the ability to write to its
own vma, it could change the offset before it's read back, causing the
kernel to misinterpret what the sender meant. If the sender happens to
send a payload with a specific shape, this could in the worst case lead
to the receiver being able to privilege escalate into the sender.
The intent is that gaining the ability to change the read-only vma of
your own process should not be exploitable, so remove this TOCTOU read
even though it's unexploitable without another Binder bug.
Cc: stable@vger.kernel.org
Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver")
Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
drivers/android/binder/thread.rs | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs
index 1f1709a6a77abc1c865cc9387e7ba7493448c71d..f58ecccf5bb10a4b916d14a38dbb3bdfdda24ff8 100644
--- a/drivers/android/binder/thread.rs
+++ b/drivers/android/binder/thread.rs
@@ -1016,12 +1016,9 @@ pub(crate) fn copy_transaction_data(
// Copy offsets if there are any.
if offsets_size > 0 {
- {
- let mut reader =
- UserSlice::new(UserPtr::from_addr(trd_data_ptr.offsets as _), offsets_size)
- .reader();
- alloc.copy_into(&mut reader, aligned_data_size, offsets_size)?;
- }
+ let mut offsets_reader =
+ UserSlice::new(UserPtr::from_addr(trd_data_ptr.offsets as _), offsets_size)
+ .reader();
let offsets_start = aligned_data_size;
let offsets_end = aligned_data_size + offsets_size;
@@ -1042,11 +1039,9 @@ pub(crate) fn copy_transaction_data(
.step_by(size_of::<u64>())
.enumerate()
{
- let offset: usize = view
- .alloc
- .read::<u64>(index_offset)?
- .try_into()
- .map_err(|_| EINVAL)?;
+ let offset = offsets_reader.read::<u64>()?;
+ view.alloc.write(index_offset, &offset)?;
+ 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.");
--
2.53.0.273.g2a3d683680-googOn Tue, Feb 17, 2026 at 3:22 PM Alice Ryhl <aliceryhl@google.com> wrote:
> When sending a transaction, its offsets array is first copied into the
> target proc's vma, and then the values are read back from there. This is
> normally fine because the vma is a read-only mapping, so the target
> process cannot change the value under us.
>
> However, if the target process somehow gains the ability to write to its
> own vma, it could change the offset before it's read back, causing the
> kernel to misinterpret what the sender meant. If the sender happens to
> send a payload with a specific shape, this could in the worst case lead
> to the receiver being able to privilege escalate into the sender.
>
> The intent is that gaining the ability to change the read-only vma of
> your own process should not be exploitable, so remove this TOCTOU read
> even though it's unexploitable without another Binder bug.
With this, the only remaining read from the ShrinkablePageRange is in
AllocationView::cleanup_object(), correct? If I understand correctly,
that is fine because it can only drop references on handles (which
userspace could equivalently do via BC_RELEASE/BC_DECREFS) and on
binders (which would probably also have its influence limited to the
process)?
> Cc: stable@vger.kernel.org
> Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver")
> Reported-by: Jann Horn <jannh@google.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Jann Horn <jannh@google.com>
On Tue, Feb 17, 2026 at 5:35 PM Jann Horn <jannh@google.com> wrote: > > On Tue, Feb 17, 2026 at 3:22 PM Alice Ryhl <aliceryhl@google.com> wrote: > > When sending a transaction, its offsets array is first copied into the > > target proc's vma, and then the values are read back from there. This is > > normally fine because the vma is a read-only mapping, so the target > > process cannot change the value under us. > > > > However, if the target process somehow gains the ability to write to its > > own vma, it could change the offset before it's read back, causing the > > kernel to misinterpret what the sender meant. If the sender happens to > > send a payload with a specific shape, this could in the worst case lead > > to the receiver being able to privilege escalate into the sender. > > > > The intent is that gaining the ability to change the read-only vma of > > your own process should not be exploitable, so remove this TOCTOU read > > even though it's unexploitable without another Binder bug. > > With this, the only remaining read from the ShrinkablePageRange is in > AllocationView::cleanup_object(), correct? If I understand correctly, > that is fine because it can only drop references on handles (which > userspace could equivalently do via BC_RELEASE/BC_DECREFS) and on > binders (which would probably also have its influence limited to the > process)? Yeah, that's the idea. Alice
Hi Alice,
kernel test robot noticed the following build errors:
[auto build test ERROR on 0f2acd3148e0ef42bdacbd477f90e8533f96b2ac]
url: https://github.com/intel-lab-lkp/linux/commits/Alice-Ryhl/rust_binder-check-ownership-before-using-vma/20260217-222439
base: 0f2acd3148e0ef42bdacbd477f90e8533f96b2ac
patch link: https://lore.kernel.org/r/20260217-binder-vma-check-v1-2-1a2b37f7b762%40google.com
patch subject: [PATCH 2/2] rust_binder: avoid reading the written value in offsets array
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20260217/202602172222.mGDpJK77-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/20260217/202602172222.mGDpJK77-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/202602172222.mGDpJK77-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 -j32 -C source O=/kbuild/obj/consumer/x86_64-rhel-9.4-rust ARCH=x86_64 SHELL=/bin/bash rustfmtcheck
make: Entering directory '/kbuild/src/consumer'
make[1]: Entering directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
>> Diff in drivers/android/binder/thread.rs:1018:
if offsets_size > 0 {
let mut offsets_reader =
UserSlice::new(UserPtr::from_addr(trd_data_ptr.offsets as _), offsets_size)
- .reader();
+ .reader();
let offsets_start = aligned_data_size;
let offsets_end = aligned_data_size + offsets_size;
>> Diff in drivers/android/binder/thread.rs:1018:
if offsets_size > 0 {
let mut offsets_reader =
UserSlice::new(UserPtr::from_addr(trd_data_ptr.offsets as _), offsets_size)
- .reader();
+ .reader();
let offsets_start = aligned_data_size;
let offsets_end = aligned_data_size + offsets_size;
make[2]: *** [Makefile:1903: rustfmt] Error 123
make[2]: Target 'rustfmtcheck' not remade because of errors.
make[1]: Leaving directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
make[1]: *** [Makefile:248: __sub-make] Error 2
make[1]: Target 'rustfmtcheck' not remade because of errors.
make: *** [Makefile:248: __sub-make] Error 2
make: Target 'rustfmtcheck' not remade because of errors.
make: Leaving directory '/kbuild/src/consumer'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2026 Red Hat, Inc.