2017-07-28 11:56:17

by Martijn Coenen

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

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


2017-07-28 11:56:20

by Martijn Coenen

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

These will be required going forward.

Signed-off-by: Martijn Coenen <[email protected]>
---
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

2017-07-28 11:58:03

by Martijn Coenen

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

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 <[email protected]>
---
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

2017-07-28 11:58:21

by Martijn Coenen

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

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 <[email protected]>
---
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

2017-07-28 23:48:37

by Greg Kroah-Hartman

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

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

2017-07-31 05:25:43

by Amit Pundir

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

On 28 July 2017 at 17:26, Martijn Coenen <[email protected]> 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 <[email protected]>

> Signed-off-by: Martijn Coenen <[email protected]>
> ---
> 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
>

2017-07-31 06:55:19

by Martijn Coenen

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

On Sat, Jul 29, 2017 at 1:22 AM, Greg KH <[email protected]> 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

2017-08-08 17:34:49

by John Stultz

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

On Fri, Jul 28, 2017 at 4:56 AM, Martijn Coenen <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>

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

2017-08-08 18:08:05

by Greg Kroah-Hartman

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

On Tue, Aug 08, 2017 at 10:34:47AM -0700, John Stultz wrote:
> On Fri, Jul 28, 2017 at 4:56 AM, Martijn Coenen <[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
>
> 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

2017-08-08 18:24:24

by John Stultz

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

On Tue, Aug 8, 2017 at 11:08 AM, Greg KH <[email protected]> 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 <[email protected]> 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 <[email protected]>
>> > ---
>> > 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 <[email protected]>
>>
>> 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

2017-08-21 18:48:42

by John Stultz

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

On Tue, Aug 8, 2017 at 11:08 AM, Greg KH <[email protected]> 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 <[email protected]> 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 <[email protected]>
>> > ---
>> > 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 <[email protected]>
>>
>> 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

2017-08-22 15:49:56

by Greg Kroah-Hartman

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

On Mon, Aug 21, 2017 at 11:48:39AM -0700, John Stultz wrote:
> On Tue, Aug 8, 2017 at 11:08 AM, Greg KH <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >> > ---
> >> > 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 <[email protected]>
> >>
> >> 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

2017-08-23 01:43:36

by Greg Kroah-Hartman

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

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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > >> > ---
> > >> > 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 <[email protected]>
> > >>
> > >> 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