From ddbe5d23cafe16de2f5342a15d9ad9e13b9857d7 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Sun, 28 Nov 2021 04:54:51 +0000 Subject: [PATCH] rust: binder: use credentials in binder security callbacks. Signed-off-by: Wedson Almeida Filho --- drivers/android/context.rs | 2 +- drivers/android/process.rs | 11 +++++-- drivers/android/thread.rs | 12 +++---- rust/kernel/security.rs | 64 +++++++++----------------------------- 4 files changed, 29 insertions(+), 60 deletions(-) diff --git a/drivers/android/context.rs b/drivers/android/context.rs index e2589f7d0fea3d..3661e7a4f646ee 100644 --- a/drivers/android/context.rs +++ b/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(); diff --git a/drivers/android/process.rs b/drivers/android/process.rs index 274e44375daf49..ce3ab176b8acec 100644 --- a/drivers/android/process.rs +++ b/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) -> Result> { + fn new(ctx: Ref, cred: Credential) -> Result> { 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> for Process { - fn open(ctx: &Ref, _file: &File) -> Result { - Self::new(ctx.clone()) + fn open(ctx: &Ref, file: &File) -> Result { + Self::new(ctx.clone(), file.cred().clone()) } } diff --git a/drivers/android/thread.rs b/drivers/android/thread.rs index b2551a1e88c73a..3891abca798b87 100644 --- a/drivers/android/thread.rs +++ b/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, 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, 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()?; diff --git a/rust/kernel/security.rs b/rust/kernel/security.rs index 3364126d677bd1..2004d01233f445 100644 --- a/rust/kernel/security.rs +++ b/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) }) }