Skip to content

binder: Reject transactions containig FDs when they're not allowed. #318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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