Hi, The tests are not clean... just a copy/past if you want to confirm this one! they are attached.
patch with policy-holder.patch then run: test-kdbus-policy-holder You should hit it! On Wed, Jun 11, 2014 at 05:27:58PM +0100, Djalal Harouni wrote: > 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 <tix...@opendz.org> > --- > I've checked all the other calls, the remaining one is: > connection.c:kdbus_conn_payload_add() it seems that it fakes the size > and hanldes the alignment, if I've more time I'll try to check it. This > one is tricky! > > connection.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/connection.c b/connection.c > index 3f27889..3e8c5de 100644 > --- a/connection.c > +++ b/connection.c > @@ -1784,12 +1784,18 @@ int kdbus_cmd_conn_update(struct kdbus_conn *conn, > { > const struct kdbus_item *item; > bool policy_provided = false; > + u64 attach_flags = 0; > int ret; > > KDBUS_ITEMS_FOREACH(item, cmd->items, KDBUS_ITEMS_SIZE(cmd, items)) { > + > + if (!KDBUS_ITEM_VALID(item, &cmd->items, > + KDBUS_ITEMS_SIZE(cmd, items))) > + return -EINVAL; > + > switch (item->type) { > case KDBUS_ITEM_ATTACH_FLAGS: > - conn->attach_flags = item->data64[0]; > + attach_flags = item->data64[0]; > break; > case KDBUS_ITEM_NAME: > case KDBUS_ITEM_POLICY_ACCESS: > @@ -1798,6 +1804,11 @@ int kdbus_cmd_conn_update(struct kdbus_conn *conn, > } > } > > + if (!KDBUS_ITEMS_END(item, cmd->items, KDBUS_ITEMS_SIZE(cmd, items))) > + return -EINVAL; > + > + conn->attach_flags = attach_flags; > + > if (!policy_provided) > return 0; > > @@ -1862,6 +1873,11 @@ int kdbus_conn_new(struct kdbus_ep *ep, > > KDBUS_ITEMS_FOREACH(item, hello->items, > KDBUS_ITEMS_SIZE(hello, items)) { > + > + if (!KDBUS_ITEM_VALID(item, &hello->items, > + KDBUS_ITEMS_SIZE(hello, items))) > + return -EINVAL; > + > switch (item->type) { > case KDBUS_ITEM_NAME: > if (!is_activator && !is_policy_holder) > @@ -1916,6 +1932,9 @@ int kdbus_conn_new(struct kdbus_ep *ep, > } > } > > + if (!KDBUS_ITEMS_END(item, hello->items, KDBUS_ITEMS_SIZE(hello, > items))) > + return -EINVAL; > + > if ((is_activator || is_policy_holder) && !name) > return -EINVAL; > > -- > 1.9.0 > -- Djalal Harouni http://opendz.org
diff --git a/test/Makefile b/test/Makefile index 83cb736..0d57eed 100644 --- a/test/Makefile +++ b/test/Makefile @@ -20,7 +20,8 @@ TESTS= \ test-kdbus-chat \ test-kdbus-timeout \ test-kdbus-sync \ - test-kdbus-prio + test-kdbus-prio \ + test-kdbus-policy-holder all: $(TESTS) diff --git a/test/kdbus-util.c b/test/kdbus-util.c index 5316263..77bfa47 100644 --- a/test/kdbus-util.c +++ b/test/kdbus-util.c @@ -109,7 +109,7 @@ struct conn *kdbus_hello(const char *path, uint64_t flags) return __kdbus_hello(path, flags, NULL, 0); } -static struct conn * +struct conn * kdbus_hello_registrar(const char *path, const char *name, const struct kdbus_policy_access *access, size_t num_access, uint64_t flags) diff --git a/test/kdbus-util.h b/test/kdbus-util.h index fde3f77..9771622 100644 --- a/test/kdbus-util.h +++ b/test/kdbus-util.h @@ -45,6 +45,9 @@ char *msg_id(uint64_t id, char *buf); int msg_send(const struct conn *conn, const char *name, uint64_t cookie, uint64_t flags, uint64_t timeout, int64_t priority, uint64_t dst_id); struct conn *kdbus_hello(const char *path, uint64_t hello_flags); +struct conn *kdbus_hello_registrar(const char *path, const char *name, + const struct kdbus_policy_access *access, + size_t num_access, uint64_t flags); struct conn *kdbus_hello_activator(const char *path, const char *name, const struct kdbus_policy_access *access, size_t num_access);
#include <stdio.h> #include <string.h> #include <fcntl.h> #include <pthread.h> #include <poll.h> #include <stdlib.h> #include <stddef.h> #include <stdint.h> #include <stdbool.h> #include <unistd.h> #include <errno.h> #include <sys/ioctl.h> #include "kdbus-util.h" #include "kdbus-enum.h" #define MAX_CONN 128 #define POLICY_PATH "foo.test.policy-test" struct conn_entry { bool policy_holder; bool cached_send; struct conn *conn; }; static int kdbus_register_policy(char *bus, struct conn_entry **e) { int ret = -1; struct conn_entry *ce; struct kdbus_policy_access access[2]; ce = malloc(sizeof(struct conn_entry)); if (!ce) return -ENOMEM; memset(ce, 0, sizeof(struct conn_entry)); access[0].type = KDBUS_POLICY_ACCESS_WORLD; access[0].access = KDBUS_POLICY_TALK; access[0].id = 1001; access[1].type = KDBUS_POLICY_ACCESS_WORLD; access[1].access = KDBUS_POLICY_TALK; ce->conn = kdbus_hello_registrar(bus, POLICY_PATH, access, 2, KDBUS_HELLO_POLICY_HOLDER); if (!ce->conn) return ret; *e = ce; return 0; } static void *kdbus_recv_echo(void *ptr) { int ret; int cnt = 3; struct pollfd fd; struct conn *conn = ptr; fd.fd = conn->fd; fd.events = POLLIN | POLLPRI | POLLHUP; fd.revents = 0; while (cnt) { cnt--; ret = poll(&fd, 1, 2000); if (ret == 0) { ret = -ETIMEDOUT; break; } if (ret > 0 && fd.revents & POLLIN) { printf("-- Connection id: %lu new received message:\n", conn->id); ret = msg_recv(conn); } if (ret >= 0 || ret != -EAGAIN) break; } return (void *)(long)ret; } static int kdbus_test_connections(struct conn_entry **conn_db) { int ret; unsigned int i, tid; unsigned long cookie = 1; unsigned int thread_nr = MAX_CONN - 1; pthread_t thread_id[MAX_CONN - 1] = {'\0'}; for (tid = 0, i = 1; tid < thread_nr; tid++, i++) { ret = pthread_create(&thread_id[tid], NULL, kdbus_recv_echo, (void *)conn_db[0]->conn); if (ret < 0) { fprintf(stderr, "error pthread_create: %d err %d (%m)\n", ret, errno); break; } ret = msg_send(conn_db[i]->conn, POLICY_PATH, cookie++, 0, 0, 0, conn_db[0]->conn->id); if (ret != 0) break; } for (tid = 0; tid < thread_nr; tid++) { int thread_ret = 0; if (thread_id[tid]) { pthread_join(thread_id[tid], (void *)&thread_ret); /* Update ret only if it is >= 0 * msg_send may finish with: * EXIT_FAILURE or EXIT_SUCCESS */ if (thread_ret < 0 && ret >= 0) ret = thread_ret; } } return ret; } static void kdbus_free_conn_entry(struct conn_entry *e) { if (e) { if (e->conn) { close(e->conn->fd); free(e->conn); } free(e); } } static int kdbus_check_policy(char *bus) { int i; int ret; struct conn_entry **conn_db = NULL; conn_db = calloc(MAX_CONN, sizeof(struct conn_entry *)); if (!conn_db) return -ENOMEM; memset(conn_db, 0, MAX_CONN * sizeof(struct conn_entry *)); printf("-- Round 1) creating connections:\n"); ret = kdbus_register_policy(bus, &conn_db[0]); if (ret < 0) goto out_free_connections; for (i = 1; i < MAX_CONN; i++) { struct conn_entry *e = malloc(sizeof(struct conn_entry)); if (!e) { ret = -ENOMEM; goto out_free_connections; } memset(e, 0, sizeof(struct conn_entry)); *(conn_db+i) = e; e->conn = kdbus_hello(bus, 0); if (!e->conn) { ret = -1; goto out_free_connections; } e->cached_send = true; add_match_empty(e->conn->fd); } ret = kdbus_test_connections(conn_db); printf("-- Round 2) testing connections............... "); if (ret < 0) { printf("FAILED\n"); goto out_free_connections; } printf("OK\n"); out_free_connections: for (i = 0; i < MAX_CONN; i++) kdbus_free_conn_entry(conn_db[i]); free(conn_db); return ret; } int main(int argc, char *argv[]) { struct { struct kdbus_cmd_make head; /* bloom size item */ struct { uint64_t size; uint64_t type; struct kdbus_bloom_parameter bloom; } bs; /* name item */ uint64_t n_size; uint64_t n_type; char name[64]; } bus_make; int fdc, ret, i; char *bus; printf("-- opening /dev/" KBUILD_MODNAME "/control\n"); fdc = open("/dev/" KBUILD_MODNAME "/control", O_RDWR|O_CLOEXEC); if (fdc < 0) { fprintf(stderr, "--- error %d (%m)\n", fdc); return EXIT_FAILURE; } memset(&bus_make, 0, sizeof(bus_make)); bus_make.bs.size = sizeof(bus_make.bs); bus_make.bs.type = KDBUS_ITEM_BLOOM_PARAMETER; bus_make.bs.bloom.size = 64; bus_make.bs.bloom.n_hash = 1; snprintf(bus_make.name, sizeof(bus_make.name), "%u-testbus", getuid()); bus_make.n_type = KDBUS_ITEM_MAKE_NAME; bus_make.n_size = KDBUS_ITEM_HEADER_SIZE + strlen(bus_make.name) + 1; bus_make.head.size = sizeof(struct kdbus_cmd_make) + sizeof(bus_make.bs) + bus_make.n_size; printf("-- creating bus '%s'\n", bus_make.name); ret = ioctl(fdc, KDBUS_CMD_BUS_MAKE, &bus_make); if (ret) { fprintf(stderr, "--- error %d (%m)\n", ret); return EXIT_FAILURE; } if (asprintf(&bus, "/dev/" KBUILD_MODNAME "/%s/bus", bus_make.name) < 0) return EXIT_FAILURE; ret = kdbus_check_policy(bus); printf("RUNNING TEST 'policy db check' "); for (i = 0; i < 15; i++) printf("."); printf(" "); if (ret < 0) { printf("FAILED\n"); return EXIT_FAILURE; } printf("OK\n"); close(fdc); free(bus); return EXIT_SUCCESS; }
_______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel