Skip to contentSkip to conversation

binder: reject mremap()

PATCH In review Carlos Llamas 1 participant 3 messages Aug 31 – Sep 1

Subsystems (3)
lore
Share & tools
lore

Carlos Llamas

author maintainer
#0 lore raw

PATCH 1/2 binder: reject mremap()

Binder does not support mremap() as it caches the mapping address in alloc->vm_start. Moving the mapping breaks the IPC communication for the process and can temporarily leak pages during a shrinker reclaim.

Fix this by explicitly rejecting the .mremap() operation.

---
 drivers/android/binder.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/android/binder.c +6 -0
index 8f2ef1bd539f..6c12e502740f 100644
@@ -6032,10 +6032,16 @@ VISIBLE_IF_KUNIT vm_fault_t binder_vm_fault(struct vm_fault *vmf)
 }
 EXPORT_SYMBOL_IF_KUNIT(binder_vm_fault);
 
+static int binder_mremap(struct vm_area_struct *vma)
+{
+	return -EINVAL;
+}
+
 static const struct vm_operations_struct binder_vm_ops = {
 	.open = binder_vma_open,
 	.close = binder_vma_close,
 	.fault = binder_vm_fault,
+	.mremap = binder_mremap,
 };
 
 static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
Signature · 1 line
2.55.0.897.gb25b4bd76c-goog
author maintainer
#1 lore raw

PATCH 2/2 rust_binder: reject mremap()

Binder does not support mremap() as it caches the mapping address in Inner::vma_addr. Moving the mapping breaks the IPC communication for the process and can temporarily leak pages during a shrinker reclaim.

Fix this by explicitly rejecting the .mremap() operation.

---
 drivers/android/binder/page_range.rs | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
drivers/android/binder/page_range.rs +12 -3
index 52ffbf3504e7..411efcc2116f 100644
@@ -24,7 +24,7 @@
 use kernel::{
     bindings,
     error::Result,
-    ffi::{c_ulong, c_void},
+    ffi::{c_int, c_ulong, c_void},
     mm::{virt, Mm, MmWithUser},
     new_mutex, new_spinlock,
     page::{Page, PAGE_SHIFT, PAGE_SIZE},
@@ -144,8 +144,17 @@ pub(crate) struct ShrinkablePageRange {
     _pin: PhantomPinned,
 }
 
-// We do not define any ops. For now, used only to check identity of vmas.
-static BINDER_VM_OPS: AssertSync<bindings::vm_operations_struct> = AssertSync(pin_init::zeroed());
+unsafe extern "C" fn binder_mremap(_: *mut bindings::vm_area_struct) -> c_int {
+    EINVAL.to_errno()
+}
+
+static BINDER_VM_OPS: AssertSync<bindings::vm_operations_struct> = {
+    let ops = bindings::vm_operations_struct {
+        mremap: Some(binder_mremap),
+        ..pin_init::zeroed()
+    };
+    AssertSync(ops)
+};
 
 // To ensure that we do not accidentally install pages into or zap pages from the wrong vma, we
 // check its vm_ops and private data before using it.
Signature · 1 line
2.55.0.897.gb25b4bd76c-goog
author maintainer
#2 lore raw

Re: [PATCH 2/2] rust_binder: reject mremap()

Quoted 4 lines · Carlos Llamas

Binder does not support mremap() as it caches the mapping address in Inner::vma_addr. Moving the mapping breaks the IPC communication for the process and can temporarily leak pages during a shrinker reclaim.

Quoted 8 lines

Fix this by explicitly rejecting the .mremap() operation.

---

Sashiko had a couple of good points that I'll be addressing in v2. --
Carlos Llamas