[PATCH 0/3] ANDROID: binder: reconcile with android common tree

[PATCH 0/3] ANDROID: binder: reconcile with android common tree

[Martijn Coenen]

When comparing the android common kernel branch with
upstream, I found several differences.

The "add padding" patch has long been applied in common,
and shipping versions of Android userspace depends on this
particular alignment; so while it does change UAPI, we
have never shipped a userspace that used the old UAPI,
so this change does not break anything.

The "add hwbinder,vndbinder" patch changes the default
binder device nodes to match what the latest Android
userspace (O) requires.

Finally, the recent patch-stack for fine-grained locking
contained a patch that for some reason was incomplete;
"fix proc->tsk check" addresses this particular problem.

Martijn Coenen (3):
  ANDROID: binder: add padding to binder_fd_array_object.
  ANDROID: binder: add hwbinder,vndbinder to BINDER_DEVICES.
  ANDROID: binder: fix proc->tsk check.

 drivers/android/Kconfig             | 2 +-
 drivers/android/binder.c            | 2 +-
 include/uapi/linux/android/binder.h | 2 ++
 kernel/configs/android-base.config  | 1 +
 4 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.14.0.rc0.400.g1c36432dff-goog

[PATCH 1/3] ANDROID: binder: add padding to binder_fd_array_object.

[Martijn Coenen]

binder_fd_array_object starts with a 4-byte header,
followed by a few fields that are 8 bytes when
ANDROID_BINDER_IPC_32BIT=N.

This can cause alignment issues in a 64-bit kernel
with a 32-bit userspace, as on x86_32 an 8-byte primitive
may be aligned to a 4-byte address. Pad with a __u32
to fix this.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 include/uapi/linux/android/binder.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h
index 51f891fb1b18..7668b5791c91 100644
--- a/include/uapi/linux/android/binder.h
+++ b/include/uapi/linux/android/binder.h
@@ -132,6 +132,7 @@ enum {
 
 /* struct binder_fd_array_object - object describing an array of fds in a buffer
  * @hdr:		common header structure
+ * @pad:		padding to ensure correct alignment
  * @num_fds:		number of file descriptors in the buffer
  * @parent:		index in offset array to buffer holding the fd array
  * @parent_offset:	start offset of fd array in the buffer
@@ -152,6 +153,7 @@ enum {
  */
 struct binder_fd_array_object {
 	struct binder_object_header	hdr;
+	__u32				pad;
 	binder_size_t			num_fds;
 	binder_size_t			parent;
 	binder_size_t			parent_offset;
-- 
2.14.0.rc0.400.g1c36432dff-goog

[PATCH 2/3] ANDROID: binder: add hwbinder,vndbinder to BINDER_DEVICES.

[Martijn Coenen]

These will be required going forward.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/android/Kconfig            | 2 +-
 kernel/configs/android-base.config | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
index 832e885349b1..4d4cdc1a6e25 100644
--- a/drivers/android/Kconfig
+++ b/drivers/android/Kconfig
@@ -22,7 +22,7 @@ config ANDROID_BINDER_IPC
 config ANDROID_BINDER_DEVICES
 	string "Android Binder devices"
 	depends on ANDROID_BINDER_IPC
-	default "binder,hwbinder"
+	default "binder,hwbinder,vndbinder"
 	---help---
 	  Default value for the binder.devices parameter.
 
diff --git a/kernel/configs/android-base.config b/kernel/configs/android-base.config
index d70829033bb7..d3fd428f4b92 100644
--- a/kernel/configs/android-base.config
+++ b/kernel/configs/android-base.config
@@ -10,6 +10,7 @@
 # CONFIG_USELIB is not set
 CONFIG_ANDROID=y
 CONFIG_ANDROID_BINDER_IPC=y
+CONFIG_ANDROID_BINDER_DEVICES=binder,hwbinder,vndbinder
 CONFIG_ANDROID_LOW_MEMORY_KILLER=y
 CONFIG_ARMV8_DEPRECATED=y
 CONFIG_ASHMEM=y
-- 
2.14.0.rc0.400.g1c36432dff-goog

[PATCH 3/3] ANDROID: binder: fix proc->tsk check.

[Martijn Coenen]

Commit c4ea41ba195d ("binder: use group leader instead of open thread")'
was incomplete and didn't update a check in binder_mmap(), causing all
mmap() calls into the binder driver to fail.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/android/binder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index f7665c31feca..831cdd7d197d 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3362,7 +3362,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
 	const char *failure_string;
 	struct binder_buffer *buffer;
 
-	if (proc->tsk != current)
+	if (proc->tsk != current->group_leader)
 		return -EINVAL;
 
 	if ((vma->vm_end - vma->vm_start) > SZ_4M)
-- 
2.14.0.rc0.400.g1c36432dff-goog

Re: [PATCH 0/3] ANDROID: binder: reconcile with android common tree

[Greg KH]

On Fri, Jul 28, 2017 at 01:56:05PM +0200, Martijn Coenen wrote:
> When comparing the android common kernel branch with
> upstream, I found several differences.
> 
> The "add padding" patch has long been applied in common,
> and shipping versions of Android userspace depends on this
> particular alignment; so while it does change UAPI, we
> have never shipped a userspace that used the old UAPI,
> so this change does not break anything.
> 
> The "add hwbinder,vndbinder" patch changes the default
> binder device nodes to match what the latest Android
> userspace (O) requires.
> 
> Finally, the recent patch-stack for fine-grained locking
> contained a patch that for some reason was incomplete;
> "fix proc->tsk check" addresses this particular problem.

Ok, do some of these need to go to Linus now for 4.13-final and to the
stable trees to match up with the 3 that are already proposed for the
stable trees?  If so, which ones?

thanks,

greg k-h

Re: [PATCH 3/3] ANDROID: binder: fix proc->tsk check.

[Amit Pundir]

On 28 July 2017 at 17:26, Martijn Coenen <maco@android.com> wrote:
> Commit c4ea41ba195d ("binder: use group leader instead of open thread")'
> was incomplete and didn't update a check in binder_mmap(), causing all
> mmap() calls into the binder driver to fail.
>

Fixes Android WiFi/BT regression reported on 4.13-rc2.

Tested-by: Amit Pundir <amit.pundir@linaro.org>

> Signed-off-by: Martijn Coenen <maco@android.com>
> ---
>  drivers/android/binder.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index f7665c31feca..831cdd7d197d 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -3362,7 +3362,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
>         const char *failure_string;
>         struct binder_buffer *buffer;
>
> -       if (proc->tsk != current)
> +       if (proc->tsk != current->group_leader)
>                 return -EINVAL;
>
>         if ((vma->vm_end - vma->vm_start) > SZ_4M)
> --
> 2.14.0.rc0.400.g1c36432dff-goog
>

Re: [PATCH 0/3] ANDROID: binder: reconcile with android common tree

[Martijn Coenen]

On Sat, Jul 29, 2017 at 1:22 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> Ok, do some of these need to go to Linus now for 4.13-final and to the
> stable trees to match up with the 3 that are already proposed for the
> stable trees?  If so, which ones?

"fix proc->tsk check" is a fix for "c4ea41ba195d ("binder: use group
leader instead of open thread") which Todd sent to LKML just two weeks
ago; it should definitely go to 4.13-final if it still can, and
basically all the -stable kernels that patch ended up in (I think Todd
proposed back to 4.4).

The other two commits are related to the multiple domains /
scatter-gather patches that were upstreamed about 6 months ago (eg
7980240b6d63e); I saw those landed in 4.11-rc1 first.

Thanks,
Martijn

>
> thanks,
>
> greg k-h

Re: [PATCH 3/3] ANDROID: binder: fix proc->tsk check.

[John Stultz]

On Fri, Jul 28, 2017 at 4:56 AM, Martijn Coenen <maco@android.com> wrote:
> Commit c4ea41ba195d ("binder: use group leader instead of open thread")'
> was incomplete and didn't update a check in binder_mmap(), causing all
> mmap() calls into the binder driver to fail.
>
> Signed-off-by: Martijn Coenen <maco@android.com>
> ---
>  drivers/android/binder.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index f7665c31feca..831cdd7d197d 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -3362,7 +3362,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
>         const char *failure_string;
>         struct binder_buffer *buffer;
>
> -       if (proc->tsk != current)
> +       if (proc->tsk != current->group_leader)
>                 return -EINVAL;
>
>         if ((vma->vm_end - vma->vm_start) > SZ_4M)

Tested-by: John Stultz <john.stultz@linaro.org>

As Amit already confirmed, this resolves the wifi and bluetooth
regression I was seeing with Android using 4.13-rc2.

Though I've not seen it show up in Linus' tree yet, so I wanted to
pester folks so it gets into 4.13-rc (its given me some slight grief
trying to bisect down a separate issue).

thanks
-john

Re: [PATCH 3/3] ANDROID: binder: fix proc->tsk check.

[Greg KH]

On Tue, Aug 08, 2017 at 10:34:47AM -0700, John Stultz wrote:
> On Fri, Jul 28, 2017 at 4:56 AM, Martijn Coenen <maco@android.com> wrote:
> > Commit c4ea41ba195d ("binder: use group leader instead of open thread")'
> > was incomplete and didn't update a check in binder_mmap(), causing all
> > mmap() calls into the binder driver to fail.
> >
> > Signed-off-by: Martijn Coenen <maco@android.com>
> > ---
> >  drivers/android/binder.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > index f7665c31feca..831cdd7d197d 100644
> > --- a/drivers/android/binder.c
> > +++ b/drivers/android/binder.c
> > @@ -3362,7 +3362,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
> >         const char *failure_string;
> >         struct binder_buffer *buffer;
> >
> > -       if (proc->tsk != current)
> > +       if (proc->tsk != current->group_leader)
> >                 return -EINVAL;
> >
> >         if ((vma->vm_end - vma->vm_start) > SZ_4M)
> 
> Tested-by: John Stultz <john.stultz@linaro.org>
> 
> As Amit already confirmed, this resolves the wifi and bluetooth
> regression I was seeing with Android using 4.13-rc2.
> 
> Though I've not seen it show up in Linus' tree yet, so I wanted to
> pester folks so it gets into 4.13-rc (its given me some slight grief
> trying to bisect down a separate issue).

I will queue this up in the next few days, I need to resolve the patches
that have been sent to me for this, sorry for the delay.

thanks,

greg k-h

Re: [PATCH 3/3] ANDROID: binder: fix proc->tsk check.

[John Stultz]

On Tue, Aug 8, 2017 at 11:08 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Aug 08, 2017 at 10:34:47AM -0700, John Stultz wrote:
>> On Fri, Jul 28, 2017 at 4:56 AM, Martijn Coenen <maco@android.com> wrote:
>> > Commit c4ea41ba195d ("binder: use group leader instead of open thread")'
>> > was incomplete and didn't update a check in binder_mmap(), causing all
>> > mmap() calls into the binder driver to fail.
>> >
>> > Signed-off-by: Martijn Coenen <maco@android.com>
>> > ---
>> >  drivers/android/binder.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
>> > index f7665c31feca..831cdd7d197d 100644
>> > --- a/drivers/android/binder.c
>> > +++ b/drivers/android/binder.c
>> > @@ -3362,7 +3362,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
>> >         const char *failure_string;
>> >         struct binder_buffer *buffer;
>> >
>> > -       if (proc->tsk != current)
>> > +       if (proc->tsk != current->group_leader)
>> >                 return -EINVAL;
>> >
>> >         if ((vma->vm_end - vma->vm_start) > SZ_4M)
>>
>> Tested-by: John Stultz <john.stultz@linaro.org>
>>
>> As Amit already confirmed, this resolves the wifi and bluetooth
>> regression I was seeing with Android using 4.13-rc2.
>>
>> Though I've not seen it show up in Linus' tree yet, so I wanted to
>> pester folks so it gets into 4.13-rc (its given me some slight grief
>> trying to bisect down a separate issue).
>
> I will queue this up in the next few days, I need to resolve the patches
> that have been sent to me for this, sorry for the delay.

No worries. Just wanted to make sure it hadn't gotten lost.

thanks
-john

Re: [PATCH 3/3] ANDROID: binder: fix proc->tsk check.

[John Stultz]

On Tue, Aug 8, 2017 at 11:08 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Aug 08, 2017 at 10:34:47AM -0700, John Stultz wrote:
>> On Fri, Jul 28, 2017 at 4:56 AM, Martijn Coenen <maco@android.com> wrote:
>> > Commit c4ea41ba195d ("binder: use group leader instead of open thread")'
>> > was incomplete and didn't update a check in binder_mmap(), causing all
>> > mmap() calls into the binder driver to fail.
>> >
>> > Signed-off-by: Martijn Coenen <maco@android.com>
>> > ---
>> >  drivers/android/binder.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
>> > index f7665c31feca..831cdd7d197d 100644
>> > --- a/drivers/android/binder.c
>> > +++ b/drivers/android/binder.c
>> > @@ -3362,7 +3362,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
>> >         const char *failure_string;
>> >         struct binder_buffer *buffer;
>> >
>> > -       if (proc->tsk != current)
>> > +       if (proc->tsk != current->group_leader)
>> >                 return -EINVAL;
>> >
>> >         if ((vma->vm_end - vma->vm_start) > SZ_4M)
>>
>> Tested-by: John Stultz <john.stultz@linaro.org>
>>
>> As Amit already confirmed, this resolves the wifi and bluetooth
>> regression I was seeing with Android using 4.13-rc2.
>>
>> Though I've not seen it show up in Linus' tree yet, so I wanted to
>> pester folks so it gets into 4.13-rc (its given me some slight grief
>> trying to bisect down a separate issue).
>
> I will queue this up in the next few days, I need to resolve the patches
> that have been sent to me for this, sorry for the delay.

Hey Greg,
  Sorry to pester you again on this, but this seems to still be
missing in -rc6. Don't want it to slip by.

thanks
-john

Re: [PATCH 3/3] ANDROID: binder: fix proc->tsk check.

[Greg KH]

On Mon, Aug 21, 2017 at 11:48:39AM -0700, John Stultz wrote:
> On Tue, Aug 8, 2017 at 11:08 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Aug 08, 2017 at 10:34:47AM -0700, John Stultz wrote:
> >> On Fri, Jul 28, 2017 at 4:56 AM, Martijn Coenen <maco@android.com> wrote:
> >> > Commit c4ea41ba195d ("binder: use group leader instead of open thread")'
> >> > was incomplete and didn't update a check in binder_mmap(), causing all
> >> > mmap() calls into the binder driver to fail.
> >> >
> >> > Signed-off-by: Martijn Coenen <maco@android.com>
> >> > ---
> >> >  drivers/android/binder.c | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> >> > index f7665c31feca..831cdd7d197d 100644
> >> > --- a/drivers/android/binder.c
> >> > +++ b/drivers/android/binder.c
> >> > @@ -3362,7 +3362,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
> >> >         const char *failure_string;
> >> >         struct binder_buffer *buffer;
> >> >
> >> > -       if (proc->tsk != current)
> >> > +       if (proc->tsk != current->group_leader)
> >> >                 return -EINVAL;
> >> >
> >> >         if ((vma->vm_end - vma->vm_start) > SZ_4M)
> >>
> >> Tested-by: John Stultz <john.stultz@linaro.org>
> >>
> >> As Amit already confirmed, this resolves the wifi and bluetooth
> >> regression I was seeing with Android using 4.13-rc2.
> >>
> >> Though I've not seen it show up in Linus' tree yet, so I wanted to
> >> pester folks so it gets into 4.13-rc (its given me some slight grief
> >> trying to bisect down a separate issue).
> >
> > I will queue this up in the next few days, I need to resolve the patches
> > that have been sent to me for this, sorry for the delay.
> 
> Hey Greg,
>   Sorry to pester you again on this, but this seems to still be
> missing in -rc6. Don't want it to slip by.

It's not lost, just on the bottom of my queue, will catch up with it
next week...

thanks,

greg k-h

Re: [PATCH 3/3] ANDROID: binder: fix proc->tsk check.

[Greg KH]

On Tue, Aug 22, 2017 at 08:49:59AM -0700, Greg KH wrote:
> On Mon, Aug 21, 2017 at 11:48:39AM -0700, John Stultz wrote:
> > On Tue, Aug 8, 2017 at 11:08 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > On Tue, Aug 08, 2017 at 10:34:47AM -0700, John Stultz wrote:
> > >> On Fri, Jul 28, 2017 at 4:56 AM, Martijn Coenen <maco@android.com> wrote:
> > >> > Commit c4ea41ba195d ("binder: use group leader instead of open thread")'
> > >> > was incomplete and didn't update a check in binder_mmap(), causing all
> > >> > mmap() calls into the binder driver to fail.
> > >> >
> > >> > Signed-off-by: Martijn Coenen <maco@android.com>
> > >> > ---
> > >> >  drivers/android/binder.c | 2 +-
> > >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >> >
> > >> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > >> > index f7665c31feca..831cdd7d197d 100644
> > >> > --- a/drivers/android/binder.c
> > >> > +++ b/drivers/android/binder.c
> > >> > @@ -3362,7 +3362,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
> > >> >         const char *failure_string;
> > >> >         struct binder_buffer *buffer;
> > >> >
> > >> > -       if (proc->tsk != current)
> > >> > +       if (proc->tsk != current->group_leader)
> > >> >                 return -EINVAL;
> > >> >
> > >> >         if ((vma->vm_end - vma->vm_start) > SZ_4M)
> > >>
> > >> Tested-by: John Stultz <john.stultz@linaro.org>
> > >>
> > >> As Amit already confirmed, this resolves the wifi and bluetooth
> > >> regression I was seeing with Android using 4.13-rc2.
> > >>
> > >> Though I've not seen it show up in Linus' tree yet, so I wanted to
> > >> pester folks so it gets into 4.13-rc (its given me some slight grief
> > >> trying to bisect down a separate issue).
> > >
> > > I will queue this up in the next few days, I need to resolve the patches
> > > that have been sent to me for this, sorry for the delay.
> > 
> > Hey Greg,
> >   Sorry to pester you again on this, but this seems to still be
> > missing in -rc6. Don't want it to slip by.
> 
> It's not lost, just on the bottom of my queue, will catch up with it
> next week...

Now queued up, sorry for the delay.

greg k-h