linux-kernel - Re: [PATCH] android: binder: fix type mismatch warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAB0TPYFkm2G7cgtuLDfANsLxkAz0Dx7wmW+mE=Dag2Kbk-DxpA@mail.gmail.com>
Date:   Wed, 20 Sep 2017 12:24:45 +0200
From:   Martijn Coenen <maco@...roid.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Christoph Hellwig <hch@...radead.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Arve Hjønnevåg <arve@...roid.com>,
        Riley Andrews <riandrews@...roid.com>,
        devel@...verdev.osuosl.org, Todd Kjos <tkjos@...roid.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        John Stultz <john.stultz@...aro.org>
Subject: Re: [PATCH] android: binder: fix type mismatch warning

On Wed, Sep 20, 2017 at 11:58 AM, Arnd Bergmann <arnd@...db.de> wrote:
> - On stable mainline kernels (unlike android-common), the v8
>   interface has never been available as a build option, and making
>   it user-selectable will required additional patches to make it
>   actually build on 32-bit ARM. This is fixable if Greg is willing
>   to take the backports into stable kernels

Yeah that would need to be fixed indeed.

>
> - Having a compile-time option to pick between two incompatible
>   ABIs is a really bad idea, we don't do that anywhere else in
>   the kernel except for things like the choice between big-endian
>   and little-endian kernels that can't be done otherwise. If we
>   want to keep supporting both ABIs going forward, I think it
>   needs to be a runtime option instead, to allow users to migrate
>   between kernel versions.

I definitely don't want to keep it going forward, just maintain the
option for older kernels.

>
> - Since you say there are existing users of recent 32-bit Android
>   including Oreo, I also think that removing support for the v7 ABI
>   is no longer an option. I only made that suggestion under the
>   assumption that there is no user space requiring that. Linux
>   has no real way of "deprecating" an existing ABI, either it is
>   currently used and has to stay around, or it is not used at all
>   and can be removed without anybody noticing.

I don't know of any Android devices shipping with 4.14 - we don't even
have a common tree for 4.14 (4.9 is the latest). So I don't think
we're hurting anyone in the Android ecosystem if we remove v7 from
4.14. From what I know, it's extremely rare for an Android device to
change their kernel version after a device ships, but even if someone
hypothetically did update their Android device to use 4.14, they can
pretty easily switch the build-time config option. I understand that
this is against Linux' philosophy around not breaking userspace, I
just want to point out that I think from an Android POV, removing v7
from 4.14 is not an issue. I'm not sure if that is good enough.

> If we end up doing a runtime switch between the ABIs instead,
> I see two ways of doing that:
>
> The easiest implementation would make it a module parameter:
> Since there is only one instance of the binder in any given
> system, and presumably all processes talking to it have to use
> the same ABI, this should be sufficient. The main downside is
> that it prevents us from having incompatible versions per
> namespace if we ever get a containerized version of binder.

Namespace support for binder is currently being investigated, but I'm
not sure we'd ever have a system where we want to mix v7 and v8. There
really is no good reason to use v7 anymore (even on 32-bit mahcines),
we just should have removed it from our userspace a lot earlier.

>
> The correct way to do it would be to unify the two versions of
> the ABI so they can be used interchangeably: any 32-bit
> process would start out with the v7 interface and a 64-bit
> process would start out with the v8 interface

This wouldn't work with the current implementation - if a 32-bit and
64-bit process communicate, then userspace would make wrong
assumptions about the sizes of transactions. Or is this what you're
proposing to fix?

> and any new
> version of the 32-bit binder user space would issue a special
> ioctl to switch to the v8 version. If we ever get a v9 version
> of the ABI, that would then add another set of ioctl commands
> with different numbers that don't conflict with the v8 interface
> but are implemented by the same kernel module.
> Actually implementing the combined v7/v8 ioctl stuff is
> of course nontrivial.

Yes, this would be a lot of work. I'll let others chime in, but I
would still prefer to remove v7 from 4.14 onward altogether.

Thanks,
Martijn