Re: [GIT PULL] kdbus for 4.1-rc1

[Andy Lutomirski]

On Mon, Apr 20, 2015 at 2:46 PM, Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, Apr 20, 2015 at 11:16:49PM +0200, Richard Weinberger wrote:
>> Greg,
>>
>> Am 20.04.2015 um 22:56 schrieb Greg Kroah-Hartman:
>> >> In which situation on a common Linux system is the current dbus too slow today?
>> >> I've never seen a issue like "Oh my system is slow because dbus is
>> >> eating too much CPU cycles".
>> >
>> > See the original email which explained all of the things we can not do
>> > with D-Bus, some of which are due to speed, that can now be done with the
>> > kdbus code.
>>
>> okay, let's do it together.
>>
>> 1. Performance
>> You write:
>> "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."
>>
>> Which applications exactly?
>> This reads to me like a solution for a non-existing problem.
>
> Anything that uses UDS for large buffers today can switch to using kdbus
> for it's data stream as it is faster.  I know the Pulse Audio people
> have discussed this, and there are other people as well (Enlightenment
> library developers, glib, wayland, etc.)  Without the code being in the
> kernel, no project is going to spend the time to convert their codebase
> to a feature that isn't accepted.

Anything that uses UDS for large buffers today can switch to using
memfd over SCM_RIGHTS right now.  If SCM_RIGHTS is too slow, then we
can fix it along the lines that Al proposed.

>
>> 2. Security
>> I don't think that you need a 13k piece of code in the kernel to solve
>> that issue.
>
> Wait, what?  How can you blow by that requirement by just saying that
> this proposal isn't acceptable?  You can't do that, sorry.  Please show
> how what we have proposed does not provide the security requirements as
> is documented.

This is backwards.  The way this discussion is going is:

kdbus promoters: here's some code

someone else: the code does such and such in a way that's wrong for xyz reason

kdbus promoters: show us the implementation bug in such and such

This is not how this discussion should work.  Richard didn't say there
was a bug in your code; he said that your code was too large.

>
>> 3. Semantics for apps with heavy data payloads
>>
>> Again, sounds like a solution for a non-existing problem.
>
> No, media apps need to share their data somehow, and kdbus provides a
> way to do that.  GNOME portals are one such proposed codebase that is
> looking to use kdbus for this, and again, so is Pulse Audio and the
> other groups listed above.

AFAICT you're talking about passing data into and out of a sandbox for
processing or UI purposes.  We have two excellent ways to do that
right now: memfd and splice, depending on exactly what you're doing.

>
>> 4. "Being in the kernel closes a lot of races which can't be fixed with
>>    the current userspace solutions."
>>
>> You really need a in-kernel dbus with 13k to solve that?
>
> Do you know of a smaller amount of code to solve this problem?  If so,
> wonderful, please show us, but we aren't playing code golf here.  We are
> proposing something that is well documented and easy to maintain, while
> still being fast and correct.  If it you think this can be done in a
> smaller amount of code, please show us where we are doing needless
> things in the patches.

I do.  Implement something like my old SCM_IDENTITY proposal, which is
kind of like kdbus metadata, opt-in, over UNIX sockets.  Except that I
never proposed most of the absurd metadata items that kdbus is
proposing, and I also suggesting doing it over plain old UNIX sockets.

If that ends up at more than 500 LOC, then something's wrong.  Also,
everyone gets the benefit, not just kdbus.

[snip]

> Because of that, and the thread where the proposed security problems
> were agreed not to be a security problem, I don't see a reason anymore
> why this code should not be merged.
>
> With the exception of Al's code review, which is being addressed.  But
> that's a minor thing, not a major design flaw at all.

My NACK stands.  A security problem was fixed, but the metadata system
has multiple problems, each of which is independently sufficient to
earn my nack.

On top of that, the policy mechanism is iffy and is probably worthy of my nack.

On top of that, I think that someone into resource management needs to
seriously consider whether having a broadcast send do get_user_pages
or the equivalent on pages supplied by untrusted recipients (plural!)
is a good idea.

On top of *that*, I have serious doubts that the whole design make
sense.  That doesn't earn my nack specifically, but it sure seems like
a lot of people share my doubts that the design makes sense, and I
don't hear a whole lot of people saying that they thing the design is
a good thing to put in the kernel.

Also, the current thread-of-lesser-doom on the systemd list greatly
decreases my confidence that the issues that have earned my nack will
get resolved.  The kdbus designers seem to be unwilling to accept that
code should be merged into the kernel merely because I (me,
personally) don't see a straightforward security exploit that the code
enables.

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