Message-ID: <CALCETrWx2sreF7+7XvLJi32jeeN+2peM_UNYxH-EiNqwh9zZVA@mail.gmail.com>
Date:	Mon, 13 Apr 2015 13:13:26 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
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 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.

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

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.

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.

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.

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.)

In summary, the general idea is good, but the implementation isn't
general enough, the policy stuff is too specialized and enshrines bad
design, the performance isn't good enough to justify it, and the
metadata is nasty.

So, for what it's worth, NACK in its present form.  Sorry.

--Andy
--
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