From 017f56c7e53f81c0cc559acafd4f446abd935d19 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Fri, 17 Oct 2014 13:43:14 +0200 Subject: [PATCH] tree-wide: rework flags negotiation (ABI break) We are obliged to reject all bits in flags fields that are not known to the kernel. In order to let userspace know which flags the kernel knowns about, we agreed to always write back to the flags field in the ioctl buffer, even if the call succeeded. The kernel will, however, will always set the KDBUS_FLAG_KERNEL bit, which consequently is always invalid when submitted by userspace. Move some checks from other place to handle.c, and update the testsuite and documentation accordingly. Signed-off-by: Daniel Mack --- connection.c | 22 +------- handle.c | 121 ++++++++++++++++++++++++++++++++++------- kdbus.h | 1 + kdbus.txt | 70 ++++++++++++++++-------- match.c | 6 -- message.c | 14 ++--- message.h | 2 +- names.c | 15 ----- test/test-connection.c | 13 ++++- test/test-domain.c | 3 + test/test-free.c | 4 +- util.c | 35 ++++++++++++ util.h | 1 + 13 files changed, 210 insertions(+), 97 deletions(-) diff --git a/connection.c b/connection.c index e4464cf..006fe9a 100644 --- a/connection.c +++ b/connection.c @@ -220,18 +220,11 @@ int kdbus_cmd_msg_recv(struct kdbus_conn *conn, struct kdbus_queue_entry *entry = NULL; int ret; - /* Reject unknown flags */ - if (recv->flags & ~(KDBUS_RECV_PEEK | - KDBUS_RECV_DROP | - KDBUS_RECV_USE_PRIORITY)) - return -EOPNOTSUPP; - if (recv->offset > 0) return -EINVAL; mutex_lock(&conn->lock); - ret = kdbus_queue_entry_peek(&conn->queue, - recv->priority, + ret = kdbus_queue_entry_peek(&conn->queue, recv->priority, recv->flags & KDBUS_RECV_USE_PRIORITY, &entry); if (ret < 0) @@ -1214,9 +1207,6 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, int ret = 0; u64 flags; - if (cmd_info->flags & ~_KDBUS_ATTACH_ALL) - return -EOPNOTSUPP; - if (cmd_info->id == 0) { const char *name; @@ -1338,9 +1328,6 @@ int kdbus_cmd_conn_update(struct kdbus_conn *conn, u64 attach_flags; int ret; - if (cmd->flags != 0) - return -EOPNOTSUPP; - KDBUS_ITEMS_FOREACH(item, cmd->items, KDBUS_ITEMS_SIZE(cmd, items)) { switch (item->type) { case KDBUS_ITEM_ATTACH_FLAGS: @@ -1414,13 +1401,6 @@ int kdbus_conn_new(struct kdbus_ep *ep, BUG_ON(*c); - /* Reject unknown flags */ - if (hello->conn_flags & ~(KDBUS_HELLO_ACCEPT_FD | - KDBUS_HELLO_ACTIVATOR | - KDBUS_HELLO_POLICY_HOLDER | - KDBUS_HELLO_MONITOR)) - return -EOPNOTSUPP; - is_monitor = hello->conn_flags & KDBUS_HELLO_MONITOR; is_activator = hello->conn_flags & KDBUS_HELLO_ACTIVATOR; is_policy_holder = hello->conn_flags & KDBUS_HELLO_POLICY_HOLDER; diff --git a/handle.c b/handle.c index 2bdd613..bca0261 100644 --- a/handle.c +++ b/handle.c @@ -299,6 +299,13 @@ static long kdbus_handle_ioctl_control(struct file *file, unsigned int cmd, make = p; + ret = kdbus_negotiate_flags(make->flags, buf, + offsetof(typeof(*make), flags), + KDBUS_MAKE_ACCESS_GROUP | + KDBUS_MAKE_ACCESS_WORLD); + if (ret < 0) + break; + ret = kdbus_items_validate(make->items, KDBUS_ITEMS_SIZE(make, items)); if (ret < 0) @@ -308,13 +315,6 @@ static long kdbus_handle_ioctl_control(struct file *file, unsigned int cmd, if (ret < 0) break; - /* Reject unknown flags */ - if (make->flags & ~(KDBUS_MAKE_ACCESS_GROUP | - KDBUS_MAKE_ACCESS_WORLD)) { - ret = -EOPNOTSUPP; - break; - } - if (make->flags & KDBUS_MAKE_ACCESS_WORLD) { mode = 0666; } else if (make->flags & KDBUS_MAKE_ACCESS_GROUP) { @@ -355,6 +355,13 @@ static long kdbus_handle_ioctl_control(struct file *file, unsigned int cmd, make = p; + ret = kdbus_negotiate_flags(make->flags, buf, + offsetof(typeof(*make), flags), + KDBUS_MAKE_ACCESS_GROUP | + KDBUS_MAKE_ACCESS_WORLD); + if (ret < 0) + break; + ret = kdbus_items_validate(make->items, KDBUS_ITEMS_SIZE(make, items)); if (ret < 0) @@ -366,13 +373,6 @@ static long kdbus_handle_ioctl_control(struct file *file, unsigned int cmd, if (ret < 0) break; - /* Reject unknown flags */ - if (make->flags & ~(KDBUS_MAKE_ACCESS_GROUP | - KDBUS_MAKE_ACCESS_WORLD)) { - ret = -EOPNOTSUPP; - break; - } - if (make->flags & KDBUS_MAKE_ACCESS_WORLD) mode = 0666; @@ -432,6 +432,13 @@ static long kdbus_handle_ioctl_ep(struct file *file, unsigned int cmd, make = p; + ret = kdbus_negotiate_flags(make->flags, buf, + offsetof(typeof(*make), flags), + KDBUS_MAKE_ACCESS_GROUP | + KDBUS_MAKE_ACCESS_WORLD); + if (ret < 0) + break; + ret = kdbus_items_validate(make->items, KDBUS_ITEMS_SIZE(make, items)); if (ret < 0) @@ -448,9 +455,6 @@ static long kdbus_handle_ioctl_ep(struct file *file, unsigned int cmd, } else if (make->flags & KDBUS_MAKE_ACCESS_GROUP) { mode = 0660; gid = current_fsgid(); - } else if (make->flags) { - ret = -EOPNOTSUPP; - break; } /* custom endpoints always have a policy db */ @@ -503,6 +507,16 @@ static long kdbus_handle_ioctl_ep(struct file *file, unsigned int cmd, hello = p; + ret = kdbus_negotiate_flags(hello->conn_flags, buf, + offsetof(struct kdbus_cmd_hello, + conn_flags), + KDBUS_HELLO_ACCEPT_FD | + KDBUS_HELLO_ACTIVATOR | + KDBUS_HELLO_POLICY_HOLDER | + KDBUS_HELLO_MONITOR); + if (ret < 0) + break; + ret = kdbus_items_validate(hello->items, KDBUS_ITEMS_SIZE(hello, items)); if (ret < 0) @@ -589,6 +603,14 @@ static long kdbus_handle_ioctl_ep_connected(struct file *file, unsigned int cmd, cmd_name = p; + ret = kdbus_negotiate_flags(cmd_name->flags, buf, + offsetof(typeof(*cmd_name), flags), + KDBUS_NAME_REPLACE_EXISTING | + KDBUS_NAME_ALLOW_REPLACEMENT | + KDBUS_NAME_QUEUE); + if (ret < 0) + break; + ret = kdbus_items_validate(cmd_name->items, KDBUS_ITEMS_SIZE(cmd_name, items)); if (ret < 0) @@ -623,6 +645,12 @@ static long kdbus_handle_ioctl_ep_connected(struct file *file, unsigned int cmd, cmd_name = p; + ret = kdbus_negotiate_flags(cmd_name->flags, buf, + offsetof(typeof(*cmd_name), flags), + 0); + if (ret < 0) + break; + ret = kdbus_items_validate(cmd_name->items, KDBUS_ITEMS_SIZE(cmd_name, items)); if (ret < 0) @@ -641,6 +669,15 @@ static long kdbus_handle_ioctl_ep_connected(struct file *file, unsigned int cmd, break; } + ret = kdbus_negotiate_flags(cmd_list.flags, buf, + offsetof(typeof(cmd_list), flags), + KDBUS_NAME_LIST_UNIQUE | + KDBUS_NAME_LIST_NAMES | + KDBUS_NAME_LIST_ACTIVATORS | + KDBUS_NAME_LIST_QUEUED); + if (ret < 0) + break; + ret = kdbus_cmd_name_list(conn->bus->name_registry, conn, &cmd_list); if (ret < 0) @@ -665,6 +702,13 @@ static long kdbus_handle_ioctl_ep_connected(struct file *file, unsigned int cmd, break; cmd_info = p; + + ret = kdbus_negotiate_flags(cmd_info->flags, buf, + offsetof(typeof(*cmd_info), flags), + _KDBUS_ATTACH_ALL); + if (ret < 0) + break; + ret = kdbus_cmd_conn_info(conn, cmd_info); if (ret < 0) break; @@ -693,6 +737,13 @@ static long kdbus_handle_ioctl_ep_connected(struct file *file, unsigned int cmd, cmd_update = p; + ret = kdbus_negotiate_flags(cmd_update->flags, buf, + offsetof(typeof(*cmd_update), + flags), + 0); + if (ret < 0) + break; + ret = kdbus_items_validate(cmd_update->items, KDBUS_ITEMS_SIZE(cmd_update, items)); if (ret < 0) @@ -719,6 +770,13 @@ static long kdbus_handle_ioctl_ep_connected(struct file *file, unsigned int cmd, cmd_match = p; + ret = kdbus_negotiate_flags(cmd_match->flags, buf, + offsetof(typeof(*cmd_match), + flags), + KDBUS_MATCH_REPLACE); + if (ret < 0) + break; + ret = kdbus_items_validate(cmd_match->items, KDBUS_ITEMS_SIZE(cmd_match, items)); if (ret < 0) @@ -746,6 +804,13 @@ static long kdbus_handle_ioctl_ep_connected(struct file *file, unsigned int cmd, cmd_match = p; + ret = kdbus_negotiate_flags(cmd_match->flags, buf, + offsetof(typeof(*cmd_match), + flags), + 0); + if (ret < 0) + break; + ret = kdbus_items_validate(cmd_match->items, KDBUS_ITEMS_SIZE(cmd_match, items)); if (ret < 0) @@ -797,11 +862,17 @@ static long kdbus_handle_ioctl_ep_connected(struct file *file, unsigned int cmd, break; } - /* handle a queued message */ ret = kdbus_copy_from_user(&cmd_recv, buf, sizeof(cmd_recv)); if (ret < 0) break; + ret = kdbus_negotiate_flags(cmd_recv.flags, buf, + offsetof(typeof(cmd_recv), flags), + KDBUS_RECV_PEEK | KDBUS_RECV_DROP | + KDBUS_RECV_USE_PRIORITY); + if (ret < 0) + break; + ret = kdbus_cmd_msg_recv(conn, &cmd_recv); if (ret < 0) break; @@ -849,8 +920,11 @@ static long kdbus_handle_ioctl_ep_connected(struct file *file, unsigned int cmd, if (ret < 0) break; - if (cmd_free.flags != 0) - return -EOPNOTSUPP; + ret = kdbus_negotiate_flags(cmd_free.flags, buf, + offsetof(typeof(cmd_free), flags), + 0); + if (ret < 0) + break; ret = kdbus_pool_release_offset(conn->pool, cmd_free.offset); break; @@ -887,6 +961,13 @@ static long kdbus_handle_ioctl_ep_owner(struct file *file, unsigned int cmd, cmd_update = p; + ret = kdbus_negotiate_flags(cmd_update->flags, buf, + offsetof(typeof(*cmd_update), + flags), + 0); + if (ret < 0) + break; + ret = kdbus_items_validate(cmd_update->items, KDBUS_ITEMS_SIZE(cmd_update, items)); if (ret < 0) diff --git a/kdbus.h b/kdbus.h index a9c4c80..453a8d1 100644 --- a/kdbus.h +++ b/kdbus.h @@ -26,6 +26,7 @@ #define KDBUS_DST_ID_NAME (0) #define KDBUS_MATCH_ID_ANY (~0ULL) #define KDBUS_DST_ID_BROADCAST (~0ULL) +#define KDBUS_FLAG_KERNEL (1ULL << 63) /** * struct kdbus_notify_id_change - name registry change message diff --git a/kdbus.txt b/kdbus.txt index 14d7719..00704b6 100644 --- a/kdbus.txt +++ b/kdbus.txt @@ -156,9 +156,12 @@ Note: use should be limited to the absolute minimum for the same reason. -3. Data Structures +3. Data Structures and flags =============================================================================== +3.1 Data structures and interconnections +---------------------------------------- + +-------------------------------------------------------------------------+ | Domain (Init Domain) | | /dev/kdbus/control | @@ -224,6 +227,19 @@ adds a ":1." prefix to the connection's unique ID. kbus itself doesn't use that notation, neither internally nor externally. However, libraries and other usespace code that aims for compatibility to D-Bus might. +3.2 Flags +--------- + +All ioctls used in the communication with the driver contain a 64-bit flags +field. All bits that are not recognized by the kernel are rejected, and the +ioctl fails with -EINVAL. Regardless of whether the kernel accepts the +provided flags or not, the flags field in the ioctl buffer will be updated with +all the bits the kernel driver knows about in its current state, and set the +highest bit (KDBUS_FLAGS_KERNEL) as well. Userspace can use the returned value +to negotiate features. The KDBUS_FLAGS_KERNEL bit will never be valid in any +flags field of any command, so setting it will always make the ioctl fail. +Hence, this is a way to probe possible kernel features. + 4. Items =============================================================================== @@ -1586,30 +1602,30 @@ For all ioctls that carry a struct as payload: For KDBUS_CMD_BUS_MAKE: - -EOPNOTSUPP The flags supplied in the kdbus_cmd_make struct are invalid - -EINVAL The supplied name does not start with the current uid and a '-' + -EINVAL The flags supplied in the kdbus_cmd_make struct are invalid or + the supplied name does not start with the current uid and a '-' -EEXIST A bus of that name already exists -ESHUTDOWN The domain for the bus is already shut down -EMFILE The maximum number of buses for the current user is exhausted -KDBUS_CMD_DOMAIN_MAKE +For KDBUS_CMD_DOMAIN_MAKE: - -EPERM The calling user does not have CAP_IPC_OWNER set - -EOPNOTSUPP The flags supplied in the kdbus_cmd_make struct are invalid - -EINVAL No name supplied for top-level domain + -EPERM The calling user does not have CAP_IPC_OWNER set, or + -EINVAL The flags supplied in the kdbus_cmd_make struct are invalid, or + no name supplied for top-level domain -EEXIST A domain of that name already exists -KDBUS_CMD_ENDPOINT_MAKE +For KDBUS_CMD_ENDPOINT_MAKE: -EPERM The calling user is not privileged (see Terminology) - -EOPNOTSUPP The flags supplied in the kdbus_cmd_make struct are invalid + -EINVAL The flags supplied in the kdbus_cmd_make struct are invalid -EEXIST An endpoint of that name already exists -KDBUS_CMD_HELLO +For KDBUS_CMD_HELLO: - -EOPNOTSUPP The flags supplied in the kdbus_cmd_make struct are invalid -EFAULT The supplied pool size was 0 or not a multiple of the page size - -EINVAL An illegal combination of KDBUS_HELLO_MONITOR, + -EINVAL The flags supplied in the kdbus_cmd_make struct are invalid, or + an illegal combination of KDBUS_HELLO_MONITOR, KDBUS_HELLO_ACTIVATOR and KDBUS_HELLO_POLICY_HOLDER was passed in the flags, or an invalid set of items was supplied -EPERM An KDBUS_ITEM_CREDS items was supplied, but the current user is @@ -1617,12 +1633,12 @@ KDBUS_CMD_HELLO -ESHUTDOWN The bus has already been shut down -EMFILE The maximum number of connection on the bus has been reached -KDBUS_CMD_BYEBYE +For KDBUS_CMD_BYEBYE: -EALREADY The connection has already been shut down -EBUSY There are still messages queued up in the connection's pool -KDBUS_CMD_MSG_SEND +For KDBUS_CMD_MSG_SEND: -EOPNOTSUPP The connection is unconnected, or a fd was passed that is either a kdbus handle itself or a unix domain socket. Both is @@ -1676,20 +1692,21 @@ KDBUS_CMD_MSG_SEND For KDBUS_CMD_MSG_RECV: - -EINVAL The offset parameter was != 0 when entering the ioctl + -EINVAL Invalid flags or offset -EAGAIN No message found in the queue -ENOMSG No message of the requested priority found For KDBUS_CMD_MSG_CANCEL: + -EINVAL Invalid flags -ENOENT Pending message with the supplied cookie not found For KDBUS_CMD_FREE: -ENXIO No pool slice found at given offset - -EINVAL The offset is valid, but the user is not allowed to free the - slice. This happens, for example, if the offset was retrieved - with KDBUS_RECV_PEEK. + -EINVAL Invalid flags provided, the offset is valid, but the user is + not allowed to free the slice. This happens, for example, if + the offset was retrieved with KDBUS_RECV_PEEK. For KDBUS_CMD_NAME_ACQUIRE: @@ -1702,21 +1719,25 @@ For KDBUS_CMD_NAME_ACQUIRE: For KDBUS_CMD_NAME_RELEASE: + -EINVAL Invalid command flags provided -ESRCH Name is not found found in the registry -EADDRINUSE Name is owned by a different connection and can't be released For KDBUS_CMD_NAME_LIST: + + -EINVAL Invalid flags -ENOBUFS No available memory in the connection's pool. For KDBUS_CMD_CONN_INFO: - -EINVAL Neither an ID nor a name was provided, or the name is not - valid. + + -EINVAL Invalid flags, or neither an ID nor a name was provided, + or the name is invalid. -ESRCH Connection lookup by name failed -ENXIO No connection with the provided number connection ID found For KDBUS_CMD_CONN_UPDATE: - -EINVAL Invalid item attached + -EINVAL Illegal flags or items -EOPNOTSUPP Connection is not attached to bus -E2BIG Too many policy items attached -EINVAL Wildcards submitted in policy entries, or illegal sequence @@ -1725,15 +1746,16 @@ For KDBUS_CMD_CONN_UPDATE: For KDBUS_CMD_ENDPOINT_UPDATE: -E2BIG Too many policy items attached - -EINVAL Wildcards submitted in policy entries, or illegal sequence - of policy items + -EINVAL Invalid flags, or wildcards submitted in policy entries, + or illegal sequence of policy items For KDBUS_CMD_MATCH_ADD: - -EINVAL Illegal items + -EINVAL Illegal flags or items -EDOM Illegal bloom filter size -EMFILE Too many matches for this connection For KDBUS_CMD_MATCH_REMOVE: + -EINVAL Illegal flags -ENOENT A match entry with the given cookie could not be found. diff --git a/match.c b/match.c index c3296ba..689c0e2 100644 --- a/match.c +++ b/match.c @@ -372,9 +372,6 @@ int kdbus_match_db_add(struct kdbus_conn *conn, lockdep_assert_held(conn); - if (cmd->flags != 0) - return -EOPNOTSUPP; - entry = kzalloc(sizeof(*entry), GFP_KERNEL); if (!entry) { ret = -ENOMEM; @@ -517,9 +514,6 @@ int kdbus_match_db_remove(struct kdbus_conn *conn, lockdep_assert_held(conn); - if (cmd->flags != 0) - return -EOPNOTSUPP; - mutex_lock(&db->entries_lock); ret = __kdbus_match_db_remove_unlocked(db, cmd->cookie); mutex_unlock(&db->entries_lock); diff --git a/message.c b/message.c index 3232dcc..1782c07 100644 --- a/message.c +++ b/message.c @@ -262,7 +262,7 @@ static int kdbus_msg_scan_items(struct kdbus_conn *conn, * Return: 0 on success, negative errno on failure. */ int kdbus_kmsg_new_from_user(struct kdbus_conn *conn, - const struct kdbus_msg __user *msg, + struct kdbus_msg __user *msg, struct kdbus_kmsg **kmsg) { struct kdbus_kmsg *m; @@ -303,13 +303,13 @@ int kdbus_kmsg_new_from_user(struct kdbus_conn *conn, goto exit_free; } - /* Reject unknown flags */ - if (m->msg.flags & ~(KDBUS_MSG_FLAGS_EXPECT_REPLY | - KDBUS_MSG_FLAGS_SYNC_REPLY | - KDBUS_MSG_FLAGS_NO_AUTO_START)) { - ret = -EOPNOTSUPP; + ret = kdbus_negotiate_flags(m->msg.flags, msg, + offsetof(struct kdbus_msg, flags), + KDBUS_MSG_FLAGS_EXPECT_REPLY | + KDBUS_MSG_FLAGS_SYNC_REPLY | + KDBUS_MSG_FLAGS_NO_AUTO_START); + if (ret < 0) goto exit_free; - } if (m->msg.flags & KDBUS_MSG_FLAGS_EXPECT_REPLY) { /* requests for replies need a timeout */ diff --git a/message.h b/message.h index f672e0e..a12b6a7 100644 --- a/message.h +++ b/message.h @@ -65,7 +65,7 @@ struct kdbus_conn; int kdbus_kmsg_new(size_t extra_size, struct kdbus_kmsg **kmsg); int kdbus_kmsg_new_from_user(struct kdbus_conn *conn, - const struct kdbus_msg __user *msg, + struct kdbus_msg __user *msg, struct kdbus_kmsg **kmsg); void kdbus_kmsg_free(struct kdbus_kmsg *kmsg); #endif diff --git a/names.c b/names.c index cc68a2f..917d66c 100644 --- a/names.c +++ b/names.c @@ -642,11 +642,6 @@ int kdbus_cmd_name_acquire(struct kdbus_name_registry *reg, const char *name; int ret; - if (cmd->flags & ~(KDBUS_NAME_REPLACE_EXISTING | - KDBUS_NAME_ALLOW_REPLACEMENT | - KDBUS_NAME_QUEUE)) - return -EOPNOTSUPP; - ret = kdbus_items_get_str(cmd->items, KDBUS_ITEMS_SIZE(cmd, items), KDBUS_ITEM_NAME, &name); if (ret < 0) @@ -688,9 +683,6 @@ int kdbus_cmd_name_release(struct kdbus_name_registry *reg, int ret; const char *name; - if (cmd->flags != 0) - return -EOPNOTSUPP; - ret = kdbus_items_get_str(cmd->items, KDBUS_ITEMS_SIZE(cmd, items), KDBUS_ITEM_NAME, &name); if (ret < 0) @@ -879,13 +871,6 @@ int kdbus_cmd_name_list(struct kdbus_name_registry *reg, policy_db = &conn->ep->policy_db; - /* Reject unknown flags */ - if (cmd->flags & ~(KDBUS_NAME_LIST_UNIQUE | - KDBUS_NAME_LIST_NAMES | - KDBUS_NAME_LIST_ACTIVATORS | - KDBUS_NAME_LIST_QUEUED)) - return -EOPNOTSUPP; - /* lock order: domain -> bus -> ep -> names -> conn */ down_read(&conn->bus->conn_rwlock); down_read(®->rwlock); diff --git a/test/test-connection.c b/test/test-connection.c index bc397d9..de74c34 100644 --- a/test/test-connection.c +++ b/test/test-connection.c @@ -57,6 +57,7 @@ int kdbus_test_bus_make(struct kdbus_test_env *env) 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; + bus_make.head.flags = 0; ret = ioctl(env->control_fd, KDBUS_CMD_BUS_MAKE, &bus_make); ASSERT_RETURN(ret == -1 && errno == EINVAL); @@ -65,6 +66,7 @@ int kdbus_test_bus_make(struct kdbus_test_env *env) 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; + bus_make.head.flags = 0; ret = ioctl(env->control_fd, KDBUS_CMD_BUS_MAKE, &bus_make); ASSERT_RETURN(ret == -1 && errno == EINVAL); @@ -73,6 +75,7 @@ int kdbus_test_bus_make(struct kdbus_test_env *env) 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; + bus_make.head.flags = 0; ret = ioctl(env->control_fd, KDBUS_CMD_BUS_MAKE, &bus_make); ASSERT_RETURN(ret == -1 && errno == EINVAL); @@ -81,6 +84,7 @@ int kdbus_test_bus_make(struct kdbus_test_env *env) 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; + bus_make.head.flags = 0; ret = ioctl(env->control_fd, KDBUS_CMD_BUS_MAKE, &bus_make); ASSERT_RETURN(ret == 0); snprintf(s, sizeof(s), "/dev/" KBUILD_MODNAME "/%u-blah-1/bus", uid); @@ -115,6 +119,7 @@ int kdbus_test_hello(struct kdbus_test_env *env) /* a size of 0 must return EMSGSIZE */ hello.size = 1; + hello.conn_flags = KDBUS_HELLO_ACCEPT_FD; ret = ioctl(fd, KDBUS_CMD_HELLO, &hello); ASSERT_RETURN(ret == -1 && errno == EINVAL); @@ -123,22 +128,28 @@ int kdbus_test_hello(struct kdbus_test_env *env) /* check faulty flags */ hello.conn_flags = 1ULL << 32; ret = ioctl(fd, KDBUS_CMD_HELLO, &hello); - ASSERT_RETURN(ret == -1 && errno == EOPNOTSUPP); + ASSERT_RETURN(ret == -1 && errno == EINVAL); + + /* kernel must have set its bit in the ioctl buffer */ + ASSERT_RETURN(hello.conn_flags & KDBUS_FLAG_KERNEL); hello.conn_flags = KDBUS_HELLO_ACCEPT_FD; /* check for faulty pool sizes */ hello.pool_size = 0; + hello.conn_flags = KDBUS_HELLO_ACCEPT_FD; ret = ioctl(fd, KDBUS_CMD_HELLO, &hello); ASSERT_RETURN(ret == -1 && errno == EFAULT); hello.pool_size = 4097; + hello.conn_flags = KDBUS_HELLO_ACCEPT_FD; ret = ioctl(fd, KDBUS_CMD_HELLO, &hello); ASSERT_RETURN(ret == -1 && errno == EFAULT); hello.pool_size = POOL_SIZE; /* success test */ + hello.conn_flags = KDBUS_HELLO_ACCEPT_FD; ret = ioctl(fd, KDBUS_CMD_HELLO, &hello); ASSERT_RETURN(ret == 0); diff --git a/test/test-domain.c b/test/test-domain.c index c223e48..9d7b08a 100644 --- a/test/test-domain.c +++ b/test/test-domain.c @@ -38,6 +38,7 @@ int kdbus_test_domain_make(struct kdbus_test_env *env) snprintf(domain_make.name, sizeof(domain_make.name), "blah"); domain_make.n_size = KDBUS_ITEM_HEADER_SIZE + strlen(domain_make.name) + 1; domain_make.head.size = sizeof(struct kdbus_cmd_make) + domain_make.n_size; + domain_make.head.flags = 0; ret = ioctl(fd, KDBUS_CMD_DOMAIN_MAKE, &domain_make); if (ret < 0 && errno == EPERM) return TEST_SKIP; @@ -47,11 +48,13 @@ int kdbus_test_domain_make(struct kdbus_test_env *env) F_OK) == 0); /* can't use the same fd for domain make twice */ + domain_make.head.flags = 0; ret = ioctl(fd, KDBUS_CMD_DOMAIN_MAKE, &domain_make); ASSERT_RETURN(ret == -1 && errno == EBADFD); /* can't register the same name twice */ fd2 = open("/dev/" KBUILD_MODNAME "/control", O_RDWR|O_CLOEXEC); + domain_make.head.flags = 0; ret = ioctl(fd2, KDBUS_CMD_DOMAIN_MAKE, &domain_make); ASSERT_RETURN(ret == -1 && errno == EEXIST); close(fd2); diff --git a/test/test-free.c b/test/test-free.c index 5a14e2b..abac022 100644 --- a/test/test-free.c +++ b/test/test-free.c @@ -19,14 +19,14 @@ int kdbus_test_free(struct kdbus_test_env *env) int ret; struct kdbus_cmd_free cmd_free; + /* free an unallocated buffer */ cmd_free.flags = 0; cmd_free.offset = 0; - - /* free an unallocated buffer */ ret = ioctl(env->conn->fd, KDBUS_CMD_FREE, &cmd_free); ASSERT_RETURN(ret == -1 && errno == ENXIO); /* free a buffer out of the pool's bounds */ + cmd_free.flags = 0; cmd_free.offset = POOL_SIZE + 1; ret = ioctl(env->conn->fd, KDBUS_CMD_FREE, &cmd_free); ASSERT_RETURN(ret == -1 && errno == ENXIO); diff --git a/util.c b/util.c index 58da71e..af4cd70 100644 --- a/util.c +++ b/util.c @@ -13,6 +13,7 @@ #include #include +#include #include "limits.h" #include "util.h" @@ -47,3 +48,37 @@ int kdbus_sysname_is_valid(const char *name) return 0; } + +/** + * kdbus_negotiate_flags() - check flags provided by user, and write the + * valid mask back + * @flags: The flags mask provided by userspace + * @buf: The buffer provided by userspace + * @offset: Offset of the flags field inside the user-provided struct + * @valid: Mask of valid bits + * + * This function will check whether the flags provided by userspace are within + * the combination of allowed bits to the kernel, with the KDBUS_FLAGS_KERNEL + * bit set in the return buffer. + * + * Return: 0 on success, -EFAULT if copy_to_user() failed, or -EINVAL if + * userspace submitted invalid bits in its mask. + */ +int kdbus_negotiate_flags(u64 flags, void __user *buf, off_t offset, u64 valid) +{ + u64 val = valid | KDBUS_FLAG_KERNEL; + + /* + * KDBUS_FLAG_KERNEL is reserved. Make sure it is never considered + * valid by any user of this function. + */ + BUG_ON(valid & KDBUS_FLAG_KERNEL); + + if (copy_to_user(((u8 __user *) buf) + offset, &val, sizeof(val))) + return -EFAULT; + + if (flags & ~valid) + return -EINVAL; + + return 0; +} diff --git a/util.h b/util.h index 5f50dc0..5bbfc6e 100644 --- a/util.h +++ b/util.h @@ -83,5 +83,6 @@ static inline bool kdbus_str_valid(const char *str, size_t size) } int kdbus_sysname_is_valid(const char *name); +int kdbus_negotiate_flags(u64 flags, void __user *buf, off_t offset, u64 valid); #endif -- 2.34.1