Skip to contentSkip to conversation

ANDROID: binder: prevent transactions into own process.

PATCH In review Martijn Coenen 2 participants 5 messages Mar 28, 2018 – Apr 23, 2018 Android drivers

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 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.

---
 drivers/android/binder.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/android/binder.c +8 -0
index 764b63a5aade..e578eee31589 100644
@@ -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) {
 			/*
Signature · 6 lines
2.17.0.rc1.321.gba9d0f2565-goog

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
maintainer
#1 lore raw
Quoted 10 lines · Martijn Coenen

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.

---
 drivers/android/binder.c | 8 ++++++++
 1 file changed, 8 insertions(+)

What is different from "v2" you sent before this? No change information from v1?

I'm totally confused as to which is the "latest" patch here :(

thanks,

greg k-h-

#2 lore raw
Quoted 2 lines · Greg KH

What is different from "v2" you sent before this? No change information from v1?

Sorry I messed this up - the first resend did not have "v2" in the subject but did contain v2 change information, the second resend had the v2 subject and did not contain the change information :( Either of the last two patches is fine - it just used "target_proc" instead of "target_node->proc".

Quoted 6 lines

I'm totally confused as to which is the "latest" patch here :(

thanks,

greg k-h-

#3 lore raw
Quoted 3 lines · Martijn Coenen
Quoted 2 lines · Greg KH

What is different from "v2" you sent before this? No change information from v1?

Greg, is this in your queue, or should I just send a v3 to clean this up? See below for my original reply.

Thanks,
Martijn

Quoted 13 lines

Sorry I messed this up - the first resend did not have "v2" in the subject but did contain v2 change information, the second resend had the v2 subject and did not contain the change information :( Either of the last two patches is fine - it just used "target_proc" instead of "target_node->proc".

Quoted 6 lines

I'm totally confused as to which is the "latest" patch here :(

thanks,

greg k-h-

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

maintainer
#4 lore raw
Quoted 7 lines · Martijn Coenen
Quoted 3 lines · Martijn Coenen
Quoted 2 lines · Greg KH

What is different from "v2" you sent before this? No change information from v1?

Greg, is this in your queue, or should I just send a v3 to clean this up? See below for my original reply.

It's in my queue, I couldn't do anything during the merge window and I was gone all last week, so am catching up on stuff now. Should get to it by the end of this week, sorry for the delay.

thanks,

greg k-h