Skip to content

Commit

Permalink
Merge pull request torvalds#577 from wedsonaf/security
Browse files Browse the repository at this point in the history 
rust: binder: use credentials in binder security callbacks.
  • Loading branch information
@wedsonaf
wedsonaf authored Nov 30, 2021
2 parents e0734ef + ddbe5d2 commit 8a4fed2
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 60 deletions.
2 changes: 1 addition & 1 deletion drivers/android/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
11 changes: 8 additions & 3 deletions drivers/android/process.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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.
Expand All @@ -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()) },
Expand Down Expand Up @@ -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())
}
}

Expand Down
12 changes: 6 additions & 6 deletions drivers/android/thread.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})?;
}
Expand All @@ -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)
})?;
}
Expand All @@ -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 =
Expand Down Expand Up @@ -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);
Expand All @@ -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()?;
Expand Down
64 changes: 14 additions & 50 deletions rust/kernel/security.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) })
}

0 comments on commit 8a4fed2

Please sign in to comment.