Daniel Mack [Wed, 7 Jan 2015 14:48:54 +0000 (15:48 +0100)]
names: ignore return value of kdbus_notify_name_change
If we cannot notify connections about a lost name, just continue.
Releasing a name with CMD_NAME_RELEASE should never fail because of
such a condition.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Djalal Harouni [Wed, 7 Jan 2015 13:33:29 +0000 (14:33 +0100)]
message: messages that expect a reply must provide a valid tracking cookie
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 7 Jan 2015 13:16:20 +0000 (14:16 +0100)]
test: test case for pending requests quota limit
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Daniel Mack [Wed, 7 Jan 2015 12:59:44 +0000 (13:59 +0100)]
Remove TODO
These have all been addressed or discussed, so the file can go away.
Signed-off-by: Daniel Mack <daniel@zonque.org>
David Herrmann [Wed, 7 Jan 2015 12:30:28 +0000 (13:30 +0100)]
kdbus: drop KDBUS_ITEM_SIGMASK
The sigset_t type is arch-dependent. We really don't want such types in
our kdbus API. Our CANCEL_FD provides a safe alternative, so use it.
If anyone wants SIGMASK support later on, we can always add it again. But
unless someone wants it, we will try hard to keep it out of kdbus.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Wed, 7 Jan 2015 11:49:48 +0000 (12:49 +0100)]
Revert commit
f198a45 "kdbus.h: Use #defines rather than an enum..."
This reverts commit:
commit
f198a45e3242a4825e1e720c0637a151a6ef5e0d
Author: Daniel Mack <daniel@zonque.org>
Date: Fri Oct 31 09:03:16 2014 +0100
kdbus.h: Use #defines rather than an enum for ioctl definition
If you want to discover available features, use autoconf.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Djalal Harouni [Tue, 6 Jan 2015 20:55:08 +0000 (21:55 +0100)]
test: standarize the userns arguments
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Mon, 5 Jan 2015 19:33:56 +0000 (20:33 +0100)]
connection: for KDBUS_MSG_SIGNAL check match db then the policy db
Minor optimization, before locking the endpoint policy db in order to
check for TALK access, check first that the destination match db is
interested in the signal.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Mon, 5 Jan 2015 14:08:26 +0000 (15:08 +0100)]
connection: code documentation for the signal access logic
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Daniel Mack [Mon, 5 Jan 2015 13:32:26 +0000 (14:32 +0100)]
connection: handle KDBUS_MSG_SIGNAL
Add missed hunks for handling KDBUS_MSG_SIGNAL.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Mon, 5 Jan 2015 12:38:31 +0000 (13:38 +0100)]
kdbus.h: close ioctl number gap (ABI break)
0x32 was used by KDBUS_CMD_CANCEL which was removed.
Close the number gap and reuse that number for CMD_FREE.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Mon, 5 Jan 2015 12:22:48 +0000 (13:22 +0100)]
kdbus.h: introduce KDBUS_MSG_SIGNAL
In order to allow directed (unicast) signals, we need to split the
handling logic and introduce KDBUS_MSG_SIGNAL as message flag.
For signals, no matter if unicast or broadcast, we apply the following
policy logic:
* The _destination_ of the message must have a TALK permission to
the _sender_
* The _destination_ must have a bloom filter installed that matches
the bloom filter attached to the message
Tests are tweaked to reflect the new implementation.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Djalal Harouni [Sun, 4 Jan 2015 13:55:40 +0000 (14:55 +0100)]
test: add kdbus_msg_send_reply()
Move send_reply() from test-sync.c to kdbus-util.c and rename it to
kdbus_msg_send_reply().
Currently there is only one user, will add a quota test for sync
messages later that will use it.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Sat, 3 Jan 2015 23:01:04 +0000 (00:01 +0100)]
connection: ignore the CANCEL_FD item on asynchronous messages
The doc states that CANCEL_FD should be installed for synchronous, and
for asynchronous messages it should be accepted but ignored, so fix the
code to reflect this.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Sat, 3 Jan 2015 22:56:38 +0000 (23:56 +0100)]
test-metadata-ns: various fixes to metadata-ns test
Rename the functions that try to match the received CREDS or PIDS items
Make sure that we dump the full queue of the monitor connection and that
the received PIDS are valid, the monitor is in the parent pidns so it
should be able to map all the received pids.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Daniel Mack [Thu, 25 Dec 2014 17:09:45 +0000 (18:09 +0100)]
connection: fix documentation
A connection can be terminated by simply closing its file descriptor.
Don't confuse readers by mentioning BYEBYE explicitly.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Djalal Harouni [Thu, 25 Dec 2014 16:28:30 +0000 (17:28 +0100)]
connection: fix kdbus_conn_wait_reply() documentation
* Add kdbus_conn_wait_reply() kerneldoc
* Fix some code doc, on some points we are referring to the calling
connection that issued the sync send.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Thu, 25 Dec 2014 15:42:38 +0000 (16:42 +0100)]
connection: doc: kdbus_conn_reply_find() callers have to take the lock
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Thu, 25 Dec 2014 15:27:54 +0000 (16:27 +0100)]
connection: ensure that if cookie_reply was provided then there is a pending request
If a cookie_reply was provided, then we must ensure that it is a real
reply message to a previously pending request. Currently we don't do
that correctly, if there are no pending request from origin then we
fallback to kdbus_conn_policy_talk() access check. Fix this by making
sure that if cookie_reply is set then the origin must have a pending
request in its queue. Its userspace responsability to sync its logic.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Thu, 25 Dec 2014 15:12:16 +0000 (16:12 +0100)]
connection: rename 'reply_count' field to 'request_count'
When reading the code you need always to remember that the reply_count
is not about conting replies, but for the pending request issued by a
connection that are still waiting for a reply. So just rename the
variable nane to reflect its usecase without having to refer to its
kerneldoc.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Thu, 25 Dec 2014 15:05:00 +0000 (16:05 +0100)]
test-sync: use ASSERT_EXIT() in child process
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Thu, 25 Dec 2014 00:05:10 +0000 (01:05 +0100)]
test-sync: add close_epipe_sync test
This test makes sure that we get -EPIPE while waiting for the sync reply
and the replying connection was closed. It's like the BYEBYE test except
that here we close the fd of the replying connection.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
David Herrmann [Wed, 24 Dec 2014 12:22:18 +0000 (13:22 +0100)]
metadata: document 'last_cap' ABI
Lets make clear that user-space can expect 'last_cap' to be equal to
/proc/sys/kernel/cap_last_cap. User-space can pre-allocate sufficient
array space this way, without checking it on each item.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Daniel Mack [Tue, 23 Dec 2014 19:34:10 +0000 (20:34 +0100)]
queue: drop unused function parameter
kdbus_queue_entry_move() doesn't use its first parameter 'conn_src'
for anything, so drop it.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Tue, 23 Dec 2014 17:25:08 +0000 (18:25 +0100)]
metadata: minor variable cleanup
Keep variables locals if we can, and give them specific names to make
the code easier to read.
Fix a small typo in kdbus.txt while at it.
Signed-off-by: Daniel Mack <daniel@zonque.org>
David Herrmann [Mon, 22 Dec 2014 16:18:32 +0000 (17:18 +0100)]
fs: flush VFS cache on node deactivation
Whenever a node is deactivated, we now invalidate any cached dentries.
This will make sure that we don't leave any dead entries in the VFS cache.
While this is not bad as is, it does make the cache slower. Therefore,
flush those entries as they will never be reused anyway.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Mon, 22 Dec 2014 16:09:50 +0000 (17:09 +0100)]
fs: keep domain->dentry backlink
We don't really want backlinks into the VFS, however, we need some way to
thrash old VFS cache entries when objects get destructed. Therefore, safe
a pointer to the root dentry in every domain.
We don't pin the dentry, as this would create circular dependencies.
Instead, we bind the dentry validity to the active lifetime of the domain.
Therefore, you can only access domain->dentry as long as it is active.
Once umount() is called, we deactivate the domain and thus drop the dentry
cache.
This backlink will be needed in a follow-up that flushes caches on object
destruction.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Mon, 22 Dec 2014 15:04:58 +0000 (16:04 +0100)]
node: add kdbus_node_is_deactivated()
This is similar to kdbus_node_is_active(), but returns 'true' if
kdbus_node_deactivate() was called on the node.
Similar to kdbus_node_is_active(), there is no guarantee that the node
stays in that state. Therefore, it's only safe to use it if you don't
care for reliability or if you have other means of synchronization against
node lifetime changes.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Mon, 22 Dec 2014 15:00:16 +0000 (16:00 +0100)]
fs: bind domain-lifetime to root dentry
Make sure we activate domains *after* we allocated the root dentry, and
deactivate them *before* destroying the root dentry. This allows us to
access the root dentry as long as we hold an active reference to the
linked domain.
During mount, it doesn't matter in which order we activate the domain or
root dentry, as the underlying superblock is still locked and inaccessible
from user-space. Similarly, during sb-kill, all user-space mounts have
already been removed so the sb is inaccessible from user-space. It's
therefore safe to use any activation/deactivation order we want.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Mon, 22 Dec 2014 14:57:18 +0000 (15:57 +0100)]
node: make clear that ->parent is valid until destruction
So far we pretended that ->parent is no longer valid once a node was
deactivated. However, we always pinned the parent until destruction for
several reasons. Update our comments to make this clear.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 19:15:30 +0000 (20:15 +0100)]
test-sync: add no_cancel test
This test passes a CANCEL_FD but doesn't signal it. Therefore, it should
have no effect.
This catches wrong POLL masks in the kernel.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Daniel Mack [Mon, 22 Dec 2014 11:29:21 +0000 (12:29 +0100)]
connection: catch return value of kdbus_queue_entry_move()
If kdbus_queue_entry_move(), we really shouldn't continue but
report the error up the call chain.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Mon, 22 Dec 2014 11:27:56 +0000 (12:27 +0100)]
queue: up-chain error in kdbus_queue_entry_move()
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Mon, 22 Dec 2014 11:27:27 +0000 (12:27 +0100)]
pool: clean up error handling in kdbus_pool_copy()
Signed-off-by: Daniel Mack <daniel@zonque.org>
Djalal Harouni [Sun, 21 Dec 2014 19:14:37 +0000 (20:14 +0100)]
connection: only one KDBUS_ITEM_CANCEL_FD item is allowed
Make sure that only one KDBUS_ITEM_CANCEL_FD item is passed.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Daniel Mack [Sun, 21 Dec 2014 16:20:06 +0000 (17:20 +0100)]
connection: pin cancel_fd before sending message
If userspace provides a bogus cancel_fd, make sure not to queue the
message on the receiver. Instead, pin the fd from kdbus_cmd_msg_send()
and pass the pinned cancel_fd to kdbus_conn_wait_reply().
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Sun, 21 Dec 2014 00:36:04 +0000 (01:36 +0100)]
message: fix double-increade of kmsg->pool_size
We're already doing this conditionally some lines above.
Fixes SD's ./test-bus-zero-copy.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Djalal Harouni [Sat, 20 Dec 2014 23:34:54 +0000 (00:34 +0100)]
connection: rename kdbus_conn_reply_sync() to kdbus_sync_reply_wakeup()
Rename kdbus_conn_reply_sync() to kdbus_sync_reply_wakeup(), using the
name object sometimes makes it easy to remember what the function is
about, but here it seems not the case, especially for the kdbus_conn_reply
object which is used in several places... so just rename this function
to make it reflect that we are waking up origin due to the sync reply
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Sat, 20 Dec 2014 23:32:05 +0000 (00:32 +0100)]
kdbus-test: fail with return not _exit() here
Fix my stupid copy paste errors
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Daniel Mack [Sat, 20 Dec 2014 23:12:58 +0000 (00:12 +0100)]
message: allocate the correct size of kmsg->iov
Allocating (n_vecs + n_memfds) number of elements in kmsg->iov
is likely too much. Calculate how many we really need.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Sat, 20 Dec 2014 22:35:37 +0000 (23:35 +0100)]
message: keep track of kmsg->iov length through kmsg->iov_count
Count the array members of kmsg->iov in a new variable called
kmsg->iov_count.
This is necessary because res->vec_count is also used for added
memfd zero-byte alignments, and the message install logic in
queue.c expect to see KDBUS_MSG_DATA_VEC entries in res->data.
It also feel more logical to have the number of array members
stored next to the actual array.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Sat, 20 Dec 2014 21:48:03 +0000 (22:48 +0100)]
queue.c: cosmetic cleanup
Spare one pair of parentheses.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Sat, 20 Dec 2014 14:35:41 +0000 (15:35 +0100)]
node, notify, pool: remove some more BUG_ON()
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Sat, 20 Dec 2014 14:25:43 +0000 (15:25 +0100)]
connection, names: some less BUG_ON()
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Sat, 20 Dec 2014 02:21:41 +0000 (03:21 +0100)]
connection: intialize pwq later
This way, we can get rid of clecnup pathes in error cases
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Fri, 19 Dec 2014 19:00:53 +0000 (20:00 +0100)]
connection: fix memory leak wrt cancel_fd
We have to poll_freewait() in error pathes.
Signed-off-by: Daniel Mack <daniel@zonque.org>
David Herrmann [Fri, 19 Dec 2014 18:54:03 +0000 (19:54 +0100)]
connection: don't pass NULL to f->poll()
We must never pass NULL to f->poll(). Instead, we reset the callback to
NULL, thus the poll function will never add more queues.
We already do this via init_poll_funcptr(&xyz, NULL) so we can safely pass
the poll context everytime. This also allows us to drop the first dummy
call to ->poll().
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Daniel Mack [Fri, 19 Dec 2014 18:53:11 +0000 (19:53 +0100)]
connection: minor comment nit
Signed-off-by: Daniel Mack <daniel@zonque.org>
David Herrmann [Fri, 19 Dec 2014 18:41:53 +0000 (19:41 +0100)]
connection: don't oops if CANCEL_FD doesn't support poll
Verify the passed CANCEL_FD does actually support ->poll(). Otherwise, we
will call a NULL pointer.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Daniel Mack [Fri, 19 Dec 2014 18:48:31 +0000 (19:48 +0100)]
connection: don't break lines unless we have to
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Fri, 19 Dec 2014 18:47:52 +0000 (19:47 +0100)]
connection: add comment on init_poll_funcptr()
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Fri, 19 Dec 2014 18:44:41 +0000 (19:44 +0100)]
kdbus.txt, Changelog: document KDBUS_ITEM_CANCEL_FD
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Fri, 19 Dec 2014 18:44:00 +0000 (19:44 +0100)]
connection: trigger on cancel_fd->poll() & POLLIN
POLLIN is the event we're waiting for, not POLLOUT.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Fri, 19 Dec 2014 18:27:18 +0000 (19:27 +0100)]
test-sync: add test for cancelling a sync send with an eventfd
Fork a process, issue a blocking send with a cancelfd, and kill the
command by writing to the cancel fd.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Fri, 19 Dec 2014 18:25:14 +0000 (19:25 +0100)]
test: add cancel_fd parameter to kdbus_msg_send_sync()
Add another parameter to kdbus_msg_send_sync() which should be set
to -1 if unused. If >= 0, it will cause the SEND cmd ioctl to carry
an item of type KDBUS_ITEM_CANCEL_FD, and put the given value into
it.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Fri, 19 Dec 2014 18:17:01 +0000 (19:17 +0100)]
connection: accept eventfd as cancellation point
Allow passing in a CANCEL_FD item with the send command (attached to
the command, not the message!) that carries a file descriptor.
When userspace writes to this fd, use it as cancellation point an
return -ECANCELED to the blocking caller of KDBUS_CMD_SEND.
For this to work, we have to sleep on two wait queues now - one for
our own connection, one for the passed in fd. For this, open-code
what select() does, and use a struct poll_wqueues on the stack
to attach both wake up source to it. Then use poll_schedule_timeout()
to actually put the task to sleep. However, we have to implement
the condition checking ourselves, in a busy loop.
item->fds[0] may be any type of file descriptor that implements
poll(). For instance, an eventfd.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Fri, 19 Dec 2014 18:16:01 +0000 (19:16 +0100)]
connection: pass struct file from ioctl handler to kdbus_conn_kmsg_send()
We need to have access to the struct file that was used to issue the
ioctl later, so let's pass it down to kdbus_conn_kmsg_send().
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Fri, 19 Dec 2014 18:11:29 +0000 (19:11 +0100)]
kdbus.h: add KDBUS_ITEM_CANCEL_FD (ABI break)
Add a new item that takes a file descriptor, used to cancel a synchronous
SEND operation.
Signed-off-by: Daniel Mack <daniel@zonque.org>
David Herrmann [Fri, 19 Dec 2014 18:02:19 +0000 (19:02 +0100)]
policy: optimize wildcard entry lookup
There is really no reason to duplicate the lookup string if all we do is
prefix comparison. Introduce kdbus_strnhash() and then do proper prefix
comparison in kdbus_policy_lookup() without string modification.
There is a reason DBus uses reverse DNS-notation. Use it!
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 17:55:41 +0000 (18:55 +0100)]
util: kdbus_str_hash() -> kdbus_strhash()
Lets make this function look more like strlen(), strcopy(), ... and
friends. Not that I like it in particular, but a followup will introduce
kdbus_strnhash() and those names get really weird once used with
underscore style.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 17:46:28 +0000 (18:46 +0100)]
policy: drop unused 'wildcard' parameter
This argument is unused, drop it from kdbus_policy_lookup().
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 17:41:59 +0000 (18:41 +0100)]
policy: drop unused kdbus_policy_check_access()
Inline this helper into kdbus_policy_query_unlocked(). No reason to split
both. The query helper is a one-liner and the only caller of
check_access().
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 17:36:23 +0000 (18:36 +0100)]
policy: return highest policy on kdbus_policy_query()
Instead of asking for a specific policy level, we now always return the
highest level found. That means, kdbus_policy_query() always searches
through all access-entries of a given name-entry and tries to find the
highest access-level that matches.
Note that this means we cannot shortcut the policy-check on a single name.
However, that should not affect the performance considerably. But the
upside is we now can properly cache any result of the policy-db. There is
no reason to limit it to TALK. Instead, a policy cache can properly take
the policy result and just cache it, regardless of the call-side.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 17:13:38 +0000 (18:13 +0100)]
connection: fix information leak on !SEE for real
In the past we noticed that if you cannot SEE a name, we should not return
EPERM but ENOENT. Otherwise, you will notice the name exists in case the
policy-check is done _after_ looking up the name.
For NAME queries we're fine, but for ID queries we're not. Lets fix this
all at once and make both lookup and policy-check use the same code-path,
and thus the same error-code.
Also add tests to verify both error codes are always the same. There is no
reason to let user-space know "you're allowed to see that name but it
doesn't exist". Stop that crazy and just return the same error regardless
whether you cannot see it or whether it doesn't exist.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 17:03:24 +0000 (18:03 +0100)]
connection: return 'bool' for policy decisions
It's rather weird to generate 'int' as result for a policy query. Such
queries must never fail for any reason but EPERM, so lets hard-code that
behavior and return 'bool'.
In case we ever re-add a cache, we *must* not make any cache-creation
errors fatal. Instead, we should simply skip caching and still return the
correct policy decision.
This is really important, because deep down in the policy-db we cannot
make any reasonable choice on error codes. For instance, we now return
ENOENT for some policy-decisions in case we don't want to leak information
to custom endpoints that cannot see the peer. However, we actually return
ESRCH in we cannot find a name later. This is really hard to track down
because both errors are generated in totally different code locations.
This commit does not fix those, it only turns the 'int' into a 'bool'.
Follow-ups will fix the error-code mess.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 16:44:36 +0000 (17:44 +0100)]
names: fix coding-style
Don't split lines needlessly. If we drop the q_ prefix, we can properly
get the whole foreach statement into one line.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 16:36:02 +0000 (17:36 +0100)]
names: skip policy-check on NAME_RELEASE
If user-space requests to release a name, we *must* grant that. Otherwise,
a policy-change after the name was acquired might prevent the user from
releasing it.
Furthermore, there really is no reason to check policy to release
_acquire_ resources. So drop that.
However, make sure to return ESRCH regardless whether the name is un-owned
or owned by someone else. We must not leak information about a name in
case the caller cannot see it. Furthermore, it's really not our business
to return information on a name when someone tries to release it. The
information that they don't own this resource should be more than enough.
I'd be even inclined to return "0" in those cases...
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 16:21:10 +0000 (17:21 +0100)]
policy: drop kdbus_policy_check_talk_access()
This helper is now redundant. We already have the same iterator in
connection.c. Move it into its own function and then drop the
policy-helper.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 15:38:34 +0000 (16:38 +0100)]
purge: drop talk cache
Once we use current->creds as policy credentials for UNICASTS, and
file->f_creds for receiver-based policy checks, we cannot use the cache as
we have it now. The creds may change at any time and it's not clear which
creds were used for a given policy check.
Furthermore, the current cache is highly inefficient. It uses only 64
buckets for the whole bus cache. It needs exclusive locking for lookups
and requires _full_ cache traversals whenever a name changes ownership, a
connection dies or a policy is changed.
Drop the cache for now.
Once our code is considered ready and we do benchmarking, we can re-add
it. However, I really think it should look more like this:
Each *connection* has a policy-cache. This caches the results of the
respective kdbus_policy_query() calls so we can skip calling into the
policy (and thus skip locking the policy/bus) on cache hits. On cache
misses, we call into the policy and cache the result with a back-link
to the policy-entry that produced this result. Whenever the policy-db
changes, it drops all cache entries that were generated by a given
policy-entry.
As a bonus, our cache can be based on conn->lock. There is no need to
lock all caches. Furthermore, the policy-db can be updated to use RCU
as policy-db modifications are definitely not considered a fast-path.
We then can synchronize on policy-updates and get a lock-free policy
db.
Anyway, unless someone does some benchmarking, it seems rather useless to
create policy-caches. Lets drop it for now, get everything in place
underneath and re-add it later, if needed (and yeah, chances are high that
some sort of optimization is needed).
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 15:37:43 +0000 (16:37 +0100)]
connection: drop unused includes/declarations
Drop the dead kdbus_conn_namespace_eq() declaration and any namespace
includes. Add them to metadata.c where they're actually used.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 14:30:57 +0000 (15:30 +0100)]
policy: speed-up and simplify query helpers
Instead of hard-coding each policy-db query type, this commit provides
kdbus_policy_query() and kdbus_policy_query_unlocked(). So far, they're
only used for SEE and OWN, but TALK can also be converted.
The policy DB is seen as generic entity from the outside, so we should
push any non-policy decisions into it. The authority on the policy
application is the bus, the endpoint or the connection, so keep it there.
Furthermore, we now pass the name-hash into the function, so we don't have
to hash multiple times if we query multiple DBs like with custom
endpoints.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 13:52:06 +0000 (14:52 +0100)]
connection: re-write notification policy
Rewrite the notification policy check and add justifications why the
different types are dropped. Also add proper WARN()s instead of silently
rejecting invalid callers.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 12:26:04 +0000 (13:26 +0100)]
connection: kdbus_ep_policy_check_src_names() -> kdbus_conn_policy_see()
Same as the previous policy helpers, move SRC-NAME verification to
connection.c. Furthermore, rename the function to a subject-based name:
kdbus_conn_policy_see(conn, whom)
It now verifies whether @conn can see @whom, which is what we really want
to know. The fact that this relies on the destination names is irrelevant
for the call-side. Policy is based on SEE policies, so let it look like it
really is.
This again allows us to drop the 'ep' argument as it is implicitly given
by the subject 'conn'.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 11:51:49 +0000 (12:51 +0100)]
connection: dont lock subjects on SEE policy check
There is no need to lock the source connection when checking SEE policies.
The only reason connection locks are needed is if we iterate the target
names. We never do that during simple SEE policy checks (but only during
full target connection SEE checks), so drop that lock.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 11:50:36 +0000 (12:50 +0100)]
connection: kdbus_ep_policy_check_see_access* -> kdbus_conn_policy_see_name()
Same as the other policy helpers, move SEE verification to connection.c
and drop the endpoint argument. It's always implicitly given via the
connection.
Also follow the easier naming style that does not clash with policy.c.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 11:27:37 +0000 (12:27 +0100)]
connection: kdbus_ep_policy_check_talk_access() -> kdbus_conn_policy_talk()
Same as for kdbus_conn_policy_own_name(), move the policy helper where it
belongs. Also drop the 'ep' argument as it is redundant.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 11:16:37 +0000 (12:16 +0100)]
connection: drop superfluous argument to kdbus_conn_check_access()
The endpoint is always the endpoint of the source connection. Drop it and
re-order arguments to more reflect the functions nature.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 11:14:03 +0000 (12:14 +0100)]
connection: kdbus_conn_kmsg_send() -> kdbus_cmd_msg_send()
The _kmsg_send() function is no longer used for non-ioctl messages, so we
can mark it as command dispatcher now, like all the other kdbus_cmd_msg_*
helpers we already have.
This also allows us to drop the "ep" argument, as we can safely use
conn_src->ep now.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 19 Dec 2014 11:08:28 +0000 (12:08 +0100)]
connection: kdbus_ep_policy_check_own_access() -> kdbus_conn_policy_own_name()
The OWN access check is not a property of an endpoint, but of a
connection. Move the function to connection.c and drop the "endpoint"
argument.
Furthermore, prepare for further renamed and use the common
kdbus_conn_policy() prefix.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Djalal Harouni [Thu, 18 Dec 2014 15:25:55 +0000 (16:25 +0100)]
connection: sync send do not block SIGKILL or SIGSTOP
SIGKILL and SIGSTOP can't be blocked so clear them out before applying
the sigmask.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Daniel Mack [Thu, 18 Dec 2014 15:06:03 +0000 (16:06 +0100)]
kdbus.txt: document KDBUS_ITEM_SIGMASK
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Thu, 18 Dec 2014 15:05:09 +0000 (16:05 +0100)]
connection: check for invalid items in kdbus_conn_kmsg_send()
Iterate the items and reject everything but KDBUS_ITEM_SIGMASK.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Thu, 18 Dec 2014 15:04:05 +0000 (16:04 +0100)]
connection: kdbus_conn_kmsg_send(): cmd_send cannot be NULL
There's only one caller of kdbus_conn_kmsg_send(), and it ensures
that cmd_send is never NULL. No need to check or allow that.
While at it, shorten the name to 'cmd'.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Thu, 18 Dec 2014 14:57:34 +0000 (15:57 +0100)]
connection: handle -EINTR correctly
When our sync send call is interrupted, we need to store our saved
sigmask to current->saved_sigmask and call set_restore_sigmask().
Signed-off-by: Daniel Mack <daniel@zonque.org>
Djalal Harouni [Thu, 18 Dec 2014 00:45:27 +0000 (01:45 +0100)]
connection: fix BUG: unable to handle kernel paging request at
ffffffffffffffc6
Fix the following oops triggered by the sync send. When fetching the
SIGMASK kdbus_items_get() will return an ERR_PTR not NULL, so just check
it and update the item to be NULL again in case of errors.
Note that the code should validate if the SIGMASK was passed two times
or more in the cmd_send and fail with -EEXIST before reaching this call
site.
[ 153.671313] BUG: unable to handle kernel paging request at
ffffffffffffffc6
[ 153.672278] IP: [<
ffffffffa0298520>] kdbus_conn_kmsg_send+0xb40/0xc30 [kdbus]
[ 153.672286] PGD
1e17067 PUD
1e19067 PMD 0
[ 153.672286] Oops: 0000 [#1] SMP
[ 153.672286] Modules linked in: kdbus(OE) nf_conntrack_netbios_ns nf_conntrack_broadcast ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_mangle ip6table_raw ip6table_filter ip6_tables iptable_mangle iptable_raw ppdev parport_pc parport i2c_piix4 microcode 8139too serio_raw bochs_drm drm_kms_helper ttm drm 8139cp ata_generic mii i2c_core pata_acpi
[ 153.672286] CPU: 0 PID: 1538 Comm: kdbus-test Tainted: G OE 3.18.0 #4
[ 153.672286] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 153.672286] task:
ffff880068d04ce0 ti:
ffff880068a40000 task.ti:
ffff880068a40000
[ 153.672286] RIP: 0010:[<
ffffffffa0298520>] [<
ffffffffa0298520>] kdbus_conn_kmsg_send+0xb40/0xc30 [kdbus]
[ 153.672286] RSP: 0018:
ffff880068a43d98 EFLAGS:
00010246
[ 153.672286] RAX:
0000000000000000 RBX:
ffff88006795a800 RCX:
0000000000000000
[ 153.672286] RDX:
0000000000000286 RSI:
ffffffffa02a5040 RDI:
ffffffffa02a5c1f
[ 153.672286] RBP:
ffff880068a43e58 R08:
0000000000000001 R09:
ffff88006800bc60
[ 153.672286] R10:
0000000000000001 R11:
0000000000aaaaaa R12:
ffff88006795a000
[ 153.672286] R13:
ffff880069d3c000 R14:
ffffffffffffffb6 R15:
ffff88006800b360
[ 153.672286] FS:
00007fd883ce1740(0000) GS:
ffff88006d200000(0000) knlGS:
0000000000000000
[ 153.672286] CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
[ 153.672286] CR2:
ffffffffffffffc6 CR3:
00000000689b1000 CR4:
00000000000006f0
[ 153.672286] Stack:
[ 153.672286]
ffff880068a43e58 0000000000003fff ffffffffffffffb6 ffff88006800bc60
[ 153.672286]
0000000129e97e4b 0000000000000000 ffff8800698d8470 0000000000000000
[ 153.672286]
ffff8800698d84b0 ffff880069d3c000 0000000000000099 0000000027a201d8
[ 153.672286] Call Trace:
[ 153.672286] [<
ffffffffa029bad0>] handle_ep_ioctl+0xcc0/0xef0 [kdbus]
[ 153.672286] [<
ffffffff811efb19>] ? shmem_add_seals+0x89/0x8a0
[ 153.672286] [<
ffffffff811efb78>] ? shmem_add_seals+0xe8/0x8a0
[ 153.672286] [<
ffffffff8126e8d0>] do_vfs_ioctl+0x300/0x520
[ 153.672286] [<
ffffffff8111d37e>] ? rcu_read_lock_held+0x6e/0x80
[ 153.672286] [<
ffffffff8127ad9e>] ? __fget_light+0xbe/0xe0
[ 153.672286] [<
ffffffff8126eb71>] SyS_ioctl+0x81/0xa0
[ 153.672286] [<
ffffffff817fcfa9>] system_call_fastpath+0x12/0x17
[ 153.672286] Code: c3 00 00 e9 4e f6 ff ff 4c 8b b5 50 ff ff ff ba 86 02 00 00 48 c7 c6 40 50 2a a0 48 c7 c7 1f 5c 2a a0 31 c0 4c 89 8d 58 ff ff ff <49> 8b 4e 10 e8 b3 65 55 e1 ba 87 02 00 00 48 c7 c6 40 50 2a a0
[ 153.672286] RIP [<
ffffffffa0298520>] kdbus_conn_kmsg_send+0xb40/0xc30 [kdbus]
[ 153.672286] RSP <
ffff880068a43d98>
[ 153.672286] CR2:
ffffffffffffffc6
[ 153.672286] ---[ end trace
3d9e048aa5474063 ]---
[ 153.672286] BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:41
[ 153.672286] in_atomic(): 0, irqs_disabled(): 1, pid: 1538, name: kdbus-test
[ 153.672286] INFO: lockdep is turned off.
[ 153.672286] irq event stamp: 1350
[ 153.672286] hardirqs last enabled at (1349): [<
ffffffff81114205>] vprintk_emit+0x245/0x620
[ 153.672286] hardirqs last disabled at (1350): [<
ffffffff817ff206>] error_sti+0x5/0x6
[ 153.672286] softirqs last enabled at (824): [<
ffffffff810ab358>] __do_softirq+0x388/0x660
[ 153.672286] softirqs last disabled at (771): [<
ffffffff810ab975>] irq_exit+0xc5/0xd0
[ 153.672286] CPU: 0 PID: 1538 Comm: kdbus-test Tainted: G D OE 3.18.0 #4
[ 153.672286] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 153.672286]
0000000000000000 00000000076526b1 ffff880068a43968 ffffffff817f28fd
[ 153.672286]
0000000000000000 0000000000000000 ffff880068a43998 ffffffff810d3fca
[ 153.672286]
0000000000000000 ffff880069049878 ffff8800690498e8 ffff880068d04ce0
[ 153.672286] Call Trace:
[ 153.672286] [<
ffffffff817f28fd>] dump_stack+0x4e/0x68
[ 153.672286] [<
ffffffff810d3fca>] __might_sleep+0x18a/0x240
[ 153.672286] [<
ffffffff817fa35a>] down_read+0x2a/0xa0
[ 153.672286] [<
ffffffff810b8a33>] exit_signals+0x33/0x150
[ 153.672286] [<
ffffffff810a8400>] do_exit+0xd0/0xc50
[ 153.672286] [<
ffffffff811156fe>] ? kmsg_dump+0x16e/0x220
[ 153.672286] [<
ffffffff811155c4>] ? kmsg_dump+0x34/0x220
[ 153.672286] [<
ffffffff81021a70>] oops_end+0xa0/0xe0
[ 153.672286] [<
ffffffff817ecd8a>] no_context+0x2f4/0x371
[ 153.672286] [<
ffffffff817ece85>] __bad_area_nosemaphore+0x7e/0x1d7
[ 153.672286] [<
ffffffff817ecff1>] bad_area_nosemaphore+0x13/0x15
[ 153.672286] [<
ffffffff81069852>] __do_page_fault+0xc2/0x5f0
[ 153.672286] [<
ffffffff810f9b86>] ? up+0x16/0x50
[ 153.672286] [<
ffffffff810fb3bf>] ? __lock_is_held+0x5f/0x90
[ 153.672286] [<
ffffffff81069e18>] trace_do_page_fault+0x68/0x400
[ 153.672286] [<
ffffffff81064e2a>] do_async_page_fault+0x1a/0x80
[ 153.672286] [<
ffffffff817fefe8>] async_page_fault+0x28/0x30
[ 153.672286] [<
ffffffffa0298520>] ? kdbus_conn_kmsg_send+0xb40/0xc30 [kdbus]
[ 153.672286] [<
ffffffffa0297ce5>] ? kdbus_conn_kmsg_send+0x305/0xc30 [kdbus]
[ 153.672286] [<
ffffffffa029bad0>] handle_ep_ioctl+0xcc0/0xef0 [kdbus]
[ 153.672286] [<
ffffffff811efb19>] ? shmem_add_seals+0x89/0x8a0
[ 153.672286] [<
ffffffff811efb78>] ? shmem_add_seals+0xe8/0x8a0
[ 153.672286] [<
ffffffff8126e8d0>] do_vfs_ioctl+0x300/0x520
[ 153.672286] [<
ffffffff8111d37e>] ? rcu_read_lock_held+0x6e/0x80
[ 153.672286] [<
ffffffff8127ad9e>] ? __fget_light+0xbe/0xe0
[ 153.672286] [<
ffffffff8126eb71>] SyS_ioctl+0x81/0xa0
[ 153.672286] [<
ffffffff817fcfa9>] system_call_fastpath+0x12/0x17
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Daniel Mack [Wed, 17 Dec 2014 22:29:57 +0000 (23:29 +0100)]
Changelog: comment CANCEL and SIGMASK changes
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Wed, 17 Dec 2014 22:20:21 +0000 (23:20 +0100)]
kdbus.h: add KDBUS_ITEM_SIGMASK (ABI break)
Introduce KDBUS_ITEM_SIGMASK to store a 64-bit signal mask.
If attached to KDBUS_CMD_SEND in sync mode, the provided signal mask
will be set before the kernel blocks the execution, and restored
afterwards, unless the syscall is interrupted. This makes the
KDBUS_CMD_SEND ioctl behavior similar to what ppoll() does.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Djalal Harouni [Wed, 17 Dec 2014 20:31:31 +0000 (21:31 +0100)]
test: when starting tests print the attach flags mask
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 17 Dec 2014 20:18:10 +0000 (21:18 +0100)]
test: tree-wide check if all uids and gids are mapped in current userns
Tests that are running in user namespaces where not all the uids and
gids are mapped will be SKIPPED automatically, this is the case of
unprivileged which turn privileged in its userns.
We skip test since even if we have CAP_SETUID we also need the target
uid to be mapped in that userns. Well, we do not need all the uids but
we are just lazy here...
The ones that do not like CONFIG_USER_NS can also test kdbus as it was
in a container, running under root:
# ./test/kdbus-test --mntns --pidns
With this change unprivileged running on kernel with CONFIR_USER_NS can run
kdbus tests:
$ ./test/kdbus-test --userns --uidns "0 1000 1" --gidns "0 1000 1" --mntns --pidns
(but some tests are skipped)
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 17 Dec 2014 20:39:01 +0000 (21:39 +0100)]
test-activator: check if all uids/gids are mapped in
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 17 Dec 2014 19:47:54 +0000 (20:47 +0100)]
test-attach: fix attach-flags so we can SKIP it in unprivleged userns
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 17 Dec 2014 19:20:09 +0000 (20:20 +0100)]
test: allow our tests to run in a namespaced setup
Tests can take run now in their namespaced setup, if the --mntns switch
is given a new mount namespace is created and kdbusfs is remounted in
that namespace.
Currently all tests succeed, only when we combine the userns switch some
test will fail the ones that call setuid(), we have to make sure that
the uid/gid are mapped in the userns, next patch will fix the ones that
fail.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 17 Dec 2014 20:36:17 +0000 (21:36 +0100)]
test: preparation support for a namespaced test setup
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Daniel Mack [Wed, 17 Dec 2014 16:41:18 +0000 (17:41 +0100)]
kdbus.h: remove KDBUS_CMD_CANCEL
Kill KDBUS_CMD_CANCEL. The API wasn't thought to the end, as it cannot
be used in a race-free way from a threaded user-space environment.
Instead, we have to implement a signal mask that is installed before
the waitqueue is entered.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Wed, 17 Dec 2014 16:35:59 +0000 (17:35 +0100)]
connection: kdbus_conn_reply_unref() can take NULL pointers
No need to check the parameter prior to calling that function.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Wed, 17 Dec 2014 16:29:38 +0000 (17:29 +0100)]
connection: return -ETIMEDOUT immediately if msg->timeout_ns <= now
If the timeout of a message has already exceeded at the time of
sending a message, there's no point in going to sleep. Return
-ETIMEDOUT immediately in such cases.
While at it, move the !reply_wait check to kdbus_conn_wait_reply().
Signed-off-by: Daniel Mack <daniel@zonque.org>
Rui Miguel Silva [Wed, 17 Dec 2014 15:45:47 +0000 (15:45 +0000)]
match: init list rule so we can remove it on error
initialise the rules_entry so we don't oops on error paths when the rule is not
in list yet.
Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
Rui Miguel Silva [Wed, 17 Dec 2014 14:04:19 +0000 (14:04 +0000)]
test: fix usage print endless loop
do not use the some iterator in nested loops
Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
Daniel Mack [Wed, 17 Dec 2014 15:01:15 +0000 (16:01 +0100)]
names: use WARN_ON() rather than BUG_ON()
Signed-off-by: Daniel Mack <daniel@zonque.org>
Daniel Mack [Wed, 17 Dec 2014 12:31:22 +0000 (13:31 +0100)]
bus.c: use WARN_ON() rather than BUG_ON()
Signed-off-by: Daniel Mack <daniel@zonque.org>