linux-kernel - Re: [PATCH v1 9/9] staging: android: binder: Add binder compat layer

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: <CAMbhsRQnY6+Ahs-itSeALNQ27tsWAJT8wUtXNGZE--44URp2-A@mail.gmail.com>
Date:	Wed, 4 Dec 2013 14:22:13 -0800
From:	Colin Cross <ccross@...roid.com>
To:	Greg KH <gregkh@...uxfoundation.org>
Cc:	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	Ian Rogers <irogers@...gle.com>,
	Serban Constantinescu <serban.constantinescu@....com>,
	lkml <linux-kernel@...r.kernel.org>,
	Arve Hjønnevåg <arve@...roid.com>,
	John Stultz <john.stultz@...aro.org>,
	David Butcher <Dave.Butcher@....com>, romlem@...roid.com
Subject: Re: [PATCH v1 9/9] staging: android: binder: Add binder compat layer

On Wed, Dec 4, 2013 at 2:02 PM, Greg KH <gregkh@...uxfoundation.org> wrote:
> On Wed, Dec 04, 2013 at 01:55:34PM -0800, Colin Cross wrote:
>> On Wed, Dec 4, 2013 at 1:43 PM, Greg KH <gregkh@...uxfoundation.org> wrote:
>> > On Wed, Dec 04, 2013 at 12:46:42PM -0800, Colin Cross wrote:
>> >> On Wed, Dec 4, 2013 at 10:35 AM, Greg KH <gregkh@...uxfoundation.org> wrote:
>> >> <snip>
>> >>
>> >> > And finally, is this all really needed?  Why not just fix the structures
>> >> > to be "correct", and then fix userspace to use the correct structures as
>> >> > well, thereby not needing a compat layer at all?
>> >>
>> >> Some of the binder ioctls take userspace pointers.  Are you suggesting
>> >> storing those pointers in a __u64 to avoid having to have a
>> >> compat_ioctl?
>> >
>> > Yes, that's the best way to solve the issue, right?
>>
>> It's the least code, but in exchange you lose all the type safety and
>> warnings when copying in and out of the pointers, as well as sparse
>> checking on the __user attribute.
>
> Not if you make the cast right at the beginning, when you first "touch"
> the data, but yes, it does take some of the type saftey away, at the
> expense of simpler code to mess up :)
>
>> That doesn't seem like a good tradeoff to me.  In addition it requires
>> modifying the existing heavily used 32 bit api, which means a
>> mostly-equivalent compat layer added in libbinder to support old
>> kernels.
>
> Wait, I thought that libbinder would have to be changed anyway here, to
> handle 64bit kernels (in both 32 and 64bit userspace).  Since you are
> already changing it, why not just "do it correctly"?

libbinder will need changes to support 64-bit userspace and especially
a mixed 64-bit and 32-bit userspace, but this patch series is only
addressing a pure 32-bit userspace on a 64-bit kernel.  Support for a
64-bit userspace in Android is obviously going to require a future
version of Android including, among other things, libbinder changes.
As far as I know, those changes won't need to change the ioctl api,
only the layout of the buffers that are passed through the ioctl api.

> Or does this patch series mean that no userspace code is changed?  Is
> that a "requirement" here?

Since this series only addresses 32-bit userspace on 64-bit kernel
support there are no associated userspace changes.  Changing the
32-bit api here means that combining the KitKat branch from
http://android.googlesource.com with a newer kernel version will not
work.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/