lkml.org 
[lkml]   [2013]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v1 9/9] staging: android: binder: Add binder compat layer
    From
    On Wed, Dec 4, 2013 at 2:02 PM, Greg KH <gregkh@linuxfoundation.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@linuxfoundation.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@linuxfoundation.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"?
    >

    Yes libbinder will have to be changed to support calls between 32 bit
    and 64 bit processes, so I don't see much value in a patchset that
    only supports all 32 bit or all 64 bit processes. If user space is
    fixed to use 64 bit pointers on a 64 bit system, then much of the code
    added in this patchset becomes useless (and probably harmful as it
    appears to prevent 32 bit processes from communicating with 64 bit
    processes).

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

    I don't think we need to support old 32 bit userspace framework code
    on a 64 bit system. I think it is more important to not prevent mixed
    mode systems.

    --
    Arve Hjønnevåg
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2013-12-05 03:21    [W:7.178 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site