ANDROID: binder: reconcile with android common tree

PATCH In review Martijn Coenen 4 participants 13 messages Jul 28, 2017 – Aug 23, 2017 Android drivers

lore

Share & tools

lore

Martijn Coenen

author

2017-07-28 11:56 UTC #0 lore raw

cover 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(-)

Signature · 1 line

2.14.0.rc0.400.g1c36432dff-goog

Martijn Coenen

author

2017-07-28 11:58 UTC #1 lore raw

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(+)

include/uapi/linux/android/binder.h +2 -0

index 51f891fb1b18..7668b5791c91 100644
@@ -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;

Signature · 1 line

2.14.0.rc0.400.g1c36432dff-goog

Martijn Coenen

author

2017-07-28 11:56 UTC #2 lore raw

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

In reply to Martijn Coenen

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(-)

drivers/android/Kconfig +1 -1

index 832e885349b1..4d4cdc1a6e25 100644
@@ -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.

kernel/configs/android-base.config +1 -0

index d70829033bb7..d3fd428f4b92 100644
@@ -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

Signature · 1 line

2.14.0.rc0.400.g1c36432dff-goog

Martijn Coenen

author

2017-07-28 11:58 UTC #3 lore raw

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

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

---
 drivers/android/binder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/android/binder.c +1 -1

index f7665c31feca..831cdd7d197d 100644
@@ -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)

Signature · 1 line

2.14.0.rc0.400.g1c36432dff-goog

Amit Pundir

Tested-by on v1 3/3

2017-07-31 05:25 UTC #4 lore raw

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

Quoted 4 lines · 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.

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

Tested-by Amit Pundir <[email protected]>

Quoted 21 lines

Signed-off-by Martijn Coenen <[email protected]>

---
 drivers/android/binder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/android/binder.c +1 -2

index f7665c31feca..831cdd7d197d 100644
@@ -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

John Stultz

Tested-by on v1 3/3

2017-08-08 17:34 UTC #5 lore raw

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

In reply to Martijn Coenen

Quoted 14 lines · 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 <[email protected]>
---
 drivers/android/binder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/android/binder.c +0 -0
index f7665c31feca..831cdd7d197d 100644

drivers/android/binder.c static int binder_mmap(struct file *filp, struct vm_area_struct *vma) · line 3368

Quoted 8 lines

drivers/android/binder.c +1 -1

@@ -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

Greg KH

maintainer

2017-08-08 18:08 UTC #6 lore raw

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

Quoted 26 lines · John Stultz

Quoted 14 lines · 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 <[email protected]>
---
 drivers/android/binder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/android/binder.c +0 -0
index f7665c31feca..831cdd7d197d 100644
drivers/android/binder.c static int binder_mmap(struct file *filp, struct vm_area_struct *vma) · line 3368
Quoted 8 lines
drivers/android/binder.c +1 -1
@@ -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]>

Quoted 6 lines

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

John Stultz

2017-08-08 18:24 UTC #7 lore raw

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

Quoted 30 lines · Greg KH

Quoted 29 lines · John Stultz
Quoted 14 lines · 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 <[email protected]>
---
 drivers/android/binder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/android/binder.c +0 -0
index f7665c31feca..831cdd7d197d 100644
drivers/android/binder.c static int binder_mmap(struct file *filp, struct vm_area_struct *vma) · line 3368
Quoted 8 lines
drivers/android/binder.c +1 -1
@@ -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.

Quoted 6 lines

Quoted 3 lines
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

John Stultz

2017-08-21 18:48 UTC #8 lore raw

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

In reply to Greg KH

Quoted 30 lines · Greg KH

Quoted 29 lines · John Stultz
Quoted 14 lines · 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 <[email protected]>
---
 drivers/android/binder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/android/binder.c +0 -0
index f7665c31feca..831cdd7d197d 100644
drivers/android/binder.c static int binder_mmap(struct file *filp, struct vm_area_struct *vma) · line 3368
Quoted 8 lines
drivers/android/binder.c +1 -1
@@ -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.

Quoted 6 lines

Quoted 3 lines
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

Greg KH

maintainer

2017-08-22 15:49 UTC #9 lore raw

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

Quoted 35 lines · John Stultz

Quoted 34 lines · Greg KH
Quoted 32 lines · John Stultz
Quoted 14 lines · 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 <[email protected]>
---
 drivers/android/binder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/android/binder.c +0 -0
index f7665c31feca..831cdd7d197d 100644
drivers/android/binder.c static int binder_mmap(struct file *filp, struct vm_area_struct *vma) · line 3368
Quoted 8 lines
drivers/android/binder.c +1 -1
@@ -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).

Quoted 6 lines

Quoted 2 lines
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

Greg KH

maintainer

2017-08-23 01:43 UTC #10 lore raw

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

Quoted 39 lines · Greg KH

Quoted 38 lines · John Stultz
Quoted 36 lines · Greg KH
Quoted 26 lines · John Stultz
Quoted 14 lines · 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 <[email protected]>
---
 drivers/android/binder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/android/binder.c +0 -0
index f7665c31feca..831cdd7d197d 100644
drivers/android/binder.c static int binder_mmap(struct file *filp, struct vm_area_struct *vma) · line 3368
Quoted 8 lines
drivers/android/binder.c +1 -1
@@ -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]>
Quoted 6 lines
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.

Quoted 6 lines

Quoted 3 lines
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

Greg KH

maintainer

2017-07-28 23:48 UTC #11 lore raw

In reply to Martijn Coenen

Quoted 9 lines · 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.

Quoted 7 lines

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

Martijn Coenen

author

2017-07-31 06:55 UTC #12 lore raw

Quoted 3 lines · Greg KH

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

Quoted 4 lines

thanks,
greg k-h