Message-ID: <8738434yjk.fsf@x220.int.ebiederm.org>
Date:	Mon, 13 Apr 2015 14:29:35 -0500
From:	ebiederm@...ssion.com (Eric W. Biederman)
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>, gnomes@...rguk.ukuu.org.uk,
	teg@...m.no, jkosina@...e.cz, luto@...capital.net,
	linux-kernel@...r.kernel.org, daniel@...que.org,
	dh.herrmann@...il.com, tixxdz@...ndz.org
Subject: Re: [GIT PULL] kdbus for 4.1-rc1

Greg Kroah-Hartman <gregkh@...uxfoundation.org> writes:

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

And the code that transfers the meta-data is wrong.

It is generally not something that userspace requires today, certainly
userspace is not using it.

You are exporting a weird set of information in a unique way that makes
it race free enough to make ``security'' decisions upon but the data
in general is not appropriate to make those decisions.

I remain opposed to this half thought out trash of an ABI for the
meta-data.

Just because something happens to be exported in a DEBUG api today does
not make it appropriate for userspace to run around making security
decisions with that information.

Nacked-by: "Eric W. Biederman" <ebiederm@...ssion.com>

I think it is premature to be merging kdbus.  You have fuddamental
issues that can not be fixed once the ABI is frozen.

The semantics of the meta-data you export are extremely poorly defined.

> The rest of this pull request message comes from the kdbus patch posting
> messages as sent to lkml previously:
>
> Reasons kdbus should be in the kernel, instead of userspace as it is
> currently done today includes the following:
>
>  * Performance: Fewer process context switches, fewer copies, fewer
>    syscalls, larger memory chunks via memfd.  This is really important
>    for a whole class of userspace programs that are ported from other
>    operating systems that are run on tiny ARM systems that rely on
>    hundreds of thousands of messages passed at boot time, and at
>    "critical" times in their user interaction loops. DBus is not used
>    for performance sensitive applications because DBus is slow.
>    We want to make it fast so we can finally use it for low-latency,
>    high-throughput applications. A simple DBus method-call+reply takes
>    200us on an up-to-date test machine, with kdbus it takes 8us (with
>    UDS about 2us). If the packet size is increased from 8k to 128k,
>    kdbus even beats UDS due to single-copy transfers.

And with a good design kdbus could be faster.

>  * Security: The peers which communicate do not have to trust each
>    other, as the only trustworthy component in the game is the kernel
>    which adds metadata and ensures that all data passed as payload is
>    either copied or sealed, so that the receiver can parse the data
>    without having to protect against changing memory while parsing
>    buffers. Also, all the data transfer is controlled by the kernel,
>    so that LSMs can track and control what is going on, without
>    involving userspace. Because of the LSM issue, security people are
>    much happier with this model than the current scheme of having to
>    hook into dbus to mediate things.
>  * More types of metadata can be attached to messages than in
>    userspace

The meta-data is poorly thought and and much of it is not appropriate
for making security decisions anywhere except in the kernel.

All I have seen with the meta-data discussion is sticking heads in the
sand and resubmitting and hoping your reviewers go away.

If you won't do a good responsible job on this before the code is merged
how can we possibly expect you to do a good job later.  Or is this going
to be another API where userspace will be broken at arbitrary moments by
arbitrary users?

How are you going to fix the security issues your poor API comes with it
when then are eventually spelled out clearly and to fix them means
breaking everyones desktop environment?

Eric

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