Merge pull request #577 from wedsonaf/security

rust: binder: use credentials in binder security callbacks.

Author: wedsonaf

drivers/android/context.rs

@@ -48,7 +48,7 @@ impl Context {
         if manager.node.is_some() {
             return Err(Error::EBUSY);
         }
-        security::binder_set_context_mgr(&node_ref.node.owner.task)?;
+        security::binder_set_context_mgr(&node_ref.node.owner.cred)?;
 
         // TODO: Get the actual caller id.
         let caller_uid = bindings::kuid_t::default();

drivers/android/process.rs

@@ -3,6 +3,7 @@
 use core::{convert::TryFrom, mem::take, ops::Range};
 use kernel::{
     bindings, c_types,
+    cred::Credential,
     file::File,
     file_operations::{FileOpener, FileOperations, IoctlCommand, IoctlHandler, PollTable},
     io_buffer::{IoBufferReader, IoBufferWriter},
@@ -245,6 +246,9 @@ pub(crate) struct Process {
     // The task leader (process).
     pub(crate) task: Task,
 
+    // Credential associated with file when `Process` is created.
+    pub(crate) cred: Credential,
+
     // TODO: For now this a mutex because we have allocations in RangeAllocator while holding the
     // lock. We may want to split up the process state at some point to use a spin lock for the
     // other fields.
@@ -260,9 +264,10 @@ unsafe impl Send for Process {}
 unsafe impl Sync for Process {}
 
 impl Process {
-    fn new(ctx: Ref<Context>) -> Result<Ref<Self>> {
+    fn new(ctx: Ref<Context>, cred: Credential) -> Result<Ref<Self>> {
         let mut process = Pin::from(UniqueRef::try_new(Self {
             ctx,
+            cred,
             task: Task::current().group_leader().clone(),
             // SAFETY: `inner` is initialised in the call to `mutex_init` below.
             inner: unsafe { Mutex::new(ProcessInner::new()) },
@@ -802,8 +807,8 @@ impl IoctlHandler for Process {
 }
 
 impl FileOpener<Ref<Context>> for Process {
-    fn open(ctx: &Ref<Context>, _file: &File) -> Result<Self::Wrapper> {
-        Self::new(ctx.clone())
+    fn open(ctx: &Ref<Context>, file: &File) -> Result<Self::Wrapper> {
+        Self::new(ctx.clone(), file.cred().clone())
     }
 }
 

drivers/android/thread.rs

@@ -402,7 +402,7 @@ impl Thread {
                         strong,
                         Some(self),
                     )?;
-                    security::binder_transfer_binder(&self.process.task, &view.alloc.process.task)?;
+                    security::binder_transfer_binder(&self.process.cred, &view.alloc.process.cred)?;
                     Ok(node)
                 })?;
             }
@@ -412,7 +412,7 @@ impl Thread {
                     // SAFETY: `handle` is a `u32`; any bit pattern is a valid representation.
                     let handle = unsafe { obj.__bindgen_anon_1.handle } as _;
                     let node = self.process.get_node_from_handle(handle, strong)?;
-                    security::binder_transfer_binder(&self.process.task, &view.alloc.process.task)?;
+                    security::binder_transfer_binder(&self.process.cred, &view.alloc.process.cred)?;
                     Ok(node)
                 })?;
             }
@@ -426,8 +426,8 @@ impl Thread {
                 let fd = unsafe { obj.__bindgen_anon_1.fd };
                 let file = File::from_fd(fd)?;
                 security::binder_transfer_file(
-                    &self.process.task,
-                    &view.alloc.process.task,
+                    &self.process.cred,
+                    &view.alloc.process.cred,
                     &file,
                 )?;
                 let field_offset =
@@ -618,7 +618,7 @@ impl Thread {
     fn oneway_transaction_inner(self: &Ref<Self>, tr: &BinderTransactionData) -> BinderResult {
         let handle = unsafe { tr.target.handle };
         let node_ref = self.process.get_transaction_node(handle)?;
-        security::binder_transaction(&self.process.task, &node_ref.node.owner.task)?;
+        security::binder_transaction(&self.process.cred, &node_ref.node.owner.cred)?;
         let completion = Ref::try_new(DeliverCode::new(BR_TRANSACTION_COMPLETE))?;
         let transaction = Transaction::new(node_ref, None, self, tr)?;
         self.inner.lock().push_work(completion);
@@ -630,7 +630,7 @@ impl Thread {
     fn transaction_inner(self: &Ref<Self>, tr: &BinderTransactionData) -> BinderResult {
         let handle = unsafe { tr.target.handle };
         let node_ref = self.process.get_transaction_node(handle)?;
-        security::binder_transaction(&self.process.task, &node_ref.node.owner.task)?;
+        security::binder_transaction(&self.process.cred, &node_ref.node.owner.cred)?;
         // TODO: We need to ensure that there isn't a pending transaction in the work queue. How
         // could this happen?
         let top = self.top_of_transaction_stack()?;

rust/kernel/security.rs

@@ -4,69 +4,33 @@
 //!
 //! C header: [`include/linux/security.h`](../../../../include/linux/security.h).
 
-use crate::{file::File, task::Task, Result};
+use crate::{bindings, cred::Credential, file::File, to_result, Result};
 
 /// Calls the security modules to determine if the given task can become the manager of a binder
 /// context.
-pub fn binder_set_context_mgr(_mgr: &Task) -> Result {
-    // TODO
-    //
-    // // SAFETY: By the `Task` invariants, `mgr.ptr` is valid.
-    // let ret = unsafe { bindings::security_binder_set_context_mgr(mgr.ptr) };
-    // if ret != 0 {
-    //     Err(Error::from_kernel_errno(ret))
-    // } else {
-    //     Ok(())
-    // }
-
-    Ok(())
+pub fn binder_set_context_mgr(mgr: &Credential) -> Result {
+    // SAFETY: By the `Credential` invariants, `mgr.ptr` is valid.
+    to_result(|| unsafe { bindings::security_binder_set_context_mgr(mgr.ptr) })
 }
 
 /// Calls the security modules to determine if binder transactions are allowed from task `from` to
 /// task `to`.
-pub fn binder_transaction(_from: &Task, _to: &Task) -> Result {
-    // TODO
-    //
-    // // SAFETY: By the `Task` invariants, `from.ptr` and `to.ptr` are valid.
-    // let ret = unsafe { bindings::security_binder_transaction(from.ptr, to.ptr) };
-    // if ret != 0 {
-    //     Err(Error::from_kernel_errno(ret))
-    // } else {
-    //     Ok(())
-    // }
-
-    Ok(())
+pub fn binder_transaction(from: &Credential, to: &Credential) -> Result {
+    // SAFETY: By the `Credential` invariants, `from.ptr` and `to.ptr` are valid.
+    to_result(|| unsafe { bindings::security_binder_transaction(from.ptr, to.ptr) })
 }
 
 /// Calls the security modules to determine if task `from` is allowed to send binder objects
 /// (owned by itself or other processes) to task `to` through a binder transaction.
-pub fn binder_transfer_binder(_from: &Task, _to: &Task) -> Result {
-    // TODO
-    //
-    // // SAFETY: By the `Task` invariants, `from.ptr` and `to.ptr` are valid.
-    // let ret = unsafe { bindings::security_binder_transfer_binder(from.ptr, to.ptr) };
-    // if ret != 0 {
-    //     Err(Error::from_kernel_errno(ret))
-    // } else {
-    //     Ok(())
-    // }
-
-    Ok(())
+pub fn binder_transfer_binder(from: &Credential, to: &Credential) -> Result {
+    // SAFETY: By the `Credential` invariants, `from.ptr` and `to.ptr` are valid.
+    to_result(|| unsafe { bindings::security_binder_transfer_binder(from.ptr, to.ptr) })
 }
 
 /// 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: &Task, _to: &Task, _file: &File) -> Result {
-    // TODO
-    //
-    // // SAFETY: By the `Task` invariants, `from.ptr` and `to.ptr` are valid. Similarly, by the
-    // // `File` invariants, `file.ptr` is also valid.
-    // let ret = unsafe { bindings::security_binder_transfer_file(from.ptr, to.ptr, file.ptr) };
-    // if ret != 0 {
-    //     Err(Error::from_kernel_errno(ret))
-    // } else {
-    //     Ok(())
-    // }
-
-    Ok(())
+pub fn binder_transfer_file(from: &Credential, to: &Credential, file: &File) -> Result {
+    // SAFETY: By the `Credential` invariants, `from.ptr` and `to.ptr` are valid. Similarly, by the
+    // `File` invariants, `file.ptr` is also valid.
+    to_result(|| unsafe { bindings::security_binder_transfer_file(from.ptr, to.ptr, file.ptr) })
 }