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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Thu, 30 Aug 2018 08:49:40 -0700
From:   Todd Kjos <tkjos@...gle.com>
To:     hch@...radead.org
Cc:     Todd Kjos <tkjos@...roid.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Arve Hjønnevåg <arve@...roid.com>,
        "open list:ANDROID DRIVERS" <devel@...verdev.osuosl.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Martijn Coenen <maco@...gle.com>,
        Christian Brauner <christian.brauner@...onical.com>,
        ben@...adent.org.uk, torvalds@...ux-foundation.org,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH] binder: use standard functions to allocate fds

On Wed, Aug 29, 2018 at 12:00 AM Christoph Hellwig <hch@...radead.org> wrote:
>
> >  config ANDROID_BINDER_IPC
> >       bool "Android Binder IPC Driver"
> > -     depends on MMU
> > +     depends on MMU && !CPU_CACHE_VIVT
>
> Thats is a purely arm specific symbol which should not be
> used in common code.  Nevermind that there generally should
> be no good reason for it.

It is true that the current design (10+ years old) has this flaw. VIVT
cache support was the rationale for using the non-standard functions
to allocate file descriptors from the sender context. There are no
known cases of recent android releases running on a device with
a VIVT cache architecture, but I did want to call this out.

We're working on refactoring to eliminate this issue.

>
> > +     fixup->offset = (uintptr_t)fdp - (uintptr_t)t->buffer->data;
>
> This looks completely broken.  Why would you care at what exact
> place the fd is placed?  Oh, because you share an array with fds
> with userspace, which is a hell of a bad idea, and then maninpulate
> that buffer mapped to userspace from kernel threads.

Well, android apps rely on this behavior (and have for 10+ years) so
there is no way to avoid the need to manipulate the buffer from
the kernel. We are working to refactor the driver to do this
using standard mechanisms instead of relying on low-level internal
functions.

>
> I think we just need to rm -rf drivers/android/binder*.c and be done
> with it, as this piece of crap should never have been merged to start
> with.

Powered by blists - more mailing lists