Skip to content

Commit

Permalink
Merge pull request #318 from wedsonaf/fail-fd
Browse files Browse the repository at this point in the history
binder: Reject transactions containig FDs when they're not allowed.
  • Loading branch information
ojeda authored May 29, 2021
2 parents 934f970 + b42fcef commit 7884043
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 7 deletions.
27 changes: 23 additions & 4 deletions drivers/android/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ impl Thread {
index_offset: usize,
alloc: &Allocation,
view: &AllocationView,
allow_fds: bool,
) -> BinderResult {
let offset = alloc.read(index_offset)?;
let header = view.read::<bindings::binder_object_header>(offset)?;
Expand All @@ -403,15 +404,26 @@ impl Thread {
self.process.get_node_from_handle(handle, strong)
})?;
}
BINDER_TYPE_FD => {
if !allow_fds {
return Err(BinderError::new_failed());
}
}
_ => pr_warn!("Unsupported binder object type: {:x}\n", header.type_),
}
Ok(())
}

fn translate_objects(&self, alloc: &mut Allocation, start: usize, end: usize) -> BinderResult {
fn translate_objects(
&self,
alloc: &mut Allocation,
start: usize,
end: usize,
allow_fds: bool,
) -> BinderResult {
let view = AllocationView::new(&alloc, start);
for i in (start..end).step_by(size_of::<usize>()) {
if let Err(err) = self.translate_object(i, alloc, &view) {
if let Err(err) = self.translate_object(i, alloc, &view, allow_fds) {
alloc.set_info(AllocationInfo { offsets: start..i });
return Err(err);
}
Expand All @@ -426,6 +438,7 @@ impl Thread {
&self,
to_process: &'a Process,
tr: &BinderTransactionData,
allow_fds: bool,
) -> BinderResult<Allocation<'a>> {
let data_size = tr.data_size as _;
let adata_size = ptr_align(data_size);
Expand All @@ -450,7 +463,12 @@ impl Thread {
alloc.copy_into(&mut reader, adata_size, offsets_size)?;

// Traverse the objects specified.
self.translate_objects(&mut alloc, adata_size, adata_size + aoffsets_size)?;
self.translate_objects(
&mut alloc,
adata_size,
adata_size + aoffsets_size,
allow_fds,
)?;
}

Ok(alloc)
Expand Down Expand Up @@ -540,7 +558,8 @@ impl Thread {
(|| -> BinderResult<_> {
let completion = Arc::try_new(DeliverCode::new(BR_TRANSACTION_COMPLETE))?;
let process = orig.from.process.clone();
let reply = Arc::try_new(Transaction::new_reply(self, process, tr)?)?;
let allow_fds = orig.flags & TF_ACCEPT_FDS != 0;
let reply = Arc::try_new(Transaction::new_reply(self, process, tr, allow_fds)?)?;
self.inner.lock().push_work(completion);
orig.from.deliver_reply(Either::Left(reply), &orig);
Ok(())
Expand Down
8 changes: 5 additions & 3 deletions drivers/android/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub(crate) struct Transaction {
to: Ref<Process>,
free_allocation: AtomicBool,
code: u32,
flags: u32,
pub(crate) flags: u32,
data_size: usize,
offsets_size: usize,
data_address: usize,
Expand All @@ -38,8 +38,9 @@ impl Transaction {
from: &Arc<Thread>,
tr: &BinderTransactionData,
) -> BinderResult<Self> {
let allow_fds = node_ref.node.flags & FLAT_BINDER_FLAG_ACCEPTS_FDS != 0;
let to = node_ref.node.owner.clone();
let alloc = from.copy_transaction_data(&to, tr)?;
let alloc = from.copy_transaction_data(&to, tr, allow_fds)?;
let data_address = alloc.ptr;
alloc.keep_alive();
Ok(Self {
Expand All @@ -61,8 +62,9 @@ impl Transaction {
from: &Arc<Thread>,
to: Ref<Process>,
tr: &BinderTransactionData,
allow_fds: bool,
) -> BinderResult<Self> {
let alloc = from.copy_transaction_data(&to, tr)?;
let alloc = from.copy_transaction_data(&to, tr, allow_fds)?;
let data_address = alloc.ptr;
alloc.keep_alive();
Ok(Self {
Expand Down

0 comments on commit 7884043

Please sign in to comment.