Daniel Mack [Fri, 15 Aug 2014 19:45:10 +0000 (21:45 +0200)]
connection, metadata: convert auxgrp information as late as possible
Same as with the cred block - do the from_kgid() conversion shortly before
installing the message in the receiver's task.
Daniel Mack [Fri, 15 Aug 2014 17:46:10 +0000 (19:46 +0200)]
metadata: allow NULL pointers in kdbus_meta_append_data()
Allow NULL pointers to be passed as data argument to
kdbus_meta_append_data(), so users can use the function to only make space
in the metadata without actually copying any payload.
Daniel Mack [Fri, 15 Aug 2014 17:22:43 +0000 (19:22 +0200)]
connection, metadata: do namespace translation as late as possible
We have to do the namespace translation just before a message is
installed into the receiver's task, so we actually know about the
currently used pid and user namespaces.
To achive this, store the byte offset pointer in the message in
kdbus_conn_queue_alloc(), and patch the values over from
kdbus_conn_msg_install().
Thanks to Djalal Harouni for pointing this out.
Daniel Mack [Fri, 15 Aug 2014 17:20:51 +0000 (19:20 +0200)]
metadata: add kdbus_meta_offset_of()
kdbus_meta_offset_of() will iterate over the given metadata and return
the offset of the item of type @type, if it exists.
Daniel Mack [Thu, 14 Aug 2014 20:20:54 +0000 (22:20 +0200)]
policy, connection: rename policy cache related functions
Just so they reflect a little better what they're doing.
Daniel Mack [Thu, 14 Aug 2014 20:14:49 +0000 (22:14 +0200)]
policy: rename send_access_hash to talk_access_hash
We've already renamed the SEND policy to TALK for more clarity,
so let's do the same with the access hash.
Daniel Mack [Thu, 14 Aug 2014 14:36:40 +0000 (16:36 +0200)]
names: flush policy TALK cache when a connection loses a name
When a connection loses a name, make sure cache entries that allow
TALK access to it are properly removed. Upon the next TALK query,
the connection's names will be iterated and checked again.
Daniel Mack [Thu, 14 Aug 2014 14:14:33 +0000 (16:14 +0200)]
test: test-kdbus: fix monitor and byebye tests
Since commit
7015a1e6 ("prevent special-purpose connections from
owning names or exchanging messages"), ioctl() calls return
-EOPNOTSUPP they are issued to a handle of a inapproprate type.
Follow this change in the test as well.
Alban Crequy [Thu, 14 Aug 2014 11:13:46 +0000 (12:13 +0100)]
bus.h: add missing include to linux/kref.h
Symptoms:
> kdbus/bus.h:56:14: error: field ‘kref’ has incomplete type
> struct kref kref;
Signed-off-by: Alban Crequy <alban.crequy@collabora.co.uk>
Daniel Mack [Tue, 12 Aug 2014 13:02:31 +0000 (15:02 +0200)]
pool: remove unneeded semicolon
Daniel Mack [Sat, 9 Aug 2014 13:00:20 +0000 (15:00 +0200)]
metadata: return -EFAULT if get_task_mm(current) fails
Daniel Mack [Sat, 9 Aug 2014 12:13:57 +0000 (14:13 +0200)]
test: add test code for KDBUS_ITEM_AUXGROUPS
Daniel Mack [Sat, 9 Aug 2014 12:12:51 +0000 (14:12 +0200)]
metadata: add new item type KDBUS_ITEM_AUXGROUPS (ABI break)
Add new data type to dump the auxiliary groups the sending task is a member
of.
Djalal Harouni [Tue, 5 Aug 2014 01:46:08 +0000 (02:46 +0100)]
test: split conn_update() into update attach-flags and update policy
Since ordinary connections are only interested in the attach-flags and
policy holders in policies, split conn_update() into:
1) conn_update_attach_flags()
2) conn_update_policy()
This way we use the conn_update_policy() function in test-kdbus-policy
with a policy-holding connection and we pass all the tests. This
prevents messing up with the attach-flags.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Tue, 5 Aug 2014 01:46:07 +0000 (02:46 +0100)]
connection: improve kdbus_cmd_conn_update() connection type checks
Do another round of connection type checks inside the KDBUS_ITEM
iterator.
We need this since we do not want to allow ordinary connections to
update policy entries that belong to another policy holder connection.
We also do it for the attach flags since only ordinary connections are
interessted in it.
And update a kdbus_policy_set() call to only pass a one name per
policy-holding connection
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Tue, 5 Aug 2014 01:46:06 +0000 (02:46 +0100)]
handle: allow KDBUS_CMD_CONN_UPDATE ioctl for policy holders
Allow KDBUS_CMD_CONN_UPDATE for KDBUS_CONN_POLICY_HOLDER connections.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Tue, 5 Aug 2014 01:09:09 +0000 (02:09 +0100)]
handle: return -EOPNOTSUPP instead of -EPERM if an operation is not supported
If userspace calls in with the wrong connection type, just return
-EOPNOTSUPP instead of -EPERM.
This will not confuse unprivileged and privileged processes, and permits
to identify legitimate -EPERM errors.
This just converts errors introduced in commit
7015a1e6746
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Tue, 5 Aug 2014 01:09:08 +0000 (02:09 +0100)]
test: sync the policy tests with the recent activators and policy holders changes
Recent commit
7015a1e6746e0c2 prevents special-purpose connections from
owning names, so update the test-kdbus-policy tests to follow and test
these changes.
Create a new policy holder connection which will register the policy for
an X name, and make the first conn_db[0] connection own that name.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Kay Sievers [Mon, 4 Aug 2014 14:18:08 +0000 (16:18 +0200)]
use conn->type instead of conn->flags where possible
Kay Sievers [Mon, 4 Aug 2014 14:05:39 +0000 (16:05 +0200)]
prevent special-purpose connections from owning names or exchanging messages
Kay Sievers [Thu, 31 Jul 2014 21:02:54 +0000 (23:02 +0200)]
kdbus.h: clarify policy holder docs
Kay Sievers [Thu, 31 Jul 2014 15:15:13 +0000 (17:15 +0200)]
do not use __ prefix for ordinary functions
Kay Sievers [Thu, 31 Jul 2014 14:59:27 +0000 (16:59 +0200)]
domain: add BUG_ON()
Kay Sievers [Thu, 31 Jul 2014 14:52:50 +0000 (16:52 +0200)]
do not use __ prefix for ordinary functions
Kay Sievers [Thu, 31 Jul 2014 14:51:08 +0000 (16:51 +0200)]
test: do not use __ prefix for ordinary functions
Djalal Harouni [Wed, 30 Jul 2014 20:11:58 +0000 (21:11 +0100)]
domain: remove dead kdbus_domain_user_find_or_new()
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 30 Jul 2014 20:11:57 +0000 (21:11 +0100)]
connection: fix user quota accounting corruption
First use kzalloc to allocate the users array, so we do not reference
unintialized values.
And free the old conn->msg_users array not the newly allocated 'users'
one.
Patch tested, and users will hit the KDBUS_CONN_MAX_MSGS_PER_USER limit
and fail with -ENOBUFS
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 30 Jul 2014 20:11:56 +0000 (21:11 +0100)]
bus: improve user quota accounting and domain locking
Currently kdbus_bus_new() execution path might take the domain lock
three times on success, four times on failure.
kdbus_bus_new():
=> kdbus_domain_user_find_or_new(): takes it 2 times (and it is racy)
+
kdbus_bus_new(): take it an extra time to account the user and link
the bus into the domain bus_list.
And as discussed in the previous patch kdbus_domain_user_find_or_new()
is racy, so convert to __kdbus_domain_user_account()
This allows also to improve the execution path to take the domain lock
only one time in case of success. On failure the domain lock will be
taken two times.
kdbus_bus_new():
=> take domain lock
=> check if domain is still active/connected
=> __kdbus_domain_user_account()
...
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 30 Jul 2014 20:11:55 +0000 (21:11 +0100)]
kdbus: improve user quota accounting by using kdbus_domain_user_account()
Currently kdbus_domain_user_find_or_new() is used to find a user
domain or create a new one and link it into the domain.
kdbus_domain_user_find_or_new() may fail due to memory allocation
errors or if the domain was shutdown, but since callers will
receive only a NULL pointer on failure, they assume -ENOMEM and
ignore -ESHUTDOWN. Fix this in kdbus_domain_user_account() by returning
the appropriate error code.
There are also some races with kdbus_domain_user_find_or_new(), if it is
called with the same parameters and if we do not find a previously
linked domain user, then both threads will race to assign a different ID
for the same uid, thus, invalidating the users array. We fix this in the
new kdbus_domain_user_account() and __kdbus_domain_user_account() by
taking the domain lock only one time.
Replace some kdbus_domain_user_find_or_new() calls with
kdbus_domain_user_account(). The last one in bus.c is updated in the
next patch.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 30 Jul 2014 20:11:54 +0000 (21:11 +0100)]
domain: add kdbus_domain_user_account()
Add kdbus_domain_user_account() to account and link users into a
domain.
This function will take the domain lock, and it will be used as a
replacement for kdbus_domain_user_find_or_new().
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 30 Jul 2014 20:11:53 +0000 (21:11 +0100)]
domain: add __kdbus_domain_user_account() to account domain users
Add __kdbus_domain_user_account() to account and link users into a
domain.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 30 Jul 2014 20:11:52 +0000 (21:11 +0100)]
domain: add kdbus_domain_user_assign_id() to assign IDs to domain users
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 23 Jul 2014 16:34:32 +0000 (17:34 +0100)]
test: correctly set the 'ret' variable
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Greg Kroah-Hartman [Wed, 2 Jul 2014 05:46:39 +0000 (22:46 -0700)]
Merge pull request #21 from torstehu/fix-warning
Fix annoying compile warning due to missing cast
Torstein Husebø [Sun, 29 Jun 2014 15:41:31 +0000 (17:41 +0200)]
Fix annoying compile warning due to missing cast
Daniel Mack [Tue, 24 Jun 2014 13:41:47 +0000 (15:41 +0200)]
connection: don't append to uninitialized metadata
kmsg->meta is only valid if conn_src != NULL.
Daniel Mack [Tue, 24 Jun 2014 09:24:49 +0000 (11:24 +0200)]
connection: clean up conditional code path
For broadcasts, we return anway, so there's no point in an 'else' branch.
Daniel Mack [Tue, 24 Jun 2014 09:15:48 +0000 (11:15 +0200)]
connection: rename kdbus_check_send_perm() to kdbus_conn_check_access()
Keep the function namespace clean.
Daniel Mack [Mon, 23 Jun 2014 17:08:27 +0000 (19:08 +0200)]
connection: factor out code into kdbus_conn_add_expected_reply()
This helps cutting down kdbus_conn_kmsg_send() to make it more readable.
Daniel Mack [Mon, 23 Jun 2014 16:44:30 +0000 (18:44 +0200)]
connection: factor out some code into kdbus_check_send_permission()
This help cutting down the size of kdbus_conn_kmsg_send(), which has grown
too big.
Daniel Mack [Mon, 23 Jun 2014 15:54:59 +0000 (17:54 +0200)]
memfd: switch to f_op->{read,write}_iter
These functions are new since 3.16 and replace the old aio_{read,write}
implementations.
Djalal Harouni [Fri, 20 Jun 2014 16:50:03 +0000 (17:50 +0100)]
policy: kdbus_policy_set() use another variable to save entries
In kdbus_policy_set() function, we use the 'e' variable to reference
each entry of the 'db->entries_hash', so at the end the variable 'e' will
for sure point to a valid one.
Next in the KDBUS_ITEMS_FOREACH() iterator and if we fail at the first
KDBUS_ITEM_VALID() test, we jmp to exit:
Which contains the following:
if (e)
kdbus_policy_entry_free(e);
Here 'e' points to a valid entry and it will be freed, so even we
restore all the other entries from that list, there will be always one
missing, the last one pointed by that 'e' variable.
To fix this, just use another 'tmp_entry' variable to reference hash
entries.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Fri, 20 Jun 2014 16:50:01 +0000 (17:50 +0100)]
policy: use the db->entries_hash to access the policy db entries
Use the db->entries_hash to access the policy db entries instead of the
db->send_access_hash which is just a cache for send entries.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Fri, 20 Jun 2014 16:49:59 +0000 (17:49 +0100)]
test: add the test-kdbus-policy test
Add the test-kdbus-policy that performs:
1) Register a policy holder
2) Try to register the same name as an activator:
kdbus will break here due to a corrupted db->entries_hash
in kdbus_policy_set().
Will be fixed in the next patches.
3) Acquire the name for the policy holder
4) Create and test the connections
5) Fork and drop privileges, then create and test connections which
should be cached in the send cache after the tests.
Here we inspect the send cache and we have located several bugs,
which will be fixed in the next patches.
6) Call kdbus_set_policy_talk() to test KDBUS_CMD_CONN_UPDATE ioctl
and restrict TALK access to KDBUS_POLICY_ACCESS_USER
7) Redo test 5), now connections should all fail with -EPERM since
TALK access was restricted.
To test this we need the right capabilities to perform the setuid() and
drop privileges, so this test just check if it was exec by root.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Fri, 20 Jun 2014 16:49:58 +0000 (17:49 +0100)]
test: add conn_update() to test KDBUS_CMD_CONN_UPDATE
Add conn_update() to perform KDBUS_CMD_CONN_UPDATE ioctl() calls.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Fri, 20 Jun 2014 16:49:57 +0000 (17:49 +0100)]
test: add simple helper to drop privileges
This is needed since we will add tests to fork() + drop privileges
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Fri, 20 Jun 2014 16:49:56 +0000 (17:49 +0100)]
test: make msg_send() return -errno
Make msg_send() return negative errno not EXIT_FAILURE. We need this
since we will add tests that fork() and drop privileges and we want all
the error codes, especially -EPERM.
After a quick grep it seems the return value of msg_send() is never
used, so it is safe to convert it, this wont break any test.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Fri, 20 Jun 2014 16:49:55 +0000 (17:49 +0100)]
test: export kdbus_hello_registrar()
Export this function since we will use it to register policy holders
connections.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Fri, 20 Jun 2014 16:50:06 +0000 (17:50 +0100)]
connection: when freeing a connection purge its cached entries
When freeing a connection remove also all the cached entries related
to this connection, otherwise if we access this cached entry through
another valid connection, we will hit memory corruption bugs.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Fri, 20 Jun 2014 16:50:05 +0000 (17:50 +0100)]
policy: kdbus_policy_check_own_access() returns 0 on success not true
kdbus_policy_check_own_access() returns 0 if access is granted,
otherwise a negative errno.
So fix this by returning 0. We did not hit this since callers were
checking negative values for errors.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Fri, 20 Jun 2014 16:50:04 +0000 (17:50 +0100)]
policy: kdbus_policy_set() fix a use after free bug
If we try to register the same name twice then kdbus_policy_add_one()
will fail with -EEXIST
In kdbus_policy_set() we have two calls to kdbus_policy_add_one() if
they fail we clean things up with kdbus_policy_entry_free(), but since
we failed ret == -EEXIST ,we hit the error path where we have another:
if (e)
kdbus_policy_entry_free(e);
We have a use after free bug here, Since 'e' is freed but never set to
NULL.
To fix this we can set that 'e' to NULL after each
kdbus_policy_entry_free() call, but it is better to just clean things up
in a one place, in the error path and remove the other
kdbus_policy_entry_free() calls.
Thix fixes the bug triggered by test-kdbus-policy when we try to
register the same name twice.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Fri, 20 Jun 2014 16:50:02 +0000 (17:50 +0100)]
policy: kdbus_policy_set() make sure we restore the right entries
If kdbus_policy_set() fails we try to restore the entries that were
previously saved in a list, however due to a typo that logic was trying
to access a previously freed entry which will just break things...
So fix the typo 'l->e' instead of 'e'.
This fixes a bug triggered by test-kdbus-policy and makes the code able
to restore previously saved entries in case of errors.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Fri, 20 Jun 2014 16:50:00 +0000 (17:50 +0100)]
connection: update attach_flags only if KDBUS_ITEM_ATTACH_FLAGS is provided
Fix a bug introcuded in commit
d92d68414fab which fixed another bug.
conn->attach_flags should only be update if KDBUS_ITEM_ATTACH_FLAGS was
provided.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Simon Peeters [Tue, 17 Jun 2014 21:18:24 +0000 (23:18 +0200)]
ease installation on non-running kernels
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Kay Sievers [Tue, 17 Jun 2014 16:17:34 +0000 (18:17 +0200)]
policy: pass items, not &items
Kay Sievers [Tue, 17 Jun 2014 16:17:01 +0000 (18:17 +0200)]
util.h: KDBUS_ITEM_VALID - fix size check
Jacek Janczyk [Tue, 17 Jun 2014 10:38:38 +0000 (12:38 +0200)]
util.h: KDBUS_ITEM_VALID fixed
Missing character leading to wrong variable name restored.
Signed-off-by: Jacek Janczyk <j.janczyk@samsung.com>
Djalal Harouni [Wed, 11 Jun 2014 16:27:59 +0000 (17:27 +0100)]
kdbus-test: fix kdbus item alignment in kdbus_hello_registrar()
Currently running the test-kdbus-activator test will fail with -EINVAL
To fix this, remove the redundant offsetof() macros since it is already
handled. The KDBUS_ITEM_SIZE() will expand into KDBUS_ITEM_HEADER_SIZE()
which expands into an offsetof() one.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Wed, 11 Jun 2014 16:27:58 +0000 (17:27 +0100)]
connection: fix cpu stall by checking kdbus item alignment
Fix the following stall triggered by a policy holder hello:
call test/kdbus-util.c:kdbus_hello_registrar() and pass
KDBUS_HELLO_POLICY_HOLDER as a flag.
While we are it make sure that kdbus_cmd_conn_update() also checks for
proper 8 byte alignment.
[ 142.731011] INFO: rcu_sched self-detected stall on CPU { 3} (t=65000 jiffies g=3085 c=3084 q=4316)
[ 142.731011] sending NMI to all CPUs:
[ 142.731011] NMI backtrace for cpu 3
[ 142.731011] CPU: 3 PID: 1352 Comm: test-kdbus-poli Tainted: G OE 3.15.0-0.rc8.git4.1.fc21.x86_64 #1
[ 142.731011] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 142.731011] task:
ffff88005a2719d0 ti:
ffff88004587c000 task.ti:
ffff88004587c000
[ 142.731011] RIP: 0010:[<
ffffffff81055a12>] [<
ffffffff81055a12>] flat_send_IPI_all+0x92/0xd0
[ 142.731011] RSP: 0018:
ffff88005dc03dc0 EFLAGS:
00010006
[ 142.731011] RAX:
0000000000000000 RBX:
0000000000000c00 RCX:
0000000000000000
[ 142.731011] RDX:
0000000000000c00 RSI:
0000000000000000 RDI:
0000000000000300
[ 142.731011] RBP:
ffff88005dc03dd8 R08:
0000000000000001 R09:
0000000000000001
[ 142.731011] R10:
ffffffff81e5d720 R11:
0000000000000001 R12:
0000000000000046
[ 142.731011] R13:
000000000000000f R14:
ffffffff81e86880 R15:
0000000000000003
[ 142.731011] FS:
00007f4b9d727740(0000) GS:
ffff88005dc00000(0000) knlGS:
0000000000000000
[ 142.731011] CS: 0010 DS: 0000 ES: 0000 CR0:
000000008005003b
[ 142.731011] CR2:
0000000000401312 CR3:
000000005b66f000 CR4:
00000000000006e0
[ 142.731011] Stack:
[ 142.731011]
0000000000002710 ffffffff81e86880 ffffffff81fa3e40 ffff88005dc03df0
[ 142.731011]
ffffffff81050654 ffff88005ddcf000 ffff88005dc03e48 ffffffff8111a194
[ 142.731011]
0000000000000000 7fffffffffffffff 0000000000000046 00000000000010dc
[ 142.731011] Call Trace:
[ 142.731011] <IRQ>
[ 142.731011] [<
ffffffff81050654>] arch_trigger_all_cpu_backtrace+0x64/0xa0
[ 142.731011] [<
ffffffff8111a194>] rcu_check_callbacks+0x584/0x850
[ 142.731011] [<
ffffffff810a74a7>] update_process_times+0x47/0x70
[ 142.731011] [<
ffffffff81126765>] tick_sched_handle.isra.19+0x25/0x60
[ 142.731011] [<
ffffffff81127061>] tick_sched_timer+0x41/0x60
[ 142.731011] [<
ffffffff810c7446>] __run_hrtimer+0x86/0x460
[ 142.731011] [<
ffffffff81127020>] ? tick_sched_do_timer+0x40/0x40
[ 142.731011] [<
ffffffff810c809f>] hrtimer_interrupt+0x10f/0x260
[ 142.731011] [<
ffffffff8104e55a>] local_apic_timer_interrupt+0x3a/0x60
[ 142.731011] [<
ffffffff8180089f>] smp_apic_timer_interrupt+0x3f/0x50
[ 142.731011] [<
ffffffff817ff1f2>] apic_timer_interrupt+0x72/0x80
[ 142.731011] <EOI>
[ 142.731011] [<
ffffffff817f42b3>] ? retint_restore_args+0x13/0x13
[ 142.731011] [<
ffffffffa02a666b>] ? kdbus_conn_new+0x25b/0xf20 [kdbus]
[ 142.731011] [<
ffffffffa02a6741>] ? kdbus_conn_new+0x331/0xf20 [kdbus]
[ 142.731011] [<
ffffffffa02a8161>] kdbus_handle_ioctl+0x221/0xad0 [kdbus]
[ 142.731011] [<
ffffffff81361a31>] ? inode_has_perm.isra.47+0x51/0x90
[ 142.731011] [<
ffffffff81251f60>] do_vfs_ioctl+0x2f0/0x520
[ 142.731011] [<
ffffffff81252211>] SyS_ioctl+0x81/0xa0
[ 142.731011] [<
ffffffff817fe4e9>] system_call_fastpath+0x16/0x1b
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Sat, 7 Jun 2014 16:26:55 +0000 (17:26 +0100)]
policy: clean up headers and code documentation
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Tue, 3 Jun 2014 15:31:15 +0000 (16:31 +0100)]
domain: rework kdbus_domain_new() error path to fix a BUG_ON()
Currently just running: test/test-kdbus will trigger the BUG_ON()
appended at the bottom.
This is due to the test in check_domain_make() where we try to register
the same domain twice line: 297, hence kdbus_domain_new() fails with
-EEXIST at line domain.c:289
Later on error path we clear the non-finalized domain:
kdbus_domain_unref()
=> __kdbus_domain_free()
=> BUG_ON(!domain->disconnected)
After a closer look, it seems we will hit this BUG_ON() on every time
kdbus_domain_new() fails. domain was not finalized so
kdbus_domain_disconnect() is never called, and domain->disconnect can't
be true.
To fix this, as suggested by Kay we remove the kdbus_domain_unref()
call and clean up things manually.
While we are it, make sure we unregister the chrdev and restore the idr
slot on error paths too.
[16254.397574] ------------[ cut here ]------------
[16254.398272] kernel BUG at /home/tixxdz/code/d-bus/domain.c:163!
[16254.398524] invalid opcode: 0000 [#1] SMP
[16254.398524] Modules linked in: kdbus(OE) ip6t_rpfilter bnep bluetooth ip6t_REJECT cfg80211 rfkill xt_conntrack ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw ppdev serio_raw 8139too i2c_piix4 parport_pc parport microcode bochs_drm drm_kms_helper ttm drm 8139cp i2c_core mii ata_generic pata_acpi
[16254.398524] CPU: 3 PID: 30638 Comm: test-kdbus Tainted: G OE 3.15.0-0.rc5.git2.9.fc21.x86_64 #1
[16254.398524] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[16254.398524] task:
ffff88005c11cd40 ti:
ffff880005958000 task.ti:
ffff880005958000
[16254.398524] RIP: 0010:[<
ffffffffa031939b>] [<
ffffffffa031939b>] __kdbus_domain_free+0x9b/0xa0 [kdbus]
[16254.398524] RSP: 0018:
ffff880005959de8 EFLAGS:
00010246
[16254.398524] RAX:
ffff88005c11cd40 RBX:
ffff880046743400 RCX:
0000000000008b40
[16254.398524] RDX:
0000000000000001 RSI:
0000000000000001 RDI:
ffff880046743400
[16254.398524] RBP:
ffff880005959df0 R08:
0000000000000000 R09:
0000000000000000
[16254.398524] R10:
0000000000000000 R11:
0000000000000000 R12:
ffff8800466fa800
[16254.398524] R13:
00000000ffffffef R14:
ffff8800466fa960 R15:
ffff880046741400
[16254.398524] FS:
00007fa151358740(0000) GS:
ffff88005dc00000(0000) knlGS:
0000000000000000
[16254.398524] CS: 0010 DS: 0000 ES: 0000 CR0:
000000008005003b
[16254.398524] CR2:
00007fa13db78048 CR3:
0000000005a25000 CR4:
00000000000006e0
[16254.398524] Stack:
[16254.398524]
ffff880046743400 ffff880005959e40 ffffffffa03199e8 ffff880005959e68
[16254.398524]
ffff8800466fa8a8 01ffffffa031c0a0 ffff880005a58540 00000000ffff0180
[16254.398524]
ffff880005a589c0 0000000000000005 00007fff3dc96990 ffff880005959ec0
[16254.398524] Call Trace:
[16254.398524] [<
ffffffffa03199e8>] kdbus_domain_new+0x218/0x4c0 [kdbus]
[16254.398524] [<
ffffffffa0315546>] kdbus_handle_ioctl+0xb46/0xbd0 [kdbus]
[16254.398524] [<
ffffffff81360c21>] ? inode_has_perm.isra.47+0x51/0x90
[16254.398524] [<
ffffffff81251540>] do_vfs_ioctl+0x2f0/0x520
[16254.398524] [<
ffffffff812517f1>] SyS_ioctl+0x81/0xa0
[16254.398524] [<
ffffffff817fc769>] system_call_fastpath+0x16/0x1b
[16254.398524] Code: 8b 7b 08 e8 98 ca ef e0 48 8b 7b 10 e8 8f ca ef e0 48 89 df e8 87 ca ef e0 5b 5d c3 0f 1f 40 00 e8 6b ff ff ff eb d8 0f 0b 0f 0b <0f> 0b 0f 1f 00 0f 1f 44 00 00 55 48 89 e5 53 8b 47 28 48 89 fb
[16254.398524] RIP [<
ffffffffa031939b>] __kdbus_domain_free+0x9b/0xa0 [kdbus]
[16254.437815] ---[ end trace
bb9a1036dec78fcc ]---
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Sat, 31 May 2014 20:23:00 +0000 (21:23 +0100)]
connection: pin the subjective cred for KDBUS_POLICY_OWN
Make sure that the credentials of the connection at creation time will
last so the kdbus_policy_check_own_access() will work as expected.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Kay Sievers [Sun, 18 May 2014 10:00:09 +0000 (12:00 +0200)]
connection: add BUG_ON for pending work at connection free()
Kay Sievers [Thu, 15 May 2014 22:57:29 +0000 (00:57 +0200)]
never schedule work for a dead connection
Jacek Janczyk [Mon, 12 May 2014 13:35:08 +0000 (15:35 +0200)]
test-kdbus: fixed unterminated string
Unterminated string array is used to populate random bus name
in check_prepare_env(). Added proper termination.
Signed-off-by: Jacek Janczyk <j.janczyk@samsung.com>
Kay Sievers [Thu, 15 May 2014 21:40:14 +0000 (23:40 +0200)]
always require explicit disconnect(), pin connection until disconnect()
Kay Sievers [Fri, 9 May 2014 08:46:26 +0000 (10:46 +0200)]
TODO: update
Maciej Wereski [Fri, 9 May 2014 07:24:05 +0000 (09:24 +0200)]
Don't use nonexistent member of kdbus_pool_slice
Signed-off-by: Maciej Wereski <m.wereski@partner.samsung.com>
John de la Garza [Fri, 2 May 2014 21:15:30 +0000 (14:15 -0700)]
test-kdbus: check for EOPNOTSUPP instead of ENOTSUPP
Signed-off-by: John de la Garza <john@jjdev.com>
Kay Sievers [Thu, 24 Apr 2014 11:49:25 +0000 (13:49 +0200)]
remove racy calls to kdbus_conn_active()
Kay Sievers [Thu, 24 Apr 2014 11:27:37 +0000 (13:27 +0200)]
remove single-called kdbus_conn_find_peer() function
Kay Sievers [Thu, 24 Apr 2014 11:15:13 +0000 (13:15 +0200)]
kdbus.h: remove left-over documentation
Kay Sievers [Thu, 24 Apr 2014 08:32:26 +0000 (10:32 +0200)]
return EOPNOTSUPP instead of in-kernel-only ENOTSUPP
Kay Sievers [Tue, 22 Apr 2014 20:25:29 +0000 (22:25 +0200)]
remove no longer needed "on behalf" logic (ABI break)
Djalal Harouni [Fri, 18 Apr 2014 01:16:57 +0000 (02:16 +0100)]
names: take the registry write lock in kdbus_name_release()
Take the write lock in kdbus_name_release() instead of
kdbus_cmd_name_release() in order to reduce the lock hold time.
This change permits to convert the kdbus_bus_find_conn_by_id() call to
kdbus_conn_find_peer() since the bus lock will also be taken later in
kdbus_name_release().
Another advantage is that now kdbus_cmd_name_release() and
kdbus_name_release() have the same semantic of kdbus_cmd_name_acquire()
and kdbus_name_acquire()
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
David Herrmann [Fri, 11 Apr 2014 20:45:51 +0000 (22:45 +0200)]
connection: keep names locked while sending messages
If we send messages and we look up well-known names, we must keep them
locked until we're done. Otherwise, the name might move to another
connection before we queue it on the resolved name.
This fixes the sending path to lock names for the entire function call.
Name-movements already lock the name-registry for the entire move, so we
should be fine now.
We also inline the lookup-helper, as it is only used once and will get
messy if we return kdbus_name_entry objects.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Fri, 11 Apr 2014 01:33:45 +0000 (03:33 +0200)]
names: turn registry lock into rwsem
Imagine a racing name-acquire and msg-send. The msg-send might lookup the
destination name, which might be an activator. But before it can queue the
message, the racing name-acquire installs and implementor and moves all
pending messages from the activator to the implementor. If the original
msg-send afterwards continues, it will still queue the in-flight message
on the activator instead of the implementor.
After 1 day of discussion, we decided on turning the registry into a
read-write lock. This way, any msg-send will be locked against
name-changes. A name-change requires a writer lock, but any in-flight
message will hold a regsitry read-lock from the name-lookup until it
queued the message.
This patch only converts the lock, following patches will close the races.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Kay Sievers [Fri, 11 Apr 2014 18:11:34 +0000 (11:11 -0700)]
handle notify messages in a bus list to preserve bus-global ordering
David Herrmann [Thu, 10 Apr 2014 19:42:46 +0000 (21:42 +0200)]
connection: simplify cmd_conn_info()
We only have one path setting 'name'. There's no reason to split
name-handling, so merge them back together.
Also fix a bug where we access the name _before_ checking for strlen.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Thu, 10 Apr 2014 19:40:30 +0000 (21:40 +0200)]
kdbus: fix 'canceled' typo
'canceled' -> 'cancelled'
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Thu, 10 Apr 2014 19:32:08 +0000 (21:32 +0200)]
names: fix kdbus_name_lookup() to not ref NULL conns (ABI break)
The previous kdbus_name_lookup() fix assumed kdbus_conn_ref(NULL) is a
no-op. Turns out, it's a panic-op instead... Fix it.
Also add the ABI-break annotation as the previous error-code change
requires new systemd user-space (fixed by Kay).
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Thu, 10 Apr 2014 18:29:02 +0000 (20:29 +0200)]
names: fix kdbus_name_lookup() race condition
We cannot return pointers to the internal name registry from
kdbus_name_lookup(). No-one guarantees that these entries will stay valid.
Therefore, return the requested information directly with proper
references. This is protected by locks in kdbus_name_lookup() itself and
thus is safe.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Thu, 10 Apr 2014 18:27:14 +0000 (20:27 +0200)]
connection: dont modify msg in case of errors in kdbus_conn_get_conn_dst
We shouldn't modify the message before we decided whether to fail or not.
Keep the name-id local and write it only after we checked for all errors.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
David Herrmann [Thu, 10 Apr 2014 18:22:11 +0000 (20:22 +0200)]
connection: return ESRCH for unknown names consistently
We use ESRCH for unknown names except in GET_CONN_INFO. That's misleading
and makes error forwarding error prone. Fix that and use ESRCH instead of
ENOENT.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Hristo Venev [Wed, 9 Apr 2014 17:20:29 +0000 (20:20 +0300)]
metadata: reflect change in task_cgroup_name
Change:
e61734c55c24cdf11b07e52a74aec4dc4a7f4bd0.
Merged:
dc5ed40686a4da95881c35d913b60f867755cbe2 in 3.15-rc1.
task_cgroup_name returns a pointer to the path or NULL if there is not
enough space in the buffer (used to return nonnegative or
-ENAMETOOLONG).
On systemd systems fixes a kernel panic about init dying while opening a
bus. Now it boots properly.
Daniel Mack [Sun, 6 Apr 2014 10:23:50 +0000 (12:23 +0200)]
connection: remove duplicate variable declaration
Daniel Mack [Fri, 4 Apr 2014 18:35:32 +0000 (20:35 +0200)]
connection: check whether a reply exists before freeing it
For KDBUS_RECV_DROP, Walk the list of pending replies and see if the
one attached to this queue item is stil there. It might have been
removed by an incoming reply, and we currently don't track reply
entries in that direction in order to prevent potentially dangling
pointers.
At least atm, KDBUS_RECV_DROP is considered a rarely used operation, so
we can live with that.
Daniel Mack [Fri, 4 Apr 2014 17:39:10 +0000 (19:39 +0200)]
policy: whitespace cleanup
Djalal Harouni [Mon, 31 Mar 2014 00:43:04 +0000 (01:43 +0100)]
kdbus: use kdbus_conn_find_peer() where appropriate
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Mon, 31 Mar 2014 00:43:03 +0000 (01:43 +0100)]
connection: add function kdbus_conn_find_peer()
Use this function to find a connection peer with a given id.
The returned connection is on the same bus of the source connection
and it is ref'ed.
This allows to reduce code duplication and will make it easy to
read code and differentiate between messages that have a source
connection and other kernel generated messages.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Mon, 31 Mar 2014 00:41:26 +0000 (01:41 +0100)]
connection: unref ep and bus and free match_db on error path
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Mon, 31 Mar 2014 00:41:25 +0000 (01:41 +0100)]
handle: change return value from -EFAULT to -EPERM
Return -EPERM if current does not have the appropriate privileges to
create custom endpoints.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Mon, 31 Mar 2014 00:41:24 +0000 (01:41 +0100)]
endpoint: protect the idr tree on release
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Djalal Harouni [Mon, 31 Mar 2014 00:41:23 +0000 (01:41 +0100)]
connection: remove useless kdbus_str_hash() call
The hash is calculated internally in kdbus_name_lookup(), so just
remove this useless kdbus_str_hash() call. It is not used.
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Ingo van Lil [Sat, 29 Mar 2014 16:54:58 +0000 (17:54 +0100)]
kdbus: Check for bloom parameter in "make bus" command
When handling a KDBUS_CMD_BUS_MAKE command the kernel must make sure
that the message contains a bloom parameter item to avoid a null-
pointer dereference.
Signed-off-by: Ingo van Lil <inguin@gmx.de>
Kay Sievers [Wed, 26 Mar 2014 03:06:47 +0000 (04:06 +0100)]
kerneldoc: remove KDBUS_CMD_EP_POLICY_SET, add KDBUS_CMD_EP_UPDATE
Daniel Mack [Tue, 25 Mar 2014 18:35:49 +0000 (19:35 +0100)]
policy: remove kdbus_policy_db_dump()
It was added for debugging only, and can easily be added back if still
needed.
Kay Sievers [Tue, 25 Mar 2014 13:11:46 +0000 (14:11 +0100)]
whitespace cleanup
Daniel Mack [Tue, 25 Mar 2014 10:22:33 +0000 (11:22 +0100)]
pool: cosmetic indent fix
Daniel Mack [Tue, 25 Mar 2014 08:44:07 +0000 (09:44 +0100)]
connection: fix indentation