public inbox for linux-mm@kvack.org
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: aliceryhl@google.com, gregkh@linuxfoundation.org
Cc: Liam.Howlett@oracle.com, akpm@linux-foundation.org,
	broonie@kernel.org,  dakr@kernel.org, jannh@google.com,
	linux-mm@kvack.org, lkp@intel.com,  llvm@lists.linux.dev,
	miguel.ojeda.sandonis@gmail.com, nathan@kernel.org,
	 oe-kbuild-all@lists.linux.dev, ojeda@kernel.org,
	rppt@kernel.org,  rust-for-linux@vger.kernel.org
Subject: [PATCH] rust_binder: use AssertSync for BINDER_VM_OPS
Date: Sat, 14 Mar 2026 11:19:51 +0000	[thread overview]
Message-ID: <20260314111951.4139029-1-aliceryhl@google.com> (raw)
In-Reply-To: <CAH5fLgixE9MGXxNEnU5b0EXdgLQoTs5CMv_0OdnfCX3v=coUpw@mail.gmail.com>

When declaring an immutable global variable in Rust, the compiler checks
that it looks thread safe, because it is generally safe to access said
global variable. When using C bindings types for these globals, we don't
really want this check, because it is conservative and assumes pointers
are not thread safe.

In the case of BINDER_VM_OPS, this is a challenge when combined with the
patch 'userfaultfd: introduce vm_uffd_ops' [1], which introduces a
pointer field to vm_operations_struct. It previously only held function
pointers, which are considered thread safe.

Rust Binder should not be assuming that vm_operations_struct contains no
pointer fields, so to fix this, use AssertSync (which Rust Binder has
already declared for another similar global of type struct
file_operations with the same problem). This ensures that even if
another commit adds a pointer field to vm_operations_struct, this does
not cause problems.

Fixes: 8ef2c15aeae0 ("rust_binder: check ownership before using vma")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202603121235.tpnRxFKO-lkp@intel.com/
Link: https://lore.kernel.org/r/20260306171815.3160826-8-rppt@kernel.org [1]
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/android/binder/page_range.rs       | 8 +++++---
 drivers/android/binder/rust_binder_main.rs | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/android/binder/page_range.rs b/drivers/android/binder/page_range.rs
index 9dfc154e5dd4..b57e0c7ba3f1 100644
--- a/drivers/android/binder/page_range.rs
+++ b/drivers/android/binder/page_range.rs
@@ -13,6 +13,8 @@
 //
 // The shrinker will use trylock methods because it locks them in a different order.
 
+use crate::AssertSync;
+
 use core::{
     marker::PhantomPinned,
     mem::{size_of, size_of_val, MaybeUninit},
@@ -143,14 +145,14 @@ pub(crate) struct ShrinkablePageRange {
 }
 
 // We do not define any ops. For now, used only to check identity of vmas.
-static BINDER_VM_OPS: bindings::vm_operations_struct = pin_init::zeroed();
+static BINDER_VM_OPS: AssertSync<bindings::vm_operations_struct> = AssertSync(pin_init::zeroed());
 
 // 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.
 fn check_vma(vma: &virt::VmaRef, owner: *const ShrinkablePageRange) -> Option<&virt::VmaMixedMap> {
     // SAFETY: Just reading the vm_ops pointer of any active vma is safe.
     let vm_ops = unsafe { (*vma.as_ptr()).vm_ops };
-    if !ptr::eq(vm_ops, &BINDER_VM_OPS) {
+    if !ptr::eq(vm_ops, &BINDER_VM_OPS.0) {
         return None;
     }
 
@@ -342,7 +344,7 @@ pub(crate) fn register_with_vma(&self, vma: &virt::VmaNew) -> Result<usize> {
 
         // SAFETY: We own the vma, and we don't use any methods on VmaNew that rely on
         // `vm_ops`.
-        unsafe { (*vma.as_ptr()).vm_ops = &BINDER_VM_OPS };
+        unsafe { (*vma.as_ptr()).vm_ops = &BINDER_VM_OPS.0 };
 
         Ok(num_pages)
     }
diff --git a/drivers/android/binder/rust_binder_main.rs b/drivers/android/binder/rust_binder_main.rs
index aa5f2a75adb4..014010662df8 100644
--- a/drivers/android/binder/rust_binder_main.rs
+++ b/drivers/android/binder/rust_binder_main.rs
@@ -306,7 +306,7 @@ fn init(_module: &'static kernel::ThisModule) -> Result<Self> {
 /// Makes the inner type Sync.
 #[repr(transparent)]
 pub struct AssertSync<T>(T);
-// SAFETY: Used only to insert `file_operations` into a global, which is safe.
+// SAFETY: Used only to insert C bindings types into globals, which is safe.
 unsafe impl<T> Sync for AssertSync<T> {}
 
 /// File operations that rust_binderfs.c can use.

base-commit: a0b9b0f1433c845bda708753db32befef78e0f1f
-- 
2.53.0.851.ga537e3e6e9-goog



  reply	other threads:[~2026-03-14 11:20 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-12  4:21 [linux-next:master 5585/5731] error[E0277]: `*const kernel::bindings::vm_uffd_ops` cannot be shared between threads safely kernel test robot
2026-03-13 21:36 ` Nathan Chancellor
2026-03-13 22:39   ` Miguel Ojeda
2026-03-14 11:12     ` Greg Kroah-Hartman
2026-03-14 11:15       ` Alice Ryhl
2026-03-14 11:19         ` Alice Ryhl [this message]
2026-03-14 13:37           ` [PATCH] rust_binder: use AssertSync for BINDER_VM_OPS Gary Guo
2026-03-14 11:30         ` [linux-next:master 5585/5731] error[E0277]: `*const kernel::bindings::vm_uffd_ops` cannot be shared between threads safely Alice Ryhl
2026-03-14 11:53           ` Greg Kroah-Hartman
2026-03-14 11:58             ` Alice Ryhl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260314111951.4139029-1-aliceryhl@google.com \
    --to=aliceryhl@google.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=broonie@kernel.org \
    --cc=dakr@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jannh@google.com \
    --cc=linux-mm@kvack.org \
    --cc=lkp@intel.com \
    --cc=llvm@lists.linux.dev \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=nathan@kernel.org \
    --cc=oe-kbuild-all@lists.linux.dev \
    --cc=ojeda@kernel.org \
    --cc=rppt@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox