Re: [GIT PULL] kdbus for 4.1-rc1

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Apr 20, 2015 at 03:06:09PM -0700, Andy Lutomirski wrote:
> On Mon, Apr 20, 2015 at 2:46 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> 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.

But that doesn't solve the latency issues.

As has been said many times in this thread, when using UDS to build a
better IPC for apps, you will probably end up with todays D-Bus
userspace implementation, and not have any of the other things that we
keep talking about kdbus having.

Bringing up SCM_RIGHTS means that this is not going to be a bus system
at all.  One principal design goal is to _not_ have peer-to-peer
connections between all communicating parties, but rather one connection
to a central component.  If that component is not in the kernel, it has
to be a userspace deamon, which in turn has all of the issues that
dbus-daemon currently has.

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

"Your code is too large" does not provide any value to this discussion
at all, sorry.  Richard is being a jerk here, please don't perpetuate
that line of discussion, it's not helpful at all.

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

That does not solve the latency issues, which is crucial for sound and
graphics.

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

We _want_ this metadata.  You don't, that's fine.  Calling our position
"absurd" does not contribute to the discussion.  We are simply exporting
data that is already accessible via /proc and other locations, and do so
in a race-free manner, something the kernel has never been able to
provide in the past.

We do not, in any way, export any additional internal kernel state,
again, we are merely closing a race gap that has been there.

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

Please note that this issue was addressed in v2, which was posted many
months ago.  It is not present in this submission at all.

> but the metadata system
> has multiple problems, each of which is independently sufficient to
> earn my nack.

If you still see a problem, please explain what it is.  At least give a
general outline so that we can try to understand where you are coming
from here.  On the systemd mailing list you said that your only issue
was that you are not convinced that this is a useful feature.  But now
you are saying you have "multiple concerns".  What are they?

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

This is a well-established concept that has worked great for many years.
Why should we break with that?

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

Recipients need TALK access to the sender to receive broadcasts.
Furthermore, even on AF_UNIX you need sender-side buffering, which might
trigger reclaim.  But sender-side buffering does not make sense for
broadcasts (there is no POLLOUT for broadcasts), which is why we
implemented the kdbus pools.  I really doubt that the netlink-way of
making all buffers kernel-owned is the way to go.  But it would be
trivial to change pool-memory on the root-memcg or even lock it, which
would be almost equivalent to kernel-owned buffers, if you think that
would solve a problem.

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

You have claimed that there is a security issue with no arguments
backing it up.  No one is expecting an actual exploit, but at least an
explanation of what you have in mind and how it applies to the kdbus
design would be appreciated.  Otherwise this is an argument that no one
can ever refute, and isn't fair.

We are trying to find proper solutions for the problems we see, and that
people tell us about.  If there is a security issue in any of this,
please let us know, and if these are unfixable we are very open to
change the design.  After all, that's why we are discussing it here in
the open.

Your review comments, and Al's, have been invaluable in helping make
this code better, and I greatly appreciate them.  The code is much
better today than the v1 submission, and it shows, your insights here
have been wonderful.  But now, by saying that somehow the existing
design details that we have picked are dangerous, without providing any
details as to _why_ they are dangerous, is leaving us with nothing to
actually be able to change.

So please, specifics please, otherwise there's no way that we can
provide a solution for this problem area.

thanks,

greg k-h