Skip to content

Commit bffa972

Browse files
committed
Merge tag 'char-misc-7.2-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc
Pull Android/IIO fixes from Greg KH: "Here is a set of bugfixes for 7.2-rc3 that resolve a bunch of reported issues in just the binder and iio codebases. Included in here are: - binder driver bugfixes for both the rust and c versions for reported problems - lots and lots of iio driver bugfixes for lots of reported issues (including a hid sensor driver bugfix) Full details are in the shortlog, all of these have been in linux-next with no reported issues" * tag 'char-misc-7.2-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc: (36 commits) iio: event: Fix event FIFO reset race iio: imu: inv_icm42600: fix timestamp clock period by using lower value iio: light: al3010: fix incorrect scale for the highest gain range iio: adc: nxp-sar-adc: Fix the delay calculation in nxp_sar_adc_wait_for() iio: light: tsl2591: return actual error from probe IRQ failure iio: imu: inv_icm42600: fix timestamping by limiting FIFO reading iio: imu: st_lsm6dsx: deselect shub page before reading whoami rust_binder: clear freeze listener on node removal rust_binder: reject context manager self-transaction rust_binder: use a u64 stride when cleaning up the offsets array binder: fix UAF in binder_free_transaction() binder: fix UAF in binder_thread_release() rust_binder: synchronize Rust Binder stats with freeze commands binder: cache secctx size before release zeroes it rust_binder: fix BINDER_GET_EXTENDED_ERROR iio: adc: ad7779: add missing 'select IIO_TRIGGERED_BUFFER' to Kconfig iio: adc: ad4130: add missing `select IIO_TRIGGERED_BUFFER` to Kconfig iio: adc: ti-ads124s08: Return reset GPIO lookup errors iio: temperature: Build mlx90635 with CONFIG_MLX90635 iio: light: al3320a: add missing REGMAP_I2C to Kconfig ...
2 parents 8a65af0 + 3585cfd commit bffa972

37 files changed

Lines changed: 362 additions & 117 deletions

drivers/android/binder.c

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1658,7 +1658,20 @@ static void binder_txn_latency_free(struct binder_transaction *t)
16581658

16591659
static void binder_free_transaction(struct binder_transaction *t)
16601660
{
1661-
struct binder_proc *target_proc = t->to_proc;
1661+
struct binder_thread *target_thread;
1662+
struct binder_proc *target_proc;
1663+
1664+
spin_lock(&t->lock);
1665+
target_proc = t->to_proc;
1666+
target_thread = t->to_thread;
1667+
/*
1668+
* Pin target_thread to keep target_proc alive. Undelivered
1669+
* transactions with !target_thread are safe, as target_proc
1670+
* can only be the current context there.
1671+
*/
1672+
if (target_thread)
1673+
atomic_inc(&target_thread->tmp_ref);
1674+
spin_unlock(&t->lock);
16621675

16631676
if (target_proc) {
16641677
binder_inner_proc_lock(target_proc);
@@ -1672,6 +1685,10 @@ static void binder_free_transaction(struct binder_transaction *t)
16721685
t->buffer->transaction = NULL;
16731686
binder_inner_proc_unlock(target_proc);
16741687
}
1688+
1689+
if (target_thread)
1690+
binder_thread_dec_tmpref(target_thread);
1691+
16751692
if (trace_binder_txn_latency_free_enabled())
16761693
binder_txn_latency_free(t);
16771694
/*
@@ -3080,6 +3097,7 @@ static void binder_transaction(struct binder_proc *proc,
30803097
int t_debug_id = atomic_inc_return(&binder_last_id);
30813098
ktime_t t_start_time = ktime_get();
30823099
struct lsm_context lsmctx = { };
3100+
size_t lsmctx_aligned_size = 0;
30833101
LIST_HEAD(sgc_head);
30843102
LIST_HEAD(pf_head);
30853103
const void __user *user_buffer = (const void __user *)
@@ -3346,7 +3364,6 @@ static void binder_transaction(struct binder_proc *proc,
33463364

33473365
if (target_node && target_node->txn_security_ctx) {
33483366
u32 secid;
3349-
size_t added_size;
33503367

33513368
security_cred_getsecid(proc->cred, &secid);
33523369
ret = security_secid_to_secctx(secid, &lsmctx);
@@ -3358,9 +3375,9 @@ static void binder_transaction(struct binder_proc *proc,
33583375
return_error_line = __LINE__;
33593376
goto err_get_secctx_failed;
33603377
}
3361-
added_size = ALIGN(lsmctx.len, sizeof(u64));
3362-
extra_buffers_size += added_size;
3363-
if (extra_buffers_size < added_size) {
3378+
lsmctx_aligned_size = ALIGN(lsmctx.len, sizeof(u64));
3379+
extra_buffers_size += lsmctx_aligned_size;
3380+
if (extra_buffers_size < lsmctx_aligned_size) {
33643381
binder_txn_error("%d:%d integer overflow of extra_buffers_size\n",
33653382
thread->pid, proc->pid);
33663383
return_error = BR_FAILED_REPLY;
@@ -3397,7 +3414,7 @@ static void binder_transaction(struct binder_proc *proc,
33973414
size_t buf_offset = ALIGN(tr->data_size, sizeof(void *)) +
33983415
ALIGN(tr->offsets_size, sizeof(void *)) +
33993416
ALIGN(extra_buffers_size, sizeof(void *)) -
3400-
ALIGN(lsmctx.len, sizeof(u64));
3417+
lsmctx_aligned_size;
34013418

34023419
t->security_ctx = t->buffer->user_data + buf_offset;
34033420
err = binder_alloc_copy_to_buffer(&target_proc->alloc,
@@ -3452,7 +3469,7 @@ static void binder_transaction(struct binder_proc *proc,
34523469
off_end_offset = off_start_offset + tr->offsets_size;
34533470
sg_buf_offset = ALIGN(off_end_offset, sizeof(void *));
34543471
sg_buf_end_offset = sg_buf_offset + extra_buffers_size -
3455-
ALIGN(lsmctx.len, sizeof(u64));
3472+
lsmctx_aligned_size;
34563473
off_min = 0;
34573474
for (buffer_offset = off_start_offset; buffer_offset < off_end_offset;
34583475
buffer_offset += sizeof(binder_size_t)) {

drivers/android/binder/allocation.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ impl Drop for Allocation {
259259

260260
if let Some(offsets) = info.offsets.clone() {
261261
let view = AllocationView::new(self, offsets.start);
262-
for i in offsets.step_by(size_of::<usize>()) {
262+
for i in offsets.step_by(size_of::<u64>()) {
263263
if view.cleanup_object(i).is_err() {
264264
pr_warn!("Error cleaning up object at offset {}\n", i)
265265
}
@@ -420,7 +420,8 @@ impl<'a> AllocationView<'a> {
420420
}
421421

422422
fn cleanup_object(&self, index_offset: usize) -> Result {
423-
let offset = self.alloc.read(index_offset)?;
423+
let offset = self.alloc.read::<u64>(index_offset)?;
424+
let offset: usize = offset.try_into().map_err(|_| EINVAL)?;
424425
let header = self.read::<BinderObjectHeader>(offset)?;
425426
match header.type_ {
426427
BINDER_TYPE_WEAK_BINDER | BINDER_TYPE_BINDER => {

drivers/android/binder/error.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,17 @@ impl fmt::Debug for BinderError {
7373
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
7474
match self.reply {
7575
BR_FAILED_REPLY => match self.source.as_ref() {
76-
Some(source) => f
77-
.debug_struct("BR_FAILED_REPLY")
78-
.field("source", source)
79-
.finish(),
76+
Some(source) => source.fmt(f),
8077
None => f.pad("BR_FAILED_REPLY"),
8178
},
8279
BR_DEAD_REPLY => f.pad("BR_DEAD_REPLY"),
8380
BR_FROZEN_REPLY => f.pad("BR_FROZEN_REPLY"),
8481
BR_TRANSACTION_PENDING_FROZEN => f.pad("BR_TRANSACTION_PENDING_FROZEN"),
8582
BR_TRANSACTION_COMPLETE => f.pad("BR_TRANSACTION_COMPLETE"),
86-
_ => f
87-
.debug_struct("BinderError")
88-
.field("reply", &self.reply)
89-
.finish(),
83+
_ => match self.source.as_ref() {
84+
Some(source) => source.fmt(f),
85+
None => self.reply.fmt(f),
86+
},
9087
}
9188
}
9289
}

drivers/android/binder/freeze.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,17 @@ impl DeliverToRead for FreezeMessage {
154154
}
155155

156156
impl FreezeListener {
157-
pub(crate) fn on_process_exit(&self, proc: &Arc<Process>) {
157+
/// Called when this freeze listener is cleared abnormally.
158+
///
159+
/// This occurs either because the process exited or because the process dropped its last
160+
/// refcount on the node ref without explicitly removing the freeze listener first.
161+
///
162+
/// The returned `KVVec` is just a value that should be dropped outside of the lock.
163+
pub(crate) fn on_process_cleanup(&self, proc: &Process) -> KVVec<Arc<Process>> {
158164
if !self.is_clearing {
159-
self.node.remove_freeze_listener(proc);
165+
return self.node.remove_freeze_listener(proc);
160166
}
167+
KVVec::new()
161168
}
162169
}
163170

drivers/android/binder/node.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -682,21 +682,23 @@ impl Node {
682682
}
683683
}
684684

685-
pub(crate) fn remove_freeze_listener(&self, p: &Arc<Process>) {
686-
let _unused_capacity;
685+
pub(crate) fn remove_freeze_listener(&self, p: &Process) -> KVVec<Arc<Process>> {
687686
let mut guard = self.owner.inner.lock();
688687
let inner = self.inner.access_mut(&mut guard);
689688
let len = inner.freeze_list.len();
690-
inner.freeze_list.retain(|proc| !Arc::ptr_eq(proc, p));
689+
inner
690+
.freeze_list
691+
.retain(|proc| !core::ptr::eq::<Process>(&**proc, p));
691692
if len == inner.freeze_list.len() {
692693
pr_warn!(
693694
"Could not remove freeze listener for {}\n",
694695
p.pid_in_current_ns()
695696
);
696697
}
697698
if inner.freeze_list.is_empty() {
698-
_unused_capacity = mem::take(&mut inner.freeze_list);
699+
return mem::take(&mut inner.freeze_list);
699700
}
701+
KVVec::new()
700702
}
701703

702704
pub(crate) fn freeze_list<'a>(&'a self, guard: &'a ProcessInner) -> &'a [Arc<Process>] {

drivers/android/binder/process.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,11 @@ impl Process {
900900
pub(crate) fn get_transaction_node(&self, handle: u32) -> BinderResult<NodeRef> {
901901
// When handle is zero, try to get the context manager.
902902
if handle == 0 {
903-
Ok(self.ctx.get_manager_node(true)?)
903+
let node_ref = self.ctx.get_manager_node(true)?;
904+
if core::ptr::eq(self, &*node_ref.node.owner) {
905+
return Err(EINVAL.into());
906+
}
907+
Ok(node_ref)
904908
} else {
905909
Ok(self.get_node_from_handle(handle, true)?)
906910
}
@@ -942,6 +946,8 @@ impl Process {
942946

943947
// To preserve original binder behaviour, we only fail requests where the manager tries to
944948
// increment references on itself.
949+
let _to_free_freeze_listener;
950+
let _to_free_freeze_listener_cleanup;
945951
let mut refs = self.node_refs.lock();
946952
if let Some(info) = refs.by_handle.get_mut(&handle) {
947953
if info.node_ref().update(inc, strong) {
@@ -957,6 +963,14 @@ impl Process {
957963
unsafe { info.node_ref2().node.remove_node_info(info) };
958964

959965
let id = info.node_ref().node.global_id();
966+
967+
if let Some(freeze) = *info.freeze() {
968+
if let Some(fl) = refs.freeze_listeners.remove(&freeze) {
969+
_to_free_freeze_listener_cleanup = fl.on_process_cleanup(&self);
970+
_to_free_freeze_listener = fl;
971+
}
972+
}
973+
960974
refs.by_handle.remove(&handle);
961975
refs.by_node.remove(&id);
962976
refs.handle_is_present.release_id(handle as usize);
@@ -1380,7 +1394,7 @@ impl Process {
13801394
// Clean up freeze listeners.
13811395
let freeze_listeners = take(&mut self.node_refs.lock().freeze_listeners);
13821396
for listener in freeze_listeners.values() {
1383-
listener.on_process_exit(&self);
1397+
listener.on_process_cleanup(&self);
13841398
}
13851399
drop(freeze_listeners);
13861400

drivers/android/binder/rust_binder_events.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ const char * const binder_command_strings[] = {
2828
"BC_DEAD_BINDER_DONE",
2929
"BC_TRANSACTION_SG",
3030
"BC_REPLY_SG",
31+
"BC_REQUEST_FREEZE_NOTIFICATION",
32+
"BC_CLEAR_FREEZE_NOTIFICATION",
33+
"BC_FREEZE_NOTIFICATION_DONE",
3134
};
3235

3336
const char * const binder_return_strings[] = {
@@ -51,7 +54,9 @@ const char * const binder_return_strings[] = {
5154
"BR_FAILED_REPLY",
5255
"BR_FROZEN_REPLY",
5356
"BR_ONEWAY_SPAM_SUSPECT",
54-
"BR_TRANSACTION_PENDING_FROZEN"
57+
"BR_TRANSACTION_PENDING_FROZEN",
58+
"BR_FROZEN_BINDER",
59+
"BR_CLEAR_FREEZE_NOTIFICATION_DONE",
5560
};
5661

5762
#define CREATE_TRACE_POINTS

drivers/android/binder/stats.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use crate::defs::*;
88
use kernel::sync::atomic::{ordering::Relaxed, Atomic};
99
use kernel::{ioctl::_IOC_NR, seq_file::SeqFile, seq_print};
1010

11-
const BC_COUNT: usize = _IOC_NR(BC_REPLY_SG) as usize + 1;
12-
const BR_COUNT: usize = _IOC_NR(BR_TRANSACTION_PENDING_FROZEN) as usize + 1;
11+
const BC_COUNT: usize = _IOC_NR(BC_FREEZE_NOTIFICATION_DONE) as usize + 1;
12+
const BR_COUNT: usize = _IOC_NR(BR_CLEAR_FREEZE_NOTIFICATION_DONE) as usize + 1;
1313

1414
pub(crate) static GLOBAL_STATS: BinderStats = BinderStats::new();
1515

drivers/android/binder/thread.rs

Lines changed: 46 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -495,9 +495,16 @@ impl Thread {
495495
Ok(())
496496
}
497497

498+
pub(crate) fn clear_extended_error(&self, debug_id: usize) {
499+
self.inner.lock().extended_error = ExtendedError::new(debug_id as u32, BR_OK, 0);
500+
}
501+
498502
pub(crate) fn get_extended_error(&self, data: UserSlice) -> Result {
499503
let mut writer = data.writer();
500-
let ee = self.inner.lock().extended_error;
504+
let mut inner = self.inner.lock();
505+
let ee = inner.extended_error;
506+
inner.extended_error = ExtendedError::new(0, BR_OK, 0);
507+
drop(inner);
501508
writer.write(&ee)?;
502509
Ok(())
503510
}
@@ -1109,7 +1116,10 @@ impl Thread {
11091116
inner.pop_transaction_to_reply(thread.as_ref())
11101117
} {
11111118
let reply = Err(BR_DEAD_REPLY);
1112-
if !transaction.from.deliver_single_reply(reply, &transaction) {
1119+
if !transaction
1120+
.from
1121+
.deliver_single_reply(reply, &transaction, None)
1122+
{
11131123
break;
11141124
}
11151125

@@ -1121,8 +1131,9 @@ impl Thread {
11211131
&self,
11221132
reply: Result<DLArc<Transaction>, u32>,
11231133
transaction: &DArc<Transaction>,
1134+
extended_error: Option<ExtendedError>,
11241135
) {
1125-
if self.deliver_single_reply(reply, transaction) {
1136+
if self.deliver_single_reply(reply, transaction, extended_error) {
11261137
transaction.from.unwind_transaction_stack();
11271138
}
11281139
}
@@ -1136,6 +1147,7 @@ impl Thread {
11361147
&self,
11371148
reply: Result<DLArc<Transaction>, u32>,
11381149
transaction: &DArc<Transaction>,
1150+
extended_error: Option<ExtendedError>,
11391151
) -> bool {
11401152
if let Ok(transaction) = &reply {
11411153
crate::trace::trace_transaction(true, transaction, Some(&self.task));
@@ -1152,6 +1164,12 @@ impl Thread {
11521164
return true;
11531165
}
11541166

1167+
if let Some(ee) = extended_error {
1168+
if inner.extended_error.command == BR_OK {
1169+
inner.extended_error = ee;
1170+
}
1171+
}
1172+
11551173
match reply {
11561174
Ok(work) => {
11571175
inner.push_work(work);
@@ -1222,6 +1240,9 @@ impl Thread {
12221240
info.buffers_size = td.buffers_size as usize;
12231241
// SAFETY: Above `read` call initializes all bytes, so this union read is ok.
12241242
info.target_handle = unsafe { td.transaction_data.target.handle };
1243+
1244+
info.debug_id = super::next_debug_id();
1245+
12251246
Ok(())
12261247
}
12271248

@@ -1230,6 +1251,8 @@ impl Thread {
12301251
let mut info = TransactionInfo::zeroed();
12311252
self.read_transaction_info(cmd, reader, &mut info)?;
12321253

1254+
self.clear_extended_error(info.debug_id);
1255+
12331256
let ret = if info.is_reply {
12341257
self.reply_inner(&mut info)
12351258
} else if info.is_oneway() {
@@ -1239,23 +1262,21 @@ impl Thread {
12391262
};
12401263

12411264
if let Err(err) = ret {
1242-
if err.reply != BR_TRANSACTION_COMPLETE {
1243-
info.reply = err.reply;
1244-
}
1245-
12461265
self.push_return_work(err.reply);
1247-
if let Some(source) = &err.source {
1248-
info.errno = source.to_errno();
1266+
if err.reply != BR_TRANSACTION_COMPLETE {
12491267
info.reply = err.reply;
1268+
if let Some(source) = &err.source {
1269+
info.errno = source.to_errno();
12501270

1251-
{
1252-
let mut ee = self.inner.lock().extended_error;
1253-
ee.command = err.reply;
1254-
ee.param = source.to_errno();
1271+
{
1272+
let mut inner = self.inner.lock();
1273+
inner.extended_error =
1274+
ExtendedError::new(info.debug_id as u32, err.reply, source.to_errno());
1275+
}
12551276
}
12561277

12571278
pr_warn!(
1258-
"{}:{} transaction to {} failed: {source:?}",
1279+
"{}:{} transaction to {} failed: {err:?}",
12591280
info.from_pid,
12601281
info.from_tid,
12611282
info.to_pid
@@ -1320,18 +1341,24 @@ impl Thread {
13201341
let allow_fds = orig.flags & TF_ACCEPT_FDS != 0;
13211342
let reply = Transaction::new_reply(self, process, info, allow_fds)?;
13221343
self.inner.lock().push_work(completion);
1323-
orig.from.deliver_reply(Ok(reply), &orig);
1344+
orig.from.deliver_reply(Ok(reply), &orig, None);
13241345
Ok(())
13251346
})()
13261347
.map_err(|mut err| {
13271348
// At this point we only return `BR_TRANSACTION_COMPLETE` to the caller, and we must let
13281349
// the sender know that the transaction has completed (with an error in this case).
1350+
13291351
pr_warn!(
1330-
"Failure {:?} during reply - delivering BR_FAILED_REPLY to sender.",
1331-
err
1352+
"{}:{} reply to {} failed: {err:?}",
1353+
info.from_pid,
1354+
info.from_tid,
1355+
info.to_pid
13321356
);
1333-
let reply = Err(BR_FAILED_REPLY);
1334-
orig.from.deliver_reply(reply, &orig);
1357+
1358+
let param = err.source.as_ref().map_or(0, |e| e.to_errno());
1359+
let ee = ExtendedError::new(info.debug_id as u32, err.reply, param);
1360+
orig.from
1361+
.deliver_reply(Err(BR_FAILED_REPLY), &orig, Some(ee));
13351362
err.reply = BR_TRANSACTION_COMPLETE;
13361363
err
13371364
});

0 commit comments

Comments
 (0)