linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Arve Hjønnevåg" <arve@android.com>,
	"Todd Kjos" <tkjos@android.com>,
	"Martijn Coenen" <maco@android.com>,
	"Joel Fernandes" <joel@joelfernandes.org>,
	"Christian Brauner" <brauner@kernel.org>,
	"Carlos Llamas" <cmllamas@google.com>,
	"Suren Baghdasaryan" <surenb@google.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@gmail.com>
Cc: linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@samsung.com>,
	"Matt Gilbride" <mattgilbride@google.com>,
	"Jeffrey Vander Stoep" <jeffv@google.com>,
	"Matthew Maurer" <mmaurer@google.com>,
	"Alice Ryhl" <aliceryhl@google.com>
Subject: [PATCH RFC 13/20] rust_binder: add BINDER_TYPE_FD support
Date: Wed, 01 Nov 2023 18:01:43 +0000	[thread overview]
Message-ID: <20231101-rust-binder-v1-13-08ba9197f637@google.com> (raw)
In-Reply-To: <20231101-rust-binder-v1-0-08ba9197f637@google.com>

Add support for sending fds over binder.

Unlike the other object types, file descriptors are not translated until
the transaction is actually received by the recipient. Until that
happens, we store `u32::MAX` as the fd.

Translating fds is done in a two-phase process. First, the file
descriptors are allocated and written to the allocation. Then, once we
have allocated all of them, we commit them to the files in question.
Using this strategy, we are able to guarantee that we either send all of
the fds, or none of them.

Co-developed-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/android/allocation.rs  | 71 ++++++++++++++++++++++++++++++++++++++++++
 drivers/android/defs.rs        |  4 ++-
 drivers/android/error.rs       |  6 ++++
 drivers/android/thread.rs      | 42 ++++++++++++++++++++++---
 drivers/android/transaction.rs | 53 ++++++++++++++++++++++++-------
 rust/helpers.c                 |  8 +++++
 rust/kernel/file.rs            |  2 +-
 rust/kernel/security.rs        | 11 +++++++
 8 files changed, 179 insertions(+), 18 deletions(-)

diff --git a/drivers/android/allocation.rs b/drivers/android/allocation.rs
index 32bc268956f2..9d777ffb7176 100644
--- a/drivers/android/allocation.rs
+++ b/drivers/android/allocation.rs
@@ -4,10 +4,12 @@
 
 use kernel::{
     bindings,
+    file::{File, FileDescriptorReservation},
     io_buffer::{IoBufferReader, ReadableFromBytes, WritableToBytes},
     pages::Pages,
     prelude::*,
     sync::Arc,
+    types::ARef,
     user_ptr::UserSlicePtrReader,
 };
 
@@ -32,6 +34,8 @@ pub(crate) struct AllocationInfo {
     pub(crate) oneway_node: Option<DArc<Node>>,
     /// Zero the data in the buffer on free.
     pub(crate) clear_on_free: bool,
+    /// List of files embedded in this transaction.
+    file_list: FileList,
 }
 
 /// Represents an allocation that the kernel is currently using.
@@ -160,6 +164,38 @@ pub(crate) fn set_info_clear_on_drop(&mut self) {
     pub(crate) fn set_info_target_node(&mut self, target_node: NodeRef) {
         self.get_or_init_info().target_node = Some(target_node);
     }
+
+    pub(crate) fn info_add_fd(&mut self, file: ARef<File>, buffer_offset: usize) -> Result {
+        self.get_or_init_info()
+            .file_list
+            .files_to_translate
+            .try_push(FileEntry {
+                file,
+                buffer_offset,
+            })?;
+
+        Ok(())
+    }
+
+    pub(crate) fn translate_fds(&mut self) -> Result<TranslatedFds> {
+        let file_list = match self.allocation_info.as_mut() {
+            Some(info) => &mut info.file_list,
+            None => return Ok(TranslatedFds::new()),
+        };
+
+        let files = core::mem::take(&mut file_list.files_to_translate);
+        let mut reservations = Vec::try_with_capacity(files.len())?;
+        for file_info in files {
+            let res = FileDescriptorReservation::new(bindings::O_CLOEXEC)?;
+            self.write::<u32>(file_info.buffer_offset, &res.reserved_fd())?;
+            reservations.try_push(Reservation {
+                res,
+                file: file_info.file,
+            })?;
+        }
+
+        Ok(TranslatedFds { reservations })
+    }
 }
 
 impl Drop for Allocation {
@@ -417,3 +453,38 @@ fn type_to_size(type_: u32) -> Option<usize> {
         }
     }
 }
+
+#[derive(Default)]
+struct FileList {
+    files_to_translate: Vec<FileEntry>,
+}
+
+struct FileEntry {
+    /// The file for which a descriptor will be created in the recipient process.
+    file: ARef<File>,
+    /// The offset in the buffer where the file descriptor is stored.
+    buffer_offset: usize,
+}
+
+pub(crate) struct TranslatedFds {
+    reservations: Vec<Reservation>,
+}
+
+struct Reservation {
+    res: FileDescriptorReservation,
+    file: ARef<File>,
+}
+
+impl TranslatedFds {
+    pub(crate) fn new() -> Self {
+        Self {
+            reservations: Vec::new(),
+        }
+    }
+
+    pub(crate) fn commit(self) {
+        for entry in self.reservations {
+            entry.res.commit(entry.file);
+        }
+    }
+}
diff --git a/drivers/android/defs.rs b/drivers/android/defs.rs
index 267266f3ad76..fa4ec3eff424 100644
--- a/drivers/android/defs.rs
+++ b/drivers/android/defs.rs
@@ -54,9 +54,10 @@ macro_rules! pub_no_prefix {
     BC_DEAD_BINDER_DONE
 );
 
+pub(crate) const FLAT_BINDER_FLAG_ACCEPTS_FDS: u32 = kernel::bindings::FLAT_BINDER_FLAG_ACCEPTS_FDS;
 pub(crate) const FLAT_BINDER_FLAG_TXN_SECURITY_CTX: u32 =
     kernel::bindings::FLAT_BINDER_FLAG_TXN_SECURITY_CTX;
-pub_no_prefix!(transaction_flags_, TF_ONE_WAY, TF_CLEAR_BUF);
+pub_no_prefix!(transaction_flags_, TF_ONE_WAY, TF_ACCEPT_FDS, TF_CLEAR_BUF);
 
 pub(crate) use bindings::{
     BINDER_TYPE_BINDER, BINDER_TYPE_FD, BINDER_TYPE_FDA, BINDER_TYPE_HANDLE, BINDER_TYPE_PTR,
@@ -104,6 +105,7 @@ fn default() -> Self {
 decl_wrapper!(BinderNodeDebugInfo, bindings::binder_node_debug_info);
 decl_wrapper!(BinderNodeInfoForRef, bindings::binder_node_info_for_ref);
 decl_wrapper!(FlatBinderObject, bindings::flat_binder_object);
+decl_wrapper!(BinderFdObject, bindings::binder_fd_object);
 decl_wrapper!(BinderObjectHeader, bindings::binder_object_header);
 decl_wrapper!(BinderBufferObject, bindings::binder_buffer_object);
 decl_wrapper!(BinderTransactionData, bindings::binder_transaction_data);
diff --git a/drivers/android/error.rs b/drivers/android/error.rs
index c9b991d133d9..6735636d2a1c 100644
--- a/drivers/android/error.rs
+++ b/drivers/android/error.rs
@@ -41,6 +41,12 @@ fn from(source: Error) -> Self {
     }
 }
 
+impl From<kernel::file::BadFdError> for BinderError {
+    fn from(source: kernel::file::BadFdError) -> Self {
+        BinderError::from(Error::from(source))
+    }
+}
+
 impl From<core::alloc::AllocError> for BinderError {
     fn from(_: core::alloc::AllocError) -> Self {
         Self {
diff --git a/drivers/android/thread.rs b/drivers/android/thread.rs
index 86bb32bbabd9..56b36dc43bcc 100644
--- a/drivers/android/thread.rs
+++ b/drivers/android/thread.rs
@@ -601,6 +601,7 @@ fn translate_object(
         offset: usize,
         object: BinderObjectRef<'_>,
         view: &mut AllocationView<'_>,
+        allow_fds: bool,
         sg_state: &mut ScatterGatherState,
     ) -> BinderResult {
         match object {
@@ -629,9 +630,31 @@ fn translate_object(
                 security::binder_transfer_binder(&self.process.cred, &view.alloc.process.cred)?;
                 view.transfer_binder_object(offset, obj, strong, node)?;
             }
-            BinderObjectRef::Fd(_obj) => {
-                pr_warn!("Using unsupported binder object type fd.");
-                return Err(EINVAL.into());
+            BinderObjectRef::Fd(obj) => {
+                if !allow_fds {
+                    return Err(EPERM.into());
+                }
+
+                // SAFETY: `fd` is a `u32`; any bit pattern is a valid representation.
+                let fd = unsafe { obj.__bindgen_anon_1.fd };
+                let file = File::from_fd(fd)?;
+                security::binder_transfer_file(
+                    &self.process.cred,
+                    &view.alloc.process.cred,
+                    &file,
+                )?;
+
+                let mut obj_write = BinderFdObject::default();
+                obj_write.hdr.type_ = BINDER_TYPE_FD;
+                // This will be overwritten with the actual fd when the transaction is received.
+                obj_write.__bindgen_anon_1.fd = u32::MAX;
+                obj_write.cookie = obj.cookie;
+                view.write::<BinderFdObject>(offset, &obj_write)?;
+
+                const FD_FIELD_OFFSET: usize =
+                    ::core::mem::offset_of!(bindings::binder_fd_object, __bindgen_anon_1.fd)
+                        as usize;
+                view.alloc.info_add_fd(file, offset + FD_FIELD_OFFSET)?;
             }
             BinderObjectRef::Ptr(obj) => {
                 let obj_length = obj.length.try_into().map_err(|_| EINVAL)?;
@@ -773,6 +796,7 @@ pub(crate) fn copy_transaction_data(
         &self,
         to_process: Arc<Process>,
         tr: &BinderTransactionDataSg,
+        allow_fds: bool,
         txn_security_ctx_offset: Option<&mut usize>,
     ) -> BinderResult<Allocation> {
         let trd = &tr.transaction_data;
@@ -869,7 +893,14 @@ pub(crate) fn copy_transaction_data(
 
                 let mut object = BinderObject::read_from(&mut buffer_reader)?;
 
-                match self.translate_object(index, offset, object.as_ref(), &mut view, sg_state) {
+                match self.translate_object(
+                    index,
+                    offset,
+                    object.as_ref(),
+                    &mut view,
+                    allow_fds,
+                    sg_state,
+                ) {
                     Ok(()) => end_of_previous_object = offset + object.size(),
                     Err(err) => {
                         pr_warn!("Error while translating object.");
@@ -1059,7 +1090,8 @@ fn reply_inner(self: &Arc<Self>, tr: &BinderTransactionDataSg) -> BinderResult {
         (|| -> BinderResult<_> {
             let completion = DTRWrap::arc_try_new(DeliverCode::new(BR_TRANSACTION_COMPLETE))?;
             let process = orig.from.process.clone();
-            let reply = Transaction::new_reply(self, process, tr)?;
+            let allow_fds = orig.flags & TF_ACCEPT_FDS != 0;
+            let reply = Transaction::new_reply(self, process, tr, allow_fds)?;
             self.inner.lock().push_work(completion);
             orig.from.deliver_reply(Either::Left(reply), &orig);
             Ok(())
diff --git a/drivers/android/transaction.rs b/drivers/android/transaction.rs
index 2faba6e1f47f..3230ea490a5b 100644
--- a/drivers/android/transaction.rs
+++ b/drivers/android/transaction.rs
@@ -11,7 +11,7 @@
 };
 
 use crate::{
-    allocation::Allocation,
+    allocation::{Allocation, TranslatedFds},
     defs::*,
     error::{BinderError, BinderResult},
     node::{Node, NodeRef},
@@ -50,19 +50,24 @@ pub(crate) fn new(
         tr: &BinderTransactionDataSg,
     ) -> BinderResult<DLArc<Self>> {
         let trd = &tr.transaction_data;
+        let allow_fds = node_ref.node.flags & FLAT_BINDER_FLAG_ACCEPTS_FDS != 0;
         let txn_security_ctx = node_ref.node.flags & FLAT_BINDER_FLAG_TXN_SECURITY_CTX != 0;
         let mut txn_security_ctx_off = if txn_security_ctx { Some(0) } else { None };
         let to = node_ref.node.owner.clone();
-        let mut alloc =
-            match from.copy_transaction_data(to.clone(), tr, txn_security_ctx_off.as_mut()) {
-                Ok(alloc) => alloc,
-                Err(err) => {
-                    if !err.is_dead() {
-                        pr_warn!("Failure in copy_transaction_data: {:?}", err);
-                    }
-                    return Err(err);
+        let mut alloc = match from.copy_transaction_data(
+            to.clone(),
+            tr,
+            allow_fds,
+            txn_security_ctx_off.as_mut(),
+        ) {
+            Ok(alloc) => alloc,
+            Err(err) => {
+                if !err.is_dead() {
+                    pr_warn!("Failure in copy_transaction_data: {:?}", err);
                 }
-            };
+                return Err(err);
+            }
+        };
         if trd.flags & TF_ONE_WAY != 0 {
             if stack_next.is_some() {
                 pr_warn!("Oneway transaction should not be in a transaction stack.");
@@ -97,9 +102,10 @@ pub(crate) fn new_reply(
         from: &Arc<Thread>,
         to: Arc<Process>,
         tr: &BinderTransactionDataSg,
+        allow_fds: bool,
     ) -> BinderResult<DLArc<Self>> {
         let trd = &tr.transaction_data;
-        let mut alloc = match from.copy_transaction_data(to.clone(), tr, None) {
+        let mut alloc = match from.copy_transaction_data(to.clone(), tr, allow_fds, None) {
             Ok(alloc) => alloc,
             Err(err) => {
                 pr_warn!("Failure in copy_transaction_data: {:?}", err);
@@ -210,6 +216,22 @@ pub(crate) fn submit(self: DLArc<Self>) -> BinderResult {
             }
         }
     }
+
+    fn prepare_file_list(&self) -> Result<TranslatedFds> {
+        let mut alloc = self.allocation.lock().take().ok_or(ESRCH)?;
+
+        match alloc.translate_fds() {
+            Ok(translated) => {
+                *self.allocation.lock() = Some(alloc);
+                Ok(translated)
+            }
+            Err(err) => {
+                // Free the allocation eagerly.
+                drop(alloc);
+                Err(err)
+            }
+        }
+    }
 }
 
 impl DeliverToRead for Transaction {
@@ -220,6 +242,13 @@ fn do_work(self: DArc<Self>, thread: &Thread, writer: &mut UserSlicePtrWriter) -
                 self.from.deliver_reply(reply, &self);
             }
         });
+        let files = if let Ok(list) = self.prepare_file_list() {
+            list
+        } else {
+            // On failure to process the list, we send a reply back to the sender and ignore the
+            // transaction on the recipient.
+            return Ok(true);
+        };
 
         let mut tr_sec = BinderTransactionDataSecctx::default();
         let tr = tr_sec.tr_data();
@@ -269,6 +298,8 @@ fn do_work(self: DArc<Self>, thread: &Thread, writer: &mut UserSlicePtrWriter) -
             alloc.keep_alive();
         }
 
+        files.commit();
+
         // When this is not a reply and not a oneway transaction, update `current_transaction`. If
         // it's a reply, `current_transaction` has already been updated appropriately.
         if self.target_node.is_some() && tr_sec.transaction_data.flags & TF_ONE_WAY == 0 {
diff --git a/rust/helpers.c b/rust/helpers.c
index 924c7a00f433..be295d8bdb46 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -349,6 +349,14 @@ int rust_helper_security_binder_transfer_binder(const struct cred *from,
 	return security_binder_transfer_binder(from, to);
 }
 EXPORT_SYMBOL_GPL(rust_helper_security_binder_transfer_binder);
+
+int rust_helper_security_binder_transfer_file(const struct cred *from,
+					      const struct cred *to,
+					      struct file *file)
+{
+	return security_binder_transfer_file(from, to, file);
+}
+EXPORT_SYMBOL_GPL(rust_helper_security_binder_transfer_file);
 #endif
 
 /*
diff --git a/rust/kernel/file.rs b/rust/kernel/file.rs
index 2e983285cc16..a0319c93f367 100644
--- a/rust/kernel/file.rs
+++ b/rust/kernel/file.rs
@@ -107,7 +107,7 @@ pub mod flags {
 /// Instances of this type are always ref-counted, that is, a call to `get_file` ensures that the
 /// allocation remains valid at least until the matching call to `fput`.
 #[repr(transparent)]
-pub struct File(Opaque<bindings::file>);
+pub struct File(pub(crate) Opaque<bindings::file>);
 
 // SAFETY: By design, the only way to access a `File` is via an immutable reference or an `ARef`.
 // This means that the only situation in which a `File` can be accessed mutably is when the
diff --git a/rust/kernel/security.rs b/rust/kernel/security.rs
index 9179fc225406..d308b8183c59 100644
--- a/rust/kernel/security.rs
+++ b/rust/kernel/security.rs
@@ -8,6 +8,7 @@
     bindings,
     cred::Credential,
     error::{to_result, Result},
+    file::File,
 };
 
 /// Calls the security modules to determine if the given task can become the manager of a binder
@@ -31,6 +32,16 @@ pub fn binder_transfer_binder(from: &Credential, to: &Credential) -> Result {
     to_result(unsafe { bindings::security_binder_transfer_binder(from.0.get(), to.0.get()) })
 }
 
+/// Calls the security modules to determine if task `from` is allowed to send the given file to
+/// task `to` (which would get its own file descriptor) through a binder transaction.
+pub fn binder_transfer_file(from: &Credential, to: &Credential, file: &File) -> Result {
+    // SAFETY: `from`, `to` and `file` are valid because the shared references guarantee nonzero
+    // refcounts.
+    to_result(unsafe {
+        bindings::security_binder_transfer_file(from.0.get(), to.0.get(), file.0.get())
+    })
+}
+
 /// A security context string.
 ///
 /// The struct has the invariant that it always contains a valid security context.

-- 
2.42.0.820.g83a721a137-goog


  parent reply	other threads:[~2023-11-01 18:03 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-01 18:01 [PATCH RFC 00/20] Setting up Binder for the future Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 01/20] rust_binder: define a Rust binder driver Alice Ryhl
2023-11-01 18:09   ` Greg Kroah-Hartman
2023-11-08 10:38     ` Alice Ryhl
2023-11-01 18:25   ` Boqun Feng
2023-11-02 10:27     ` Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 02/20] rust_binder: add binderfs support to Rust binder Alice Ryhl
2023-11-01 18:10   ` Greg Kroah-Hartman
2023-11-08 10:42     ` Alice Ryhl
2023-11-03 10:11   ` Finn Behrens
2023-11-08 10:31     ` Alice Ryhl
2023-11-03 16:30   ` Benno Lossin
2023-11-03 17:34     ` Boqun Feng
2023-11-08 10:25     ` Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 03/20] rust_binder: add threading support Alice Ryhl
2023-11-03 10:51   ` Finn Behrens
2023-11-08 10:27     ` Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 04/20] rust_binder: add work lists Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 05/20] rust_binder: add nodes and context managers Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 06/20] rust_binder: add oneway transactions Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 07/20] rust_binder: add epoll support Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 08/20] rust_binder: add non-oneway transactions Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 09/20] rust_binder: serialize oneway transactions Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 10/20] rust_binder: add death notifications Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 11/20] rust_binder: send nodes in transactions Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 12/20] rust_binder: add BINDER_TYPE_PTR support Alice Ryhl
2023-11-01 18:01 ` Alice Ryhl [this message]
2023-11-01 18:01 ` [PATCH RFC 14/20] rust_binder: add BINDER_TYPE_FDA support Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 15/20] rust_binder: add process freezing Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 16/20] rust_binder: add TF_UPDATE_TXN support Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 17/20] rust_binder: add oneway spam detection Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 18/20] rust_binder: add binder_logs/state Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 19/20] rust_binder: add vma shrinker Alice Ryhl
2023-11-01 18:01 ` [PATCH RFC 20/20] binder: delete the C implementation Alice Ryhl
2023-11-01 18:15   ` Greg Kroah-Hartman
2023-11-01 18:39   ` Carlos Llamas
2023-11-01 18:34 ` [PATCH RFC 00/20] Setting up Binder for the future Carlos Llamas
2023-11-02 13:33   ` 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=20231101-rust-binder-v1-13-08ba9197f637@google.com \
    --to=aliceryhl@google.com \
    --cc=a.hindborg@samsung.com \
    --cc=alex.gaynor@gmail.com \
    --cc=arve@android.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=brauner@kernel.org \
    --cc=cmllamas@google.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jeffv@google.com \
    --cc=joel@joelfernandes.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maco@android.com \
    --cc=mattgilbride@google.com \
    --cc=mmaurer@google.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=surenb@google.com \
    --cc=tkjos@android.com \
    --cc=wedsonaf@gmail.com \
    /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;
as well as URLs for NNTP newsgroup(s).