Date: Wed, 22 Apr 2015 10:58:28 +0200
From: Borislav Petkov <bp@alien8.de>
To: "Eric W. Biederman" <ebiederm@xmission.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Arnd Bergmann <arnd@arndb.de>, gnomes@lxorguk.ukuu.org.uk, teg@jklm.no,
        jkosina@suse.cz, luto@amacapital.net, linux-kernel@vger.kernel.org,
        daniel@zonque.org, dh.herrmann@gmail.com, tixxdz@opendz.org
Subject: Re: [GIT PULL] kdbus for 4.1-rc1
Message-ID: <20150422085827.GA6962@pd.tnic>
References: <20150413190350.GA9485@kroah.com>
 <8738434yjk.fsf@x220.int.ebiederm.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <8738434yjk.fsf@x220.int.ebiederm.org>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2876
Lines: 70

On Mon, Apr 13, 2015 at 02:29:35PM -0500, Eric W. Biederman wrote:
> 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@xmission.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.

Not only that - it looks like a serious amount of work on each sent
packet. So I did some staring, correct me if I missed something:

kdbus_cmd_send	- KDBUS_CMD_SEND, ioctl cmd, copy stuff from userspace
|-> kdbus_kmsg_new_from_cmd(), kmalloc+memset + prepare a *lot* of stuff like:
    |-> m->proc_meta = kdbus_meta_proc_new();
	m->conn_meta = kdbus_meta_conn_new();
	...
    |-> kdbus_bus_broadcast(conn->ep->bus, conn, kmsg); let's look at the broadcast mode
        |-> hash_for_each(bus->conn_hash, i, conn_dst, hentry) { 	iterate over hash buckets, O(256)
	    |-> kdbus_meta_proc_collect(kmsg->proc_meta, attach_flags);	collect a *lot* of stuff from current etc
	    |-> kdbus_meta_conn_collect(kmsg->conn_meta, kmsg, conn_src, attach_flags); collect more stuff

and this happens on *every* send. A *lot* of work.

Now multiply that by the amount of messages this thing is going to send
per second. It piles up. So you have the overhead right then and there
in the design without even being able to fix it. Or at least pretty damn
hard to fix.

So unless I'm missing something, this right there is a design problem.

Why can't this messaging be done with a nifty O(1) scheme like sending
parties issuing auth tokens and whatever and the kernel doing the
arbitration and distribution of those tokens?

That gets you sandboxing, dropping privileges and whatever else fancy
containers people wanna do for free. Token recipient has the token -
that's all that counts.

Again, this is from a short staring only, I might just as well be
missing something but you'll tell me :-)

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/