Message-ID: <20150413204547.GB1760@kroah.com>
Date:	Mon, 13 Apr 2015 22:45:47 +0200
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Arnd Bergmann <arnd@...db.de>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	Tom Gundersen <teg@...m.no>, Jiri Kosina <jkosina@...e.cz>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Daniel Mack <daniel@...que.org>,
	David Herrmann <dh.herrmann@...il.com>,
	Djalal Harouni <tixxdz@...ndz.org>
Subject: Re: [GIT PULL] kdbus for 4.1-rc1

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@...uxfoundation.org> 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.

And lots of things has changed from userspace, based on a decade of
knowledge of how dbus works, and how dbus itself was implemented.  The
design, and code, has been reviewed by those developers.  Where issues
were raised, they were fixed.

Yes, dbus is "odd", but it serves a real need, and does so quite well,
and now kdbus is the next evolution of that system, fixing and
addressing the issues learned from implementing and designing dbus and
previous versions of this type of ipc (corba, dcom, com, etc.)

> Kdbus claims to be very fast.  Unfortunately, requests for a broad set
> of benchmarks have mostly been ignored, my attempts to benchmark it
> (admittedly I didn't try that hard) were several times worse than
> published figures, and, most tellingly, *no one* has claimed that
> kdbus is faster than AF_UNIX.  In fact, everyone seems to acknowledge
> that kdbus is several times slower than AF_UNIX.

It does more than AF_UNIX, so of course it's going to be slower.  But
you can't do all the things you need to do with dbus with just AF_UNIX,
it's a different model.  Again, the documentation should explain this.

And the benchmarks and source were posted by David previously, with full
details, this is the first time I've heard you could not reproduce them
using that code.

> The metadata thing is problematic.  It seems to be intended to serve
> two purposes: data gathering for logging and authentication.
> Unfortunately, it has issues.  There are no fewer than *three*
> metadata capture points: creation of a bus, connection to a bus, and
> sending of a message.  The kdbus authors like to point out that these
> are all optional, but IMO that's bunk.  Someone will write a userspace
> library that rejects messages from people who don't enable all of
> them, then then we're screwed.

Remember, you asked for it to be optional, it wasn't in the beginning :)

So let's make it not optional, great.  And the capture points are in
different places as it is different data and entry points.

> Why are we screwed?  Because any kdbus client *won't know which
> metadata matters*.  That means that we automatically have the worst of
> all worlds, not the best.  Also, the bus creation metadata is
> completely worthless for anything other than logging, but someone will
> use it for something other than logging, at which point it's
> vulnerable to a DoS.  No one has explained to my satisfaction why this
> isn't a problem.

I don't think the creation data is worthless, I'm pretty sure the
SELinux people are using it to validate things, but I could be wrong.
Others on the cc: know more about that than I do and can provide
details.

> Also, the metadata code captures things that are, in my book
> completely unacceptable, such as cmdline and (!) capabilities.  I bet
> that the cmdline capture is extra special fscked up when cgroups and
> such are in play because *it reads from the sender's VM*.  IOW it's
> insecure and pointless.  (OK, it has a point: logging.  But I really
> don't think that belongs in the kernel.)

The sender's vm is what is wanted here.  And cmdline is something that
userspace gets today, and does things with, as does SELinux, and
auditing.  Same for capabilities, it's not insecure and pointless, it's
the same thing that is provided to userspace, and userspace makes
decisions on today, independent of kdbus/dbus.

thanks,

greg k-h
--
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/

Powered by blists - more mailing lists