[PATCH] ANDROID: binder: prevent transactions into own process.

[PATCH] ANDROID: binder: prevent transactions into own process.

[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.

Reported-by: syzbot+09e05aba06723a94d43d@syzkaller.appspotmail.com
Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/android/binder.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index e7e4560e4c6e..57d4ba926ed0 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3001,6 +3001,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_node->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) {
 			/*
-- 
2.17.0.rc0.231.g781580f067-goog

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] ANDROID: binder: prevent transactions into own process.

[Greg KH]

On Wed, Mar 28, 2018 at 09:29:03AM +0200, Martijn Coenen wrote:
> 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@syzkaller.appspotmail.com
> Signed-off-by: Martijn Coenen <maco@android.com>

Does this need to go to older kernels as well?

I have a script that picks up everything the syzbot finds and tries to
backport them, after they are applied in Linus's tree.  Might as well
catch things before we have to rely on my script :)

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] ANDROID: binder: prevent transactions into own process.

[Martijn Coenen]

On Wed, Mar 28, 2018 at 10:19 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> Does this need to go to older kernels as well?

Yes, this should apply cleanly to 4.14 as well. It won't apply to
pre-4.14 kernels because of the fine-grained locking changes, but the
same issue exists there and I suspect it would cause the same crash.
Do you want me to send a separate patch for pre-4.14?

Also, I'm going to send a v2 for this because it can be a bit cleaner
(avoiding a deref).

Thanks,
Martijn

>
> I have a script that picks up everything the syzbot finds and tries to
> backport them, after they are applied in Linus's tree.  Might as well
> catch things before we have to rely on my script :)
>
> thanks,
>
> greg k-h

Re: [PATCH] ANDROID: binder: prevent transactions into own process.

[Greg KH]

On Wed, Mar 28, 2018 at 11:06:54AM +0200, Martijn Coenen wrote:
> On Wed, Mar 28, 2018 at 10:19 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > Does this need to go to older kernels as well?
> 
> Yes, this should apply cleanly to 4.14 as well. It won't apply to
> pre-4.14 kernels because of the fine-grained locking changes, but the
> same issue exists there and I suspect it would cause the same crash.
> Do you want me to send a separate patch for pre-4.14?

I can mark it for stable, and then when you get the "this did not apply
to this tree" email, you can send a backported patch to me so I know to
take that one then.

thanks,

greg k-h

Re: [PATCH] ANDROID: binder: prevent transactions into own process.

[Martijn Coenen]

On Wed, Mar 28, 2018 at 1:29 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> I can mark it for stable, and then when you get the "this did not apply
> to this tree" email, you can send a backported patch to me so I know to
> take that one then.

Ack, thanks.

>
> thanks,
>
> greg k-h