linux-kernel - Re: [PATCH v1 2/2] binderfs: reserve devices for initial mount

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20190103220841.ycoavsqcbauayypl@brauner.io>
Date:   Thu, 3 Jan 2019 23:08:42 +0100
From:   Christian Brauner <christian@...uner.io>
To:     Todd Kjos <tkjos@...gle.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Todd Kjos <tkjos@...roid.com>,
        "open list:ANDROID DRIVERS" <devel@...verdev.osuosl.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Arve Hjønnevåg <arve@...roid.com>,
        Martijn Coenen <maco@...roid.com>, joel@...lfernandes.org
Subject: Re: [PATCH v1 2/2] binderfs: reserve devices for initial mount

On Thu, Jan 03, 2019 at 01:47:13PM -0800, Todd Kjos wrote:
> On Thu, Jan 3, 2019 at 12:34 PM Christian Brauner <christian@...uner.io> wrote:
> >
> > On Thu, Jan 03, 2019 at 12:25:24PM -0800, Todd Kjos wrote:
> > > On Sun, Dec 23, 2018 at 6:36 AM Christian Brauner <christian@...uner.io> wrote:
> > > >
> > > > The binderfs instance in the initial ipc namespace will always have a
> > > > reserve of 4 binder devices unless explicitly capped by specifying a lower
> > > > value via the "max" mount option.
> > > > This ensures when binder devices are removed (on accident or on purpose)
> > > > they can always be recreated without risking that all minor numbers have
> > > > already been used up.
> > > >
> > > > Cc: Todd Kjos <tkjos@...gle.com>
> > > > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > > > Signed-off-by: Christian Brauner <christian.brauner@...ntu.com>
> > > > ---
> > > > v1:
> > > > - patch introduced
> > > > v0:
> > > > - patch not present
> > > > ---
> > > >  drivers/android/binderfs.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > > > index 873adda064ac..aa635c7ea727 100644
> > > > --- a/drivers/android/binderfs.c
> > > > +++ b/drivers/android/binderfs.c
> > > > @@ -40,6 +40,8 @@
> > > >  #define INODE_OFFSET 3
> > > >  #define INTSTRLEN 21
> > > >  #define BINDERFS_MAX_MINOR (1U << MINORBITS)
> > > > +/* Ensure that the initial ipc namespace always has a devices available. */
> > > > +#define BINDERFS_MAX_MINOR_CAPPED (BINDERFS_MAX_MINOR - 4)
> > >
> > > Why do you ever need more minors per instance than the number of
> > > devices listed in CONFIG_ANDROID_BINDER_DEVICES?
> >
> > Are you asking asking why this is 4 and not 3? In that case we should
> > probably parse CONFIG_ANDROID_BINDER_DEVICES once at init time and then
> > reserve that many binder devices. Thoughts?
> 
> I'm asking why CONFIG_ANDROID_BINDER_DEVICES isn't the source of truth
> for the number of devices (it normally specifies 3 devices, but could
> be different). I can't think of a reason why you'd want
> CONFIG_MAX_MINOR_CAPPED to be different than the number of devices
> indicated by CONFIG_ANDROID_BINDER_DEVICES and having it specified in
> two places seems error prone.

I'm not following. The CONFIG_MAX_MINOR_CAPPED and
CONFIG_ANDROID_BINDER_DEVICES do not have a relation to each other just
like binder devices requested in CONFIG_ANDROID_BINDER_DEVICES do not
have a relation to binderfs binder devices as was requested for this
patchset.
I also don't know what you mean "appear in two places".

The fact is, binderfs binder devices are independent of binderfs binder
devices. So it is perfectly reasonable for someone to say
CONFIG_ANDROID_BINDER_DEVICES="" and *only* rely on binderfs itself.
Which absolutely need to be possible.
What I want in such scenarios is that people always have a number of
binderfs binder devices guaranteed to be available in the binderfs mount
in the initial ipc namespace no matter how many devices are allocated in
non-initial ipc namespace binderfs mounts. That's why allo non-initial
ipc namespace binderfs mounts will use the CONFIG_MAX_MINOR_CAPPED macro
which guarantees that there's number of devices reserved for the
binderfs instance in the initial ipc namespace.