Re: [GIT PULL] kdbus for 4.1-rc1

[Greg Kroah-Hartman]

On Tue, Apr 14, 2015 at 11:57:22AM -0700, Andy Lutomirski wrote:
> On Tue, Apr 14, 2015 at 10:50 AM, Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Mon, Apr 13, 2015 at 02:01:21PM -0700, Andy Lutomirski wrote:
> >> On Mon, Apr 13, 2015 at 1:45 PM, Greg Kroah-Hartman
> >> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >> > On Mon, Apr 13, 2015 at 01:13:26PM -0700, Andy Lutomirski wrote:
> >> >> On Mon, Apr 13, 2015 at 12:03 PM, Greg Kroah-Hartman
> >> >> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >> >> > The following changes since commit 9eccca0843205f87c00404b663188b88eb248051:
> >> >> >
> >> >> >   Linux 4.0-rc3 (2015-03-08 16:09:09 -0700)
> >> >> >
> >> >> > are available in the git repository at:
> >> >> >
> >> >> >   git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/ tags/kdbus-4.1-rc1
> >> >> >
> >> >> > for you to fetch changes up to 9fb9cd0f4434a23487b6ef3237e733afae90e336:
> >> >> >
> >> >> >   kdbus: avoid the use of struct timespec (2015-04-10 14:34:53 +0200)
> >> >> >
> >> >> > ----------------------------------------------------------------
> >> >> > kdbus for 4.1-rc1
> >> >> >
> >> >> > Here's the kdbus pull request for 4.1-rc1.
> >> >> >
> >> >> > It's been under development for many years now, and been in linux-next
> >> >> > for many months, and has undergone loads of testing a review and even a few
> >> >> > good arguments.  It comes with full documentation and tests.
> >> >> >
> >> >> > There has been a few complaints about the code, notably from people who
> >> >> > don't like the use of metadata in the bus messages.  That is actually
> >> >> > one of the main features here, as we can get this data in a secure and
> >> >> > reliable way, and it's something that userspace requires today.  So
> >> >> > while it does look "odd" to people who are not familiar with dbus, this
> >> >> > is something that finally fixes a number of almost unfixable races in
> >> >> > the current dbus implementations.
> >> >>
> >> >> While I generally like the concept of having a better in-kernel IPC
> >> >> mechanism, after some consideration I don't think this belongs in the
> >> >> kernel in its current form.  Here's why.
> >> >>
> >> >> First, the naming is counterintuitive.  There are "endpoints", but you
> >> >> don't send messages to endpoints.  In fact, an basic kdbus setup will
> >> >> have exactly one endpoint AFAICT.  Wtf?  This makes talking about it
> >> >> awkward.
> >> >
> >> > Did you read the documentation?  We've been over this before, and it
> >> > should all be addressed in the documentation based on this coming up.
> >> >
> >> >> A lot of the design seems to be to violate the concept of "mechanism,
> >> >> not policy".  Kdbus is very much a port of userspace dbus to the
> >> >> kernel, and it appears to be a port designed to preserve some
> >> >> questionable design decisions instead of learning from them.
> >> >>
> >> >> For example, kdbus sticks a whole policy database in the kernel, but
> >> >> that policy database (AFAICT -- holy crap it's overcomplicated) is
> >> >> *not* a simple set of rules like "if A then allow B".  Instead it has
> >> >> really weird dependencies not on what name you're sending to but on
> >> >> what *other* names the thing you're sending to has.  Sorry, but this
> >> >> way lies (a) the inability for a large set of developers to understand
> >> >> what's going on and (b) security bugs.  Also, the result probably
> >> >> can't be reused as part of a non-legacy-filled sensible design
> >> >
> >> > What policy database?  Matching messages to subscribers?  That's the
> >> > same type of "database" that other ipc subsystems need/want, there's
> >> > nothing radical here.
> >>
> >> Let me quote from the latest version of the kdbus docs:
> >>
> >>       Note that TALK access is checked against all names of a connection. For
> >>       example, if a connection owns both <constant>'org.foo.bar'</constant> and
> >>       <constant>'org.blah.baz'</constant>, and the policy database allows
> >>       <constant>'org.blah.baz'</constant> to be talked to by WORLD, then this
> >>       permission is also granted to <constant>'org.foo.bar'</constant>. That
> >>       might sound illogical, but after all, we allow messages to be directed to
> >>       either the ID or a well-known name, and policy is applied to the
> >>       connection, not the name. In other words, the effective TALK policy for a
> >>       connection is the most permissive of all names the connection owns.
> >>
> >> In my humble opinion, this paragraph speaks for itself.  The design is
> >> bad, full stop.
> >
> > First off, thanks for reading the docs, I appreciate that.  But realize
> > also, that this is straight from the D-Bus spec.  We aren't doing
> > anything "radical" here, this is what your desktop uses that you are
> > typing your email from.
> >
> > Yes, it's an unfortunate design, but one that we are all stuck with
> > (think of it as having to implement code for horrid hardware that you
> > have to get to work properly.)
> 
> I agree.  You've sent a pull request for an unfortunate design.  I
> don't think that unfortunate design belongs in the kernel.  If it says
> in userspace, then user programmers could potentially fix it some day.

You might not like the design, but it is a valid design.  Again, we
don't refuse to support hardware that is designed badly.  Or support
protocols we don't necessarily like, that's not the job of a kernel or
operating system.

And here's Havoc's response as to why actually, this is a good design:
	http://lists.freedesktop.org/archives/dbus/2015-April/016651.html

so while we might not think it's nice, maybe we are just not that
knowledgeable in this design space, and need to trust those that are.

I know I do.

I'll respond to the rest after I get some dinner...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/