Skip to content

Commit 4cb9e13

Browse files
Darksonngregkh
authored andcommitted
rust_binder: avoid reading the written value in offsets array
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 <stable@kernel.org> Fixes: eafedbc ("rust_binder: add Rust Binder driver") Reported-by: Jann Horn <jannh@google.com> Reviewed-by: Jann Horn <jannh@google.com> Signed-off-by: Alice Ryhl <aliceryhl@google.com> Acked-by: Liam R. Howlett <Liam.Howlett@oracle.com> Link: https://patch.msgid.link/20260218-binder-vma-check-v2-2-60f9d695a990@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 8ef2c15 commit 4cb9e13

1 file changed

Lines changed: 6 additions & 11 deletions

File tree

drivers/android/binder/thread.rs

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,12 +1015,9 @@ impl Thread {
10151015

10161016
// Copy offsets if there are any.
10171017
if offsets_size > 0 {
1018-
{
1019-
let mut reader =
1020-
UserSlice::new(UserPtr::from_addr(trd_data_ptr.offsets as _), offsets_size)
1021-
.reader();
1022-
alloc.copy_into(&mut reader, aligned_data_size, offsets_size)?;
1023-
}
1018+
let mut offsets_reader =
1019+
UserSlice::new(UserPtr::from_addr(trd_data_ptr.offsets as _), offsets_size)
1020+
.reader();
10241021

10251022
let offsets_start = aligned_data_size;
10261023
let offsets_end = aligned_data_size + offsets_size;
@@ -1041,11 +1038,9 @@ impl Thread {
10411038
.step_by(size_of::<u64>())
10421039
.enumerate()
10431040
{
1044-
let offset: usize = view
1045-
.alloc
1046-
.read::<u64>(index_offset)?
1047-
.try_into()
1048-
.map_err(|_| EINVAL)?;
1041+
let offset = offsets_reader.read::<u64>()?;
1042+
view.alloc.write(index_offset, &offset)?;
1043+
let offset: usize = offset.try_into().map_err(|_| EINVAL)?;
10491044

10501045
if offset < end_of_previous_object || !is_aligned(offset, size_of::<u32>()) {
10511046
pr_warn!("Got transaction with invalid offset.");

0 commit comments

Comments
 (0)