[PATCH] ANDROID: binder: remove 32-bit binder interface.

[PATCH] ANDROID: binder: remove 32-bit binder interface.

[Martijn Coenen]

New devices launching with Android P need to use the 64-bit
binder interface, even on 32-bit SoCs [0].

This change removes the Kconfig option to select the 32-bit
binder interface. We don't think this will affect existing
userspace for the following reasons:
1) The latest Android common tree is 4.14, so we don't
   believe any Android devices are on kernels >4.14.
2) Android devices launch on an LTS release and stick with
   it, so we wouldn't expect devices running on <= 4.14 now
   to upgrade to 4.17 or later. But even if they did, they'd
   rebuild the world (kernel + userspace) anyway.
3) Other userspaces like 'anbox' are already using the
   64-bit interface.

Note that this change doesn't remove the 32-bit UAPI
itself; the reason for that is that Android userspace
always uses the latest UAPI headers from upstream, and
userspace retains 32-bit support for devices that are
upgrading. This will be removed as well in 2-3 years,
at which point we can remove the code from the UAPI
as well.

[0]: https://android-review.googlesource.com/c/platform/build/+/595193

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/android/Kconfig  | 13 -------------
 drivers/android/binder.c |  4 ----
 2 files changed, 17 deletions(-)

diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
index 7dce3795b887..432e9ad77070 100644
--- a/drivers/android/Kconfig
+++ b/drivers/android/Kconfig
@@ -32,19 +32,6 @@ config ANDROID_BINDER_DEVICES
 	  created. Each binder device has its own context manager, and is
 	  therefore logically separated from the other devices.
 
-config ANDROID_BINDER_IPC_32BIT
-	bool "Use old (Android 4.4 and earlier) 32-bit binder API"
-	depends on !64BIT && ANDROID_BINDER_IPC
-	default y
-	---help---
-	  The Binder API has been changed to support both 32 and 64bit
-	  applications in a mixed environment.
-
-	  Enable this to support an old 32-bit Android user-space (v4.4 and
-	  earlier).
-
-	  Note that enabling this will break newer Android user-space.
-
 config ANDROID_BINDER_IPC_SELFTEST
 	bool "Android Binder IPC Driver Selftest"
 	depends on ANDROID_BINDER_IPC
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index e578eee31589..2ee9fb02dfb8 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -72,10 +72,6 @@
 #include <linux/security.h>
 #include <linux/spinlock.h>
 
-#ifdef CONFIG_ANDROID_BINDER_IPC_32BIT
-#define BINDER_IPC_32BIT 1
-#endif
-
 #include <uapi/linux/android/binder.h>
 #include "binder_alloc.h"
 #include "binder_trace.h"
-- 
2.17.0.441.gb46fe60e1d-goog

Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1262 bytes --]

Hi Martijn,

I love your patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.17-rc3 next-20180504]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Martijn-Coenen/ANDROID-binder-remove-32-bit-binder-interface/20180505-130632
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=m68k 

All errors (new ones prefixed by >>):

   drivers/android/binder.o: In function `binder_thread_write':
>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad'
   binder.c:(.text+0x6c9a): undefined reference to `__get_user_bad'
   binder.c:(.text+0x701e): undefined reference to `__get_user_bad'
   binder.c:(.text+0x7414): undefined reference to `__get_user_bad'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45127 bytes --]

Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.

[Martijn Coenen]

On Sat, May 5, 2018 at 2:10 PM, kbuild test robot <lkp@intel.com> wrote:
>    drivers/android/binder.o: In function `binder_thread_write':
>>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad'

Looks like m68k doesn't support 64-bit get_user(). I could just have
binder depend on !CONFIG_M68K, but there may be other architectures
still that don't support this. Another alternative would be to
whitelist the architectures Android supports - eg arm, arm64, x86,
x86_64. But I'm not sure if arch-limited drivers are considered bad
form. Does anybody have suggestions for how to deal with this?

Thanks,
Martijn

>    binder.c:(.text+0x6c9a): undefined reference to `__get_user_bad'
>    binder.c:(.text+0x701e): undefined reference to `__get_user_bad'
>    binder.c:(.text+0x7414): undefined reference to `__get_user_bad'
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.

[Greg KH]

On Fri, May 11, 2018 at 09:57:52AM +0200, Martijn Coenen wrote:
> On Sat, May 5, 2018 at 2:10 PM, kbuild test robot <lkp@intel.com> wrote:
> >    drivers/android/binder.o: In function `binder_thread_write':
> >>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad'
> 
> Looks like m68k doesn't support 64-bit get_user(). I could just have
> binder depend on !CONFIG_M68K, but there may be other architectures
> still that don't support this. Another alternative would be to
> whitelist the architectures Android supports - eg arm, arm64, x86,
> x86_64. But I'm not sure if arch-limited drivers are considered bad
> form. Does anybody have suggestions for how to deal with this?

I think using !CONFIG_M68K is a good start.  We can blacklist any other
arch that doesn't support this, and that list should be small as I doubt
any new ones will be added without this support.

thanks,

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

Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.

[Martijn Coenen]

On Fri, May 11, 2018 at 10:08 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> I think using !CONFIG_M68K is a good start.  We can blacklist any other
> arch that doesn't support this, and that list should be small as I doubt
> any new ones will be added without this support.

Thanks, I will send a v2.

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

Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.

[Christoph Hellwig]

On Fri, May 11, 2018 at 09:57:52AM +0200, Martijn Coenen wrote:
> On Sat, May 5, 2018 at 2:10 PM, kbuild test robot <lkp@intel.com> wrote:
> >    drivers/android/binder.o: In function `binder_thread_write':
> >>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad'
> 
> Looks like m68k doesn't support 64-bit get_user(). I could just have
> binder depend on !CONFIG_M68K, but there may be other architectures
> still that don't support this. Another alternative would be to
> whitelist the architectures Android supports - eg arm, arm64, x86,
> x86_64. But I'm not sure if arch-limited drivers are considered bad
> form. Does anybody have suggestions for how to deal with this?

The proper fix is to just support 640bit get/put_user on m68k instead
of working around this.

Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.

[Geert Uytterhoeven]

Hi Christoph,

On Mon, May 14, 2018 at 2:03 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Fri, May 11, 2018 at 09:57:52AM +0200, Martijn Coenen wrote:
>> On Sat, May 5, 2018 at 2:10 PM, kbuild test robot <lkp@intel.com> wrote:
>> >    drivers/android/binder.o: In function `binder_thread_write':
>> >>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad'
>>
>> Looks like m68k doesn't support 64-bit get_user(). I could just have
>> binder depend on !CONFIG_M68K, but there may be other architectures
>> still that don't support this. Another alternative would be to
>> whitelist the architectures Android supports - eg arm, arm64, x86,
>> x86_64. But I'm not sure if arch-limited drivers are considered bad
>> form. Does anybody have suggestions for how to deal with this?
>
> The proper fix is to just support 640bit get/put_user on m68k instead

I hope we'll never need 640bit support in {get,put}_user() ;-)

> of working around this.

Patch sent.

BTW, sh also doesn't seem to have 64-bit get_user().
There may be others.

BTW2, does the Android Binder need to care about endianness when talking
to userspace?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.

[Martijn Coenen]

On Mon, May 14, 2018 at 4:00 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Patch sent.

Thanks for the quick turn-around!

>
> BTW, sh also doesn't seem to have 64-bit get_user().
> There may be others.

I checked quickly, nios2 is the only other arch that explicitly
doesn't support it and would result in a build error; some other archs
don't define __get_user, but in that case they just fall back to
raw_copy_from_user().

>
> BTW2, does the Android Binder need to care about endianness when talking
> to userspace?

No, I don't think it should.

Thanks,
Martijn

>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel