On 05/15/2018 12:38 PM, Stephen Smalley wrote:
> On 05/15/2018 09:43 AM, Stephen Smalley wrote:
>> On 05/15/2018 09:36 AM, Stephen Smalley wrote:
>>> This test is failing for me (with or without -v):
>>> # ./test -v
>>> 1..6
>>> Manager PID: 5608 Process context:
>>> unconfined_u:unconfined_r:test_binder_mgr_t:s0-s0:c0.c1023
>>> Client PID: 5609 Process context:
>>> unconfined_u:unconfined_r:test_binder_client_t:s0-s0:c0.c1023
>>> Client read_consumed: 28
>>> Manager read_consumed: 72
>>> Client command: BR_NOOP
>>> Manager command: BR_NOOP
>>> Client command: BR_INCREFS
>>> Manager command: BR_TRANSACTION
>>> Client command: BR_TRANSACTION_COMPLETE
>>> BR_TRANSACTION data:
>>> handle: 0
>>> cookie: 0
>>> code: 0
>>> flag: TF_ACCEPT_FDS
>>> sender pid: 5609
>>> sender euid: 0
>>> data_size: 24
>>> offsets_size: 8
>>> Sending BC_REPLY
>>> Manager read_consumed: 8
>>> Manager command: BR_NOOP
>>> Manager command: BR_TRANSACTION_COMPLETE
>>> Client read_consumed: 72
>>> Client command: BR_NOOP
>>> Client command: BR_REPLY
>>> BR_REPLY data:
>>> handle: 0
>>> cookie: 0
>>> code: 0
>>> flag: TF_ACCEPT_FDS
>>> sender pid: 0
>>> sender euid: 0
>>> data_size: 24
>>> offsets_size: 8
>>> Retrieved Managers fd: 4 st_dev: 6
>>> Client read_consumed: 8
>>> Client using Managers FD command: BR_NOOP
>>> Client using Managers FD command: BR_FAILED_REPLY
>>> Client using Managers received FD failed response
>>> Manager read_consumed: 4
>>> Manager command: BR_NOOP
>>> not ok 1
>>> # Failed test at ./test line 36.
>>
>> Just realized that I was testing with a kernel that still had Casey's stacking support enabled.
>> Will re-try without that.
>
> Still fails for me on F28 with stock/linus 4.17.0-rc5. No AVC messages from the failing test itself,
> just the other ones.
Traced the client and saw that it was getting BR_FAILED_REPLY from the kernel.
Looked at /sys/kernel/debug/binder/failed_transaction_log and saw that the failure
occurs on line 2847. Did a git blame on that line and found this commit,
commit 7aa135fcf26377f92dc0680a57566b4c7f3e281b
Author: Martijn Coenen <maco@xxxxxxxxxxx>
Date: Wed Mar 28 11:14:50 2018 +0200
ANDROID: binder: prevent transactions into own process.
This can't happen with normal nodes (because you can't get a ref
to a node you own), but it could happen with the context manager;
to make the behavior consistent with regular nodes, reject
transactions into the context manager by the process owning it.
Reported-by: syzbot+09e05aba06723a94d43d@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Martijn Coenen <maco@xxxxxxxxxxx>
Cc: stable <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 764b63a5aade..e578eee31589 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2839,6 +2839,14 @@ static void binder_transaction(struct binder_proc *proc,
else
return_error = BR_DEAD_REPLY;
mutex_unlock(&context->context_mgr_node_lock);
+ if (target_node && target_proc == proc) {
+ binder_user_error("%d:%d got transaction to context manager from process owning it\n",
+ proc->pid, thread->pid);
+ return_error = BR_FAILED_REPLY;
+ return_error_param = -EINVAL;
+ return_error_line = __LINE__;
+ goto err_invalid_target_handle;
+ }
}
if (!target_node) {
/*
So that's a change in kernel behavior in v4.17-rc3 and later that breaks your test code.
