Add kdbus_fork_test_by_id() to test connections by id under different
uids. Currently we succeed at this test.

Update:
1) kdbus_msg_recv() to get the offset of the slice, so we can
release it later.

2) kdbus_msg_recv_poll() to pass the offset argument to
kdbus_msg_recv().

We do this to follow best practice and to be able to free the returned
kdbus_msg and the slice pointed by that offset.

Signed-off-by: Djalal Harouni <tix...@opendz.org>
---
Hi Daniel, before applying please make sure that we want this. It
follows what I've discussed in the other mail, otherwise just test it,
it will give a better view of the routing policy.

 test/kdbus-util.c       |  15 ++++-
 test/kdbus-util.h       |   7 ++-
 test/test-activator.c   |   2 +-
 test/test-chat.c        |   4 +-
 test/test-connection.c  |   4 +-
 test/test-daemon.c      |   2 +-
 test/test-fd.c          |   2 +-
 test/test-message.c     |   2 +-
 test/test-metadata-ns.c |   2 +-
 test/test-monitor.c     |   7 ++-
 test/test-policy-ns.c   | 160 ++++++++++++++++++++++++++++++++++++++++--------
 test/test-sync.c        |   2 +-
 12 files changed, 166 insertions(+), 43 deletions(-)

diff --git a/test/kdbus-util.c b/test/kdbus-util.c
index c72b8fe..fe4565c 100644
--- a/test/kdbus-util.c
+++ b/test/kdbus-util.c
@@ -630,6 +630,9 @@ void kdbus_msg_free(struct kdbus_msg *msg)
        const struct kdbus_item *item;
        int nfds, i;
 
+       if (!msg)
+               return;
+
        KDBUS_ITEM_FOREACH(item, msg, items) {
                switch (item->type) {
                /* close all memfds */
@@ -648,7 +651,9 @@ void kdbus_msg_free(struct kdbus_msg *msg)
        }
 }
 
-int kdbus_msg_recv(struct kdbus_conn *conn, struct kdbus_msg **msg_out)
+int kdbus_msg_recv(struct kdbus_conn *conn,
+                  struct kdbus_msg **msg_out,
+                  uint64_t *offset)
 {
        struct kdbus_cmd_recv recv = {};
        struct kdbus_msg *msg;
@@ -665,6 +670,9 @@ int kdbus_msg_recv(struct kdbus_conn *conn, struct 
kdbus_msg **msg_out)
 
        if (msg_out) {
                *msg_out = msg;
+
+               if (offset)
+                       *offset = recv.offset;
        } else {
                kdbus_msg_free(msg);
 
@@ -677,13 +685,14 @@ int kdbus_msg_recv(struct kdbus_conn *conn, struct 
kdbus_msg **msg_out)
 }
 
 int kdbus_msg_recv_poll(struct kdbus_conn *conn,
+                       unsigned int timeout_ms,
                        struct kdbus_msg **msg_out,
-                       unsigned int timeout_ms)
+                       uint64_t *offset)
 {
        int ret;
 
        for (; timeout_ms; timeout_ms--) {
-               ret = kdbus_msg_recv(conn, NULL);
+               ret = kdbus_msg_recv(conn, msg_out, offset);
                if (ret == -EAGAIN) {
                        usleep(1000);
                        continue;
diff --git a/test/kdbus-util.h b/test/kdbus-util.h
index c2370e7..8e3d0a8 100644
--- a/test/kdbus-util.h
+++ b/test/kdbus-util.h
@@ -56,9 +56,10 @@ int kdbus_name_release(struct kdbus_conn *conn, const char 
*name);
 int kdbus_name_acquire(struct kdbus_conn *conn, const char *name,
                       uint64_t flags);
 void kdbus_msg_free(struct kdbus_msg *msg);
-int kdbus_msg_recv(struct kdbus_conn *conn, struct kdbus_msg **msg);
-int kdbus_msg_recv_poll(struct kdbus_conn *conn, struct kdbus_msg **msg_out,
-                       unsigned int timeout_ms);
+int kdbus_msg_recv(struct kdbus_conn *conn,
+                  struct kdbus_msg **msg, uint64_t *offset);
+int kdbus_msg_recv_poll(struct kdbus_conn *conn, unsigned int timeout_ms,
+                       struct kdbus_msg **msg_out, uint64_t *offset);
 int kdbus_free(const struct kdbus_conn *conn, uint64_t offset);
 void kdbus_msg_dump(const struct kdbus_conn *conn,
                    const struct kdbus_msg *msg);
diff --git a/test/test-activator.c b/test/test-activator.c
index eb57b52..7bdcbf3 100644
--- a/test/test-activator.c
+++ b/test/test-activator.c
@@ -79,7 +79,7 @@ int kdbus_test_activator(struct kdbus_test_env *env)
                }
 
                if (fds[1].revents & POLLIN) {
-                       kdbus_msg_recv(env->conn, NULL);
+                       kdbus_msg_recv(env->conn, NULL, NULL);
                        break;
                }
        }
diff --git a/test/test-chat.c b/test/test-chat.c
index 11c140d..b2bcc31 100644
--- a/test/test-chat.c
+++ b/test/test-chat.c
@@ -86,7 +86,7 @@ int kdbus_test_chat(struct kdbus_test_env *env)
                        if (count > 2)
                                kdbus_name_release(conn_a, "foo.bar.baz");
 
-                       ret = kdbus_msg_recv(conn_a, NULL);
+                       ret = kdbus_msg_recv(conn_a, NULL, NULL);
                        ASSERT_RETURN(ret == 0);
                        ret = kdbus_msg_send(conn_a, NULL,
                                             0xc0000000 | cookie++,
@@ -95,7 +95,7 @@ int kdbus_test_chat(struct kdbus_test_env *env)
                }
 
                if (fds[1].revents & POLLIN) {
-                       ret = kdbus_msg_recv(conn_b, NULL);
+                       ret = kdbus_msg_recv(conn_b, NULL, NULL);
                        ASSERT_RETURN(ret == 0);
                        ret = kdbus_msg_send(conn_b, NULL,
                                             0xc0000000 | cookie++,
diff --git a/test/test-connection.c b/test/test-connection.c
index 355e359..dccb1b3 100644
--- a/test/test-connection.c
+++ b/test/test-connection.c
@@ -245,7 +245,7 @@ int kdbus_test_conn_update(struct kdbus_test_env *env)
        ret = kdbus_msg_send(env->conn, NULL, 0x12345678, 0, 0, 0, conn->id);
        ASSERT_RETURN(ret == 0);
 
-       ret = kdbus_msg_recv(conn, &msg);
+       ret = kdbus_msg_recv(conn, &msg, NULL);
        ASSERT_RETURN(ret == 0);
 
        KDBUS_ITEM_FOREACH(item, msg, items)
@@ -267,7 +267,7 @@ int kdbus_test_conn_update(struct kdbus_test_env *env)
        ret = kdbus_msg_send(env->conn, NULL, 0x12345678, 0, 0, 0, conn->id);
        ASSERT_RETURN(ret == 0);
 
-       ret = kdbus_msg_recv(conn, &msg);
+       ret = kdbus_msg_recv(conn, &msg, NULL);
        ASSERT_RETURN(ret == 0);
 
        KDBUS_ITEM_FOREACH(item, msg, items)
diff --git a/test/test-daemon.c b/test/test-daemon.c
index 2ebb27d..fa79303 100644
--- a/test/test-daemon.c
+++ b/test/test-daemon.c
@@ -51,7 +51,7 @@ int kdbus_test_daemon(struct kdbus_test_env *env)
                        break;
 
                if (fds[0].revents & POLLIN) {
-                       ret = kdbus_msg_recv(env->conn, NULL);
+                       ret = kdbus_msg_recv(env->conn, NULL, NULL);
                        ASSERT_RETURN(ret == 0);
                }
 
diff --git a/test/test-fd.c b/test/test-fd.c
index aee5064..9ffdc35 100644
--- a/test/test-fd.c
+++ b/test/test-fd.c
@@ -87,7 +87,7 @@ int kdbus_test_fd_passing(struct kdbus_test_env *env)
        ret = send_fd(conn_src, conn_dst->id, fds[0]);
        ASSERT_RETURN(ret == 0);
 
-       ret = kdbus_msg_recv(conn_dst, &msg);
+       ret = kdbus_msg_recv(conn_dst, &msg, NULL);
        ASSERT_RETURN(ret == 0);
 
        KDBUS_ITEM_FOREACH(item, msg, items) {
diff --git a/test/test-message.c b/test/test-message.c
index 84962ff..387ef70 100644
--- a/test/test-message.c
+++ b/test/test-message.c
@@ -125,7 +125,7 @@ int kdbus_test_message_prio(struct kdbus_test_env *env)
        ASSERT_RETURN(msg_recv_prio(conn_a, 10, -100) == 0);
 
        kdbus_printf("--- get priority (all)\n");
-       ASSERT_RETURN(kdbus_msg_recv(conn_a, NULL) == 0);
+       ASSERT_RETURN(kdbus_msg_recv(conn_a, NULL, NULL) == 0);
 
        kdbus_conn_free(conn_a);
        kdbus_conn_free(conn_b);
diff --git a/test/test-metadata-ns.c b/test/test-metadata-ns.c
index 372cd5a..d83b8e1 100644
--- a/test/test-metadata-ns.c
+++ b/test/test-metadata-ns.c
@@ -166,7 +166,7 @@ static int kdbus_clone_userns_test(const char *bus, struct 
kdbus_conn *conn)
        }
 
        /* Receive in the original (root privileged) user namespace */
-       ret = kdbus_msg_recv_poll(conn, NULL, 1000);
+       ret = kdbus_msg_recv_poll(conn, 1000, NULL, NULL);
        ASSERT_RETURN(ret == 0);
 
        ret = waitpid(pid, &status, 0);
diff --git a/test/test-monitor.c b/test/test-monitor.c
index 90308af..db8fde6 100644
--- a/test/test-monitor.c
+++ b/test/test-monitor.c
@@ -26,6 +26,7 @@ int kdbus_test_monitor(struct kdbus_test_env *env)
 {
        struct kdbus_conn *monitor, *conn;
        unsigned int cookie = 0xdeadbeef;
+       uint64_t offset = 0;
        struct kdbus_cmd_name *cmd_name;
        struct kdbus_msg *msg;
        size_t size;
@@ -56,16 +57,18 @@ int kdbus_test_monitor(struct kdbus_test_env *env)
        ASSERT_RETURN(ret == 0);
 
        /* the recipient should have got the message */
-       ret = kdbus_msg_recv(conn, &msg);
+       ret = kdbus_msg_recv(conn, &msg, &offset);
        ASSERT_RETURN(ret == 0);
        ASSERT_RETURN(msg->cookie == cookie);
        kdbus_msg_free(msg);
+       kdbus_free(conn, offset);
 
        /* and so should the monitor */
-       ret = kdbus_msg_recv(monitor, &msg);
+       ret = kdbus_msg_recv(monitor, &msg, &offset);
        ASSERT_RETURN(ret == 0);
        ASSERT_RETURN(msg->cookie == cookie);
        kdbus_msg_free(msg);
+       kdbus_free(monitor, offset);
 
        kdbus_conn_free(monitor);
        kdbus_conn_free(conn);
diff --git a/test/test-policy-ns.c b/test/test-policy-ns.c
index 517278c..8b48d45 100644
--- a/test/test-policy-ns.c
+++ b/test/test-policy-ns.c
@@ -54,7 +54,7 @@ static void *kdbus_recv_echo(void *ptr)
        int ret;
        struct kdbus_conn *conn = ptr;
 
-       ret = kdbus_msg_recv_poll(conn, NULL, 1000);
+       ret = kdbus_msg_recv_poll(conn, 1000, NULL, NULL);
 
        return (void *)(long)ret;
 }
@@ -124,27 +124,6 @@ static int kdbus_register_policy_holder(char *bus, const 
char *name,
        return TEST_OK;
 }
 
-/* return TEST_OK or TEST_ERR on failure */
-static int kdbus_receiver_acquire_name(char *bus, const char *name,
-                                       struct kdbus_conn **conn)
-{
-       int ret;
-       struct kdbus_conn *c;
-
-       c = kdbus_hello(bus, 0, NULL, 0);
-       ASSERT_RETURN(c);
-
-       ret = kdbus_add_match_empty(c);
-       ASSERT_RETURN(ret == 0);
-
-       ret = kdbus_name_acquire(c, name, 0);
-       ASSERT_RETURN(ret == 0);
-
-       *conn = c;
-
-       return TEST_OK;
-}
-
 /**
  * Create new threads for receiving from multiple senders,
  * The 'conn_db' will be populated by newly created connections.
@@ -220,6 +199,102 @@ static int kdbus_normal_test(const char *bus, const char 
*name,
        return TEST_OK;
 }
 
+static int kdbus_fork_test_by_id(const char *bus,
+                                struct kdbus_conn **conn_db,
+                                int parent_status, int child_status)
+{
+       int ret;
+       pid_t pid;
+       uint64_t cookie = 0x9876ecba;
+       struct kdbus_msg *msg = NULL;
+       uint64_t offset = 0;
+       int status = 0;
+
+       /*
+        * If the child_status is not EXIT_SUCCESS, then we expect
+        * that sending from the child will fail, thus receiving
+        * from parent must error with -ETIMEDOUT, and vice versa.
+        */
+       bool parent_timedout = !!child_status;
+       bool child_timedout = !!parent_status;
+
+       pid = fork();
+       ASSERT_RETURN_VAL(pid >= 0, pid);
+
+       if (pid == 0) {
+               struct kdbus_conn *conn_src;
+
+               ret = prctl(PR_SET_PDEATHSIG, SIGKILL);
+               ASSERT_EXIT(ret == 0);
+
+               ret = drop_privileges(65534, 65534);
+               ASSERT_EXIT(ret == 0);
+
+               conn_src = kdbus_hello(bus, 0, NULL, 0);
+               ASSERT_EXIT(conn_src);
+
+               ret = kdbus_add_match_empty(conn_src);
+               ASSERT_EXIT(ret == 0);
+
+               /*
+                * child_status is always checked against send
+                * operations, in case it fails always return
+                * EXIT_FAILURE.
+                */
+               ret = kdbus_msg_send(conn_src, NULL, cookie,
+                                    0, 0, 0, conn_db[0]->id);
+               ASSERT_EXIT(ret == child_status);
+
+               ret = kdbus_msg_recv_poll(conn_src, 1000, NULL, NULL);
+
+               kdbus_conn_free(conn_src);
+
+               /*
+                * Child kdbus_msg_recv_poll() should timeout since
+                * the parent_status was set to a non EXIT_SUCCESS
+                * value.
+                */
+               if (child_timedout)
+                       _exit(ret == -ETIMEDOUT ? EXIT_SUCCESS : EXIT_FAILURE);
+
+               _exit(ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
+       }
+
+       ret = kdbus_msg_recv_poll(conn_db[0], 1000, &msg, &offset);
+       /*
+        * If parent_timedout is set then this should fail with
+        * -ETIMEDOUT since the child_status was set to a non
+        * EXIT_SUCCESS value. Otherwise, assume
+        * that kdbus_msg_recv_poll() has succeeded.
+        */
+       if (parent_timedout) {
+               ASSERT_RETURN_VAL(ret == -ETIMEDOUT, TEST_ERR);
+
+               /* timedout no need to continue, we don't have the
+                * child connection ID, so just terminate. */
+               goto out;
+       } else {
+               ASSERT_RETURN_VAL(ret == 0, ret);
+       }
+
+       ret = kdbus_msg_send(conn_db[0], NULL, ++cookie,
+                            0, 0, 0, msg->src_id);
+       /*
+        * parent_status is checked against send operations,
+        * on failures always return TEST_ERR.
+        */
+       ASSERT_RETURN_VAL(ret == parent_status, TEST_ERR);
+
+       kdbus_msg_free(msg);
+       kdbus_free(conn_db[0], offset);
+
+out:
+       ret = waitpid(pid, &status, 0);
+       ASSERT_RETURN_VAL(ret >= 0, ret);
+
+       return (status == EXIT_SUCCESS) ? TEST_OK : TEST_ERR;
+}
+
 /*
  * Return: TEST_OK, TEST_ERR or TEST_SKIP
  * we return TEST_OK only if the childs return with the expected
@@ -239,7 +314,7 @@ static int kdbus_fork_test(const char *bus, const char 
*name,
 
        if (pid == 0) {
                ret = prctl(PR_SET_PDEATHSIG, SIGKILL);
-               ASSERT_EXIT(pid >= 0);
+               ASSERT_EXIT(ret == 0);
 
                ret = drop_privileges(65534, 65534);
                ASSERT_EXIT(ret == 0);
@@ -378,7 +453,7 @@ static int kdbus_clone_userns_test(const char *bus,
         * Receive in the original (root privileged) user namespace,
         * must fail with -ETIMEDOUT.
         */
-       ret = kdbus_msg_recv_poll(conn_db[0], NULL, 1000);
+       ret = kdbus_msg_recv_poll(conn_db[0], 1000, NULL, NULL);
        ASSERT_RETURN_VAL(ret == -ETIMEDOUT, ret);
 
        ret = waitpid(pid, &status, 0);
@@ -406,6 +481,16 @@ int kdbus_test_policy_ns(struct kdbus_test_env *env)
 
        memset(conn_db, 0, MAX_CONN * sizeof(struct kdbus_conn *));
 
+       conn_db[0] = kdbus_hello(bus, 0, NULL, 0);
+       ASSERT_RETURN(conn_db[0]);
+
+       ret = kdbus_add_match_empty(conn_db[0]);
+       ASSERT_RETURN(ret == 0);
+
+       ret = kdbus_fork_test_by_id(bus, conn_db,
+                                   EXIT_SUCCESS, EXIT_SUCCESS);
+       ASSERT_EXIT(ret == 0);
+
        ret = kdbus_register_policy_holder(bus, POLICY_NAME,
                                           &policy_holder);
        ASSERT_RETURN(ret == 0);
@@ -415,7 +500,8 @@ int kdbus_test_policy_ns(struct kdbus_test_env *env)
                                            &activator);
        ASSERT_RETURN(ret == 0);
 
-       ret = kdbus_receiver_acquire_name(bus, POLICY_NAME, &conn_db[0]);
+       /* Acquire POLICY_NAME */
+       ret = kdbus_name_acquire(conn_db[0], POLICY_NAME, 0);
        ASSERT_RETURN(ret == 0);
 
        ret = kdbus_normal_test(bus, POLICY_NAME, conn_db);
@@ -431,6 +517,18 @@ int kdbus_test_policy_ns(struct kdbus_test_env *env)
        ASSERT_RETURN(ret == 0);
 
        /*
+        * childs connections are able to talk to conn_db[0] since
+        * current POLICY_NAME TALK type is KDBUS_POLICY_ACCESS_WORLD,
+        * so expect EXIT_SUCCESS when sending from child. However,
+        * since the child's connection does not own any well-known
+        * name, The parent connection conn_db[0] should fail with
+        * -EPERM when sending to it.
+        */
+       ret = kdbus_fork_test_by_id(bus, conn_db,
+                                   -EPERM, EXIT_SUCCESS);
+       ASSERT_EXIT(ret == 0);
+
+       /*
         * Connections that can talk are perhaps being destroyed now.
         * Restrict the policy and purge cache entries where the
         * conn_db[0] is the destination.
@@ -449,6 +547,18 @@ int kdbus_test_policy_ns(struct kdbus_test_env *env)
        ret = kdbus_fork_test(bus, POLICY_NAME, conn_db, -EPERM);
        ASSERT_RETURN(ret == 0);
 
+       /*
+        * Now expect that both parent and child to fail.
+        *
+        * Child should fail with -EPERM since we just restricted
+        * the POLICY_NAME TALK to uid 0 and its uid is 65534.
+        *
+        * Since the parent's connection will timeout when receiving
+        * from the child, we never continue. FWIW just put -EPERM.
+        */
+       ret = kdbus_fork_test_by_id(bus, conn_db, -EPERM, -EPERM);
+       ASSERT_EXIT(ret == 0);
+
        /* Check if the name can be reached in a new userns */
        ret = kdbus_clone_userns_test(bus, POLICY_NAME, conn_db, -EPERM);
        ASSERT_RETURN(ret == 0);
diff --git a/test/test-sync.c b/test/test-sync.c
index 55091be..66d41cf 100644
--- a/test/test-sync.c
+++ b/test/test-sync.c
@@ -35,7 +35,7 @@ static void *run_thread(void *data)
 
        if (fd.revents & POLLIN) {
                kdbus_printf("Thread received message, sending reply ...\n");
-               kdbus_msg_recv(conn_a, NULL);
+               kdbus_msg_recv(conn_a, NULL, NULL);
                kdbus_msg_send(conn_a, NULL, 0, 0, cookie, 0, conn_b->id);
        }
 
-- 
1.9.3

_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to