From 633c65c2e471a421e2094365bfeea5bd98abdc18 Mon Sep 17 00:00:00 2001 From: Corey Wharton Date: Sat, 29 Feb 2020 14:51:42 -0800 Subject: [PATCH 01/19] scripts: Dynamically add driver subsystems to subsystems list This change extends the parse_syscalls.py script to scan for a __subsystem sentinal added to driver api declarations. It thens generates a list that is passed into gen_kobject_list.py to extend the subsystems list. This allows subsystems to be declared in the code instead of a separate python list and provides a mechanism for defining out-of-tree subsystems. Signed-off-by: Corey Wharton --- CMakeLists.txt | 18 +++++++-- cmake/kobj.cmake | 5 ++- include/toolchain/common.h | 5 +++ scripts/gen_kobject_list.py | 17 +++++++- scripts/parse_syscalls.py | 78 ++++++++++++++++++++++++------------- 5 files changed, 90 insertions(+), 33 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c786770bee4f2..82407c7284c4e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -494,6 +494,7 @@ endif() set(syscall_list_h ${CMAKE_CURRENT_BINARY_DIR}/include/generated/syscall_list.h) set(syscalls_json ${CMAKE_CURRENT_BINARY_DIR}/misc/generated/syscalls.json) +set(subsys_json ${CMAKE_CURRENT_BINARY_DIR}/misc/generated/subsystems.json) # The syscalls subdirs txt file is constructed by python containing a list of folders to use for # dependency handling, including empty folders. @@ -585,12 +586,14 @@ endforeach() add_custom_command( OUTPUT ${syscalls_json} + ${subsys_json} COMMAND ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/parse_syscalls.py --include ${ZEPHYR_BASE}/include # Read files from this dir ${parse_syscalls_include_args} # Read files from these dirs also - --json-file ${syscalls_json} # Write this file + --json-file ${syscalls_json} # Write this file + --subsystem-file ${subsys_json} # Write subsystem list to this file DEPENDS ${syscalls_subdirs_trigger} ${PARSE_SYSCALLS_HEADER_DEPENDS} ) @@ -617,6 +620,9 @@ add_custom_command(OUTPUT include/generated/syscall_dispatch.c ${syscall_list_h} DEPENDS ${syscalls_json} ) +# This is passed into all calls to the gen_kobject_list.py script. +set(gen_kobject_list_include_args --include ${subsys_json}) + set(DRV_VALIDATION ${PROJECT_BINARY_DIR}/include/generated/driver-validation.h) add_custom_command( OUTPUT ${DRV_VALIDATION} @@ -624,8 +630,11 @@ add_custom_command( ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/gen_kobject_list.py --validation-output ${DRV_VALIDATION} + ${gen_kobject_list_include_args} $<$:--verbose> - DEPENDS ${ZEPHYR_BASE}/scripts/gen_kobject_list.py + DEPENDS + ${ZEPHYR_BASE}/scripts/gen_kobject_list.py + ${subsys_json} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) add_custom_target(${DRIVER_VALIDATION_H_TARGET} DEPENDS ${DRV_VALIDATION}) @@ -941,8 +950,11 @@ if(CONFIG_USERSPACE) ${GEN_KOBJ_LIST} --kernel $ --gperf-output ${OBJ_LIST} + ${gen_kobject_list_include_args} $<$:--verbose> - DEPENDS ${ZEPHYR_PREBUILT_EXECUTABLE} + DEPENDS + ${ZEPHYR_PREBUILT_EXECUTABLE} + ${subsys_json} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) add_custom_target(obj_list DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${OBJ_LIST}) diff --git a/cmake/kobj.cmake b/cmake/kobj.cmake index bd16d3538d9f0..f54e148a60d83 100644 --- a/cmake/kobj.cmake +++ b/cmake/kobj.cmake @@ -21,8 +21,11 @@ function(gen_kobj gen_dir_out) --kobj-types-output ${KOBJ_TYPES} --kobj-otype-output ${KOBJ_OTYPE} --kobj-size-output ${KOBJ_SIZE} + ${gen_kobject_list_include_args} $<$:--verbose> - DEPENDS $ENV{ZEPHYR_BASE}/scripts/gen_kobject_list.py + DEPENDS + $ENV{ZEPHYR_BASE}/scripts/gen_kobject_list.py + ${subsys_json} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) add_custom_target(${KOBJ_TYPES_H_TARGET} DEPENDS ${KOBJ_TYPES} ${KOBJ_OTYPE}) diff --git a/include/toolchain/common.h b/include/toolchain/common.h index 6f48245fa7ab4..331ed433be059 100644 --- a/include/toolchain/common.h +++ b/include/toolchain/common.h @@ -132,6 +132,11 @@ #define __syscall #endif /* #ifndef ZTEST_UNITTEST */ +/* Used as a sentinel by parse_syscalls.py to identify what API structs + * define driver subsystems. + */ +#define __subsystem + #ifndef BUILD_ASSERT /* compile-time assertion that makes the build fail */ #define BUILD_ASSERT(EXPR) \ diff --git a/scripts/gen_kobject_list.py b/scripts/gen_kobject_list.py index 596102627c71e..f0f9811588799 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -56,6 +56,7 @@ import math import os import struct +import json from elf_helper import ElfHelper, kobject_to_enum from collections import OrderedDict @@ -90,8 +91,6 @@ ("k_futex", (None, True)) ]) - - subsystems = [ "adc_driver_api", "aio_cmp_driver_api", @@ -331,6 +330,11 @@ def write_kobj_size_output(fp): fp.write("#endif\n") +def parse_subsystems_list_file(path): + with open(path, "r") as fp: + subsys_list = json.load(fp) + subsystems.extend(subsys_list) + def parse_args(): global args @@ -355,6 +359,11 @@ def parse_args(): parser.add_argument( "-Z", "--kobj-size-output", required=False, help="Output case statements for obj_size_get()") + parser.add_argument("-i", "--include-subsystem-list", required=False, action='append', + help='''Specifies a file with a JSON encoded list of subsystem names to append to + the driver subsystems list. Can be specified multiple times: + -i file1 -i file2 ...''') + parser.add_argument("-v", "--verbose", action="store_true", help="Print extra debugging information") args = parser.parse_args() @@ -365,6 +374,10 @@ def parse_args(): def main(): parse_args() + if args.include_subsystem_list is not None: + for list_file in args.include_subsystem_list: + parse_subsystems_list_file(list_file) + if args.gperf_output: assert args.kernel, "--kernel ELF required for --gperf-output" eh = ElfHelper(args.kernel, args.verbose, kobjects, subsystems) diff --git a/scripts/parse_syscalls.py b/scripts/parse_syscalls.py index 2d79b535b0776..25c56223da84e 100644 --- a/scripts/parse_syscalls.py +++ b/scripts/parse_syscalls.py @@ -5,7 +5,7 @@ # SPDX-License-Identifier: Apache-2.0 """ -Script to scan Zephyr include directories and emit system call metadata +Script to scan Zephyr include directories and emit system call and subsystem metadata System calls require a great deal of boilerplate code in order to implement completely. This script is the first step in the build system's process of @@ -26,7 +26,7 @@ import os import json -api_regex = re.compile(r''' +syscall_regex = re.compile(r''' __syscall\s+ # __syscall attribute, must be first ([^(]+) # type and name of system call (split later) [(] # Function opening parenthesis @@ -34,9 +34,16 @@ [)] # Closing parenthesis ''', re.MULTILINE | re.VERBOSE) +subsys_regex = re.compile(r''' +__subsystem\s+ # __subsystem attribute, must be first +struct\s+ # struct keyword is next +([^{]+) # name of subsystem +[{] # Open curly bracket +''', re.MULTILINE | re.VERBOSE) def analyze_headers(multiple_directories): - ret = [] + syscall_ret = [] + subsys_ret = [] for base_path in multiple_directories: for root, dirs, files in os.walk(base_path, topdown=True): @@ -44,23 +51,41 @@ def analyze_headers(multiple_directories): files.sort() for fn in files: - # toolchain/common.h has the definition of __syscall which we + # toolchain/common.h has the definitions of __syscall and __subsystem which we # don't want to trip over path = os.path.join(root, fn) if not fn.endswith(".h") or path.endswith(os.path.join(os.sep, 'toolchain', 'common.h')): continue with open(path, "r", encoding="utf-8") as fp: - try: - result = [(mo.groups(), fn) - for mo in api_regex.finditer(fp.read())] - except Exception: - sys.stderr.write("While parsing %s\n" % fn) - raise + contents = fp.read() + + try: + syscall_result = [(mo.groups(), fn) + for mo in syscall_regex.finditer(contents)] + subsys_result = [mo.groups()[0].strip() + for mo in subsys_regex.finditer(contents)] + except Exception: + sys.stderr.write("While parsing %s\n" % fn) + raise - ret.extend(result) + syscall_ret.extend(syscall_result) + subsys_ret.extend(subsys_result) - return ret + return syscall_ret, subsys_ret + + +def update_file_if_changed(path, new): + if os.path.exists(path): + with open(path, 'r') as fp: + old = fp.read() + + if new != old: + with open(path, 'w') as fp: + fp.write(new) + else: + with open(path, 'w') as fp: + fp.write(new) def parse_args(): @@ -76,34 +101,33 @@ def parse_args(): parser.add_argument( "-j", "--json-file", required=True, help="Write system call prototype information as json to file") + parser.add_argument( + "-s", "--subsystem-file", required=True, + help="Write subsystem name information as json to file") args = parser.parse_args() def main(): parse_args() - syscalls = analyze_headers(args.include) + syscalls, subsys = analyze_headers(args.include) + + # Only write json files if they don't exist or have changes since + # they will force and incremental rebuild. syscalls_in_json = json.dumps( syscalls, indent=4, sort_keys=True ) + update_file_if_changed(args.json_file, syscalls_in_json) - # Check if the file already exists, and if there are no changes, - # don't touch it since that will force an incremental rebuild - path = args.json_file - new = syscalls_in_json - if os.path.exists(path): - with open(path, 'r') as fp: - old = fp.read() - - if new != old: - with open(path, 'w') as fp: - fp.write(new) - else: - with open(path, 'w') as fp: - fp.write(new) + subsys_in_json = json.dumps( + subsys, + indent=4, + sort_keys=True + ) + update_file_if_changed(args.subsystem_file, subsys_in_json) if __name__ == "__main__": From 885358920a556e232e739f4fb7d5f2cb1145aa01 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Mon, 16 Mar 2020 12:48:00 -0700 Subject: [PATCH 02/19] build: prevent subsystems.json from being gen Actual files make terrible dependency targets in CMake. Wrap the generation of subsystems.json into a custom target to get around this. Fixes a problem where parse_syscalls.py was being called multiple times. Fixes: #23504 Signed-off-by: Andrew Boie --- CMakeLists.txt | 10 ++++++---- cmake/kobj.cmake | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 82407c7284c4e..96b499909ce3d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -54,7 +54,7 @@ set(SYSCALL_LIST_H_TARGET syscall_list_h_target) set(DRIVER_VALIDATION_H_TARGET driver_validation_h_target) set(KOBJ_TYPES_H_TARGET kobj_types_h_target) set(LINKER_SCRIPT_TARGET linker_script_target) - +set(PARSE_SYSCALLS_TARGET parse_syscalls_target) define_property(GLOBAL PROPERTY PROPERTY_OUTPUT_FORMAT BRIEF_DOCS " " FULL_DOCS " ") set_property( GLOBAL PROPERTY PROPERTY_OUTPUT_FORMAT elf32-little${ARCH}) # BFD format @@ -598,6 +598,8 @@ add_custom_command( ) add_custom_target(${SYSCALL_LIST_H_TARGET} DEPENDS ${syscall_list_h}) +add_custom_target(${PARSE_SYSCALLS_TARGET} + DEPENDS ${syscalls_json} ${subsys_json}) # 64-bit systems do not require special handling of 64-bit system call # parameters or return values, indicate this to the system call boilerplate @@ -617,7 +619,7 @@ add_custom_command(OUTPUT include/generated/syscall_dispatch.c ${syscall_list_h} --syscall-list ${syscall_list_h} ${SYSCALL_LONG_REGISTERS_ARG} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} - DEPENDS ${syscalls_json} + DEPENDS ${PARSE_SYSCALLS_TARGET} ) # This is passed into all calls to the gen_kobject_list.py script. @@ -634,7 +636,7 @@ add_custom_command( $<$:--verbose> DEPENDS ${ZEPHYR_BASE}/scripts/gen_kobject_list.py - ${subsys_json} + ${PARSE_SYSCALLS_TARGET} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) add_custom_target(${DRIVER_VALIDATION_H_TARGET} DEPENDS ${DRV_VALIDATION}) @@ -954,7 +956,7 @@ if(CONFIG_USERSPACE) $<$:--verbose> DEPENDS ${ZEPHYR_PREBUILT_EXECUTABLE} - ${subsys_json} + ${PARSE_SYSCALLS_TARGET} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) add_custom_target(obj_list DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${OBJ_LIST}) diff --git a/cmake/kobj.cmake b/cmake/kobj.cmake index f54e148a60d83..66393373276a3 100644 --- a/cmake/kobj.cmake +++ b/cmake/kobj.cmake @@ -25,7 +25,7 @@ function(gen_kobj gen_dir_out) $<$:--verbose> DEPENDS $ENV{ZEPHYR_BASE}/scripts/gen_kobject_list.py - ${subsys_json} + ${PARSE_SYSCALLS_TARGET} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) add_custom_target(${KOBJ_TYPES_H_TARGET} DEPENDS ${KOBJ_TYPES} ${KOBJ_OTYPE}) From 685b2e38a7ffa35bec3ce7887b6c6750d52510f6 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Wed, 11 Mar 2020 06:37:42 -0700 Subject: [PATCH 03/19] kernel: use a union for kobject data values Rather than stuffing various values in a uintptr_t based on type using casts, use a union for this instead. No functional difference, but the semantics of the data member are now much clearer to the casual observer since it is now formally defined by this union. Signed-off-by: Andrew Boie --- doc/reference/usermode/kernelobjects.rst | 5 ++--- include/kernel.h | 21 ++++++++++++++++++++- kernel/futex.c | 2 +- kernel/thread.c | 6 +++--- kernel/userspace.c | 8 ++++---- lib/os/mutex.c | 2 +- scripts/elf_helper.py | 4 ++-- scripts/gen_kobject_list.py | 14 +++++++++++++- 8 files changed, 46 insertions(+), 16 deletions(-) diff --git a/doc/reference/usermode/kernelobjects.rst b/doc/reference/usermode/kernelobjects.rst index f493d32ac4ebd..c12145042d90b 100644 --- a/doc/reference/usermode/kernelobjects.rst +++ b/doc/reference/usermode/kernelobjects.rst @@ -128,9 +128,8 @@ includes: instance of :cpp:enum:`k_objects`. * A set of flags for that object. This is currently used to track initialization state and whether an object is public or not. -* An extra data field. This is currently used for thread stack objects - to denote how large the stack is, and for thread objects to indicate - the thread's index in kernel object permission bitfields. +* An extra data field. The semantics of this field vary by object type, see + the definition of :c:type:`union z_object_data`. Dynamic objects allocated at runtime are tracked in a runtime red/black tree which is used in parallel to the gperf table when validating object pointers. diff --git a/include/kernel.h b/include/kernel.h index 9e70c05cc2110..f6abf0e6efd3b 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -135,6 +135,7 @@ struct k_poll_signal; struct k_mem_domain; struct k_mem_partition; struct k_futex; +struct z_futex_data; /** * @brief Kernel Object Types @@ -165,6 +166,24 @@ enum k_objects { */ #ifdef CONFIG_USERSPACE +/* Object extra data. Only some objects use this, determined by object type */ +union z_object_data { + /* Backing mutex for K_OBJ_SYS_MUTEX */ + struct k_mutex *mutex; + + /* Numerical thread ID for K_OBJ_THREAD */ + unsigned int thread_id; + + /* Stack buffer size for K_OBJ__THREAD_STACK_ELEMENT */ + size_t stack_size; + + /* Futex wait queue and spinlock for K_OBJ_FUTEX */ + struct z_futex_data *futex_data; + + /* All other objects */ + int unused; +}; + /* Table generated by gperf, these objects are retrieved via * z_object_find() */ struct _k_object { @@ -172,7 +191,7 @@ struct _k_object { u8_t perms[CONFIG_MAX_THREAD_BYTES]; u8_t type; u8_t flags; - uintptr_t data; + union z_object_data data; } __packed __aligned(4); struct _k_object_assignment { diff --git a/kernel/futex.c b/kernel/futex.c index c31fb0892acda..8507cd2d01450 100644 --- a/kernel/futex.c +++ b/kernel/futex.c @@ -21,7 +21,7 @@ static struct z_futex_data *k_futex_find_data(struct k_futex *futex) return NULL; } - return (struct z_futex_data *)obj->data; + return obj->data.futex_data; } int z_impl_k_futex_wake(struct k_futex *futex, bool wake_all) diff --git a/kernel/thread.c b/kernel/thread.c index 2d3517f8ba20b..8feee474b47b3 100644 --- a/kernel/thread.c +++ b/kernel/thread.c @@ -642,9 +642,9 @@ k_tid_t z_vrfy_k_thread_create(struct k_thread *new_thread, /* Testing less-than-or-equal since additional room may have been * allocated for alignment constraints */ - Z_OOPS(Z_SYSCALL_VERIFY_MSG(total_size <= stack_object->data, - "stack size %zu is too big, max is %lu", - total_size, stack_object->data)); + Z_OOPS(Z_SYSCALL_VERIFY_MSG(total_size <= stack_object->data.stack_size, + "stack size %zu is too big, max is %zu", + total_size, stack_object->data.stack_size)); /* User threads may only create other user threads and they can't * be marked as essential diff --git a/kernel/userspace.c b/kernel/userspace.c index c54a22898f18c..26b5f5bed0849 100644 --- a/kernel/userspace.c +++ b/kernel/userspace.c @@ -263,7 +263,7 @@ void *z_impl_k_object_alloc(enum k_objects otype) return NULL; } - dyn_obj->kobj.data = tidx; + dyn_obj->kobj.data.thread_id = tidx; } /* The allocating thread implicitly gets permission on kernel objects @@ -297,7 +297,7 @@ void k_object_free(void *obj) sys_dlist_remove(&dyn_obj->obj_list); if (dyn_obj->kobj.type == K_OBJ_THREAD) { - thread_idx_free(dyn_obj->kobj.data); + thread_idx_free(dyn_obj->kobj.data.thread_id); } } k_spin_unlock(&objfree_lock, key); @@ -340,7 +340,7 @@ void z_object_wordlist_foreach(_wordlist_cb_func_t func, void *context) } #endif /* CONFIG_DYNAMIC_OBJECTS */ -static int thread_index_get(struct k_thread *thread) +static unsigned int thread_index_get(struct k_thread *thread) { struct _k_object *ko; @@ -350,7 +350,7 @@ static int thread_index_get(struct k_thread *thread) return -1; } - return ko->data; + return ko->data.thread_id; } static void unref_check(struct _k_object *ko, uintptr_t index) diff --git a/lib/os/mutex.c b/lib/os/mutex.c index 84f207074ffa6..0e952e8e940c3 100644 --- a/lib/os/mutex.c +++ b/lib/os/mutex.c @@ -18,7 +18,7 @@ static struct k_mutex *get_k_mutex(struct sys_mutex *mutex) return NULL; } - return (struct k_mutex *)obj->data; + return obj->data.mutex; } static bool check_sys_mutex_addr(struct sys_mutex *addr) diff --git a/scripts/elf_helper.py b/scripts/elf_helper.py index 44b53162e2ea3..f5842d2f50f44 100644 --- a/scripts/elf_helper.py +++ b/scripts/elf_helper.py @@ -71,10 +71,10 @@ def __init__(self, type_obj, addr): self.data = thread_counter thread_counter = thread_counter + 1 elif self.type_obj.name == "sys_mutex": - self.data = "(uintptr_t)(&kernel_mutexes[%d])" % sys_mutex_counter + self.data = "&kernel_mutexes[%d]" % sys_mutex_counter sys_mutex_counter += 1 elif self.type_obj.name == "k_futex": - self.data = "(uintptr_t)(&futex_data[%d])" % futex_counter + self.data = "&futex_data[%d]" % futex_counter futex_counter += 1 else: self.data = 0 diff --git a/scripts/gen_kobject_list.py b/scripts/gen_kobject_list.py index f0f9811588799..04e402b9a2d47 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -162,6 +162,12 @@ #endif """ +metadata_names = { + "K_OBJ_THREAD" : "thread_id", + "K_OBJ__THREAD_STACK_ELEMENT" : "stack_size", + "K_OBJ_SYS_MUTEX" : "mutex", + "K_OBJ_FUTEX" : "futex_data" +} def write_gperf_table(fp, eh, objs, static_begin, static_end): fp.write(header) @@ -222,7 +228,13 @@ def write_gperf_table(fp, eh, objs, static_begin, static_end): if is_driver: flags += " | K_OBJ_FLAG_DRIVER" - fp.write("\", {}, %s, %s, %s\n" % (obj_type, flags, str(ko.data))) + if ko.type_name in metadata_names: + tname = metadata_names[ko.type_name] + else: + tname = "unused" + + fp.write("\", {}, %s, %s, { .%s = %s }\n" % (obj_type, flags, + tname, str(ko.data))) if obj_type == "K_OBJ_THREAD": idx = math.floor(ko.data / 8) From c8fa294df9a4c61c846a15593ec0b290e286cb90 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Wed, 11 Mar 2020 07:13:07 -0700 Subject: [PATCH 04/19] kernel: rename struct _k_object Private type, internal to the kernel, not directly associated with any k_object_* APIs. Is the return value of z_object_find(). Rename to struct z_object. Signed-off-by: Andrew Boie --- CMakeLists.txt | 2 +- doc/guides/documentation/index.rst | 4 +- doc/reference/usermode/kernelobjects.rst | 2 +- include/kernel.h | 2 +- include/syscall_handler.h | 28 ++++++------- kernel/futex.c | 2 +- kernel/thread.c | 4 +- kernel/userspace.c | 42 +++++++++---------- kernel/userspace_handler.c | 8 ++-- lib/os/mutex.c | 2 +- scripts/gen_kobject_list.py | 6 +-- tests/kernel/mem_protect/userspace/src/main.c | 2 +- 12 files changed, 52 insertions(+), 52 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 96b499909ce3d..670b784a2aed6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -986,7 +986,7 @@ if(CONFIG_USERSPACE) ${PROCESS_GPERF} -i ${OUTPUT_SRC_PRE} -o ${OUTPUT_SRC} - -p "struct _k_object" + -p "struct z_object" $<$:--verbose> DEPENDS output_src_pre ${OUTPUT_SRC_PRE} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} diff --git a/doc/guides/documentation/index.rst b/doc/guides/documentation/index.rst index eb17b5773ce2f..df2f4b13361cc 100644 --- a/doc/guides/documentation/index.rst +++ b/doc/guides/documentation/index.rst @@ -396,7 +396,7 @@ For example:: .. code-block:: c - struct _k_object { + struct z_object { char *name; u8_t perms[CONFIG_MAX_THREAD_BYTES]; u8_t type; @@ -412,7 +412,7 @@ This would be rendered as: .. code-block:: c - struct _k_object { + struct z_object { char *name; u8_t perms[CONFIG_MAX_THREAD_BYTES]; u8_t type; diff --git a/doc/reference/usermode/kernelobjects.rst b/doc/reference/usermode/kernelobjects.rst index c12145042d90b..b07a69cb64e20 100644 --- a/doc/reference/usermode/kernelobjects.rst +++ b/doc/reference/usermode/kernelobjects.rst @@ -116,7 +116,7 @@ be prevented. When a device struct is found, its API pointer is examined to determine what subsystem the driver belongs to. The table itself maps kernel object memory addresses to instances of -:c:type:`struct _k_object`, which has all the metadata for that object. This +:c:type:`struct z_object`, which has all the metadata for that object. This includes: * A bitfield indicating permissions on that object. All threads have a diff --git a/include/kernel.h b/include/kernel.h index f6abf0e6efd3b..6af9c51bc3eed 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -186,7 +186,7 @@ union z_object_data { /* Table generated by gperf, these objects are retrieved via * z_object_find() */ -struct _k_object { +struct z_object { void *name; u8_t perms[CONFIG_MAX_THREAD_BYTES]; u8_t type; diff --git a/include/syscall_handler.h b/include/syscall_handler.h index 4a0ec651a1e6c..60b14f8894179 100644 --- a/include/syscall_handler.h +++ b/include/syscall_handler.h @@ -46,19 +46,19 @@ enum _obj_init_check { * -EPERM If the caller does not have permissions * -EINVAL Object is not initialized */ -int z_object_validate(struct _k_object *ko, enum k_objects otype, - enum _obj_init_check init); +int z_object_validate(struct z_object *ko, enum k_objects otype, + enum _obj_init_check init); /** * Dump out error information on failed z_object_validate() call * * @param retval Return value from z_object_validate() * @param obj Kernel object we were trying to verify - * @param ko If retval=-EPERM, struct _k_object * that was looked up, or NULL + * @param ko If retval=-EPERM, struct z_object * that was looked up, or NULL * @param otype Expected type of the kernel object */ -extern void z_dump_object_error(int retval, void *obj, struct _k_object *ko, - enum k_objects otype); +extern void z_dump_object_error(int retval, void *obj, struct z_object *ko, + enum k_objects otype); /** * Kernel object validation function @@ -70,14 +70,14 @@ extern void z_dump_object_error(int retval, void *obj, struct _k_object *ko, * @return Kernel object's metadata, or NULL if the parameter wasn't the * memory address of a kernel object */ -extern struct _k_object *z_object_find(void *obj); +extern struct z_object *z_object_find(void *obj); -typedef void (*_wordlist_cb_func_t)(struct _k_object *ko, void *context); +typedef void (*_wordlist_cb_func_t)(struct z_object *ko, void *context); /** * Iterate over all the kernel object metadata in the system * - * @param func function to run on each struct _k_object + * @param func function to run on each struct z_object * @param context Context pointer to pass to each invocation */ extern void z_object_wordlist_foreach(_wordlist_cb_func_t func, void *context); @@ -97,7 +97,7 @@ extern void z_thread_perms_inherit(struct k_thread *parent, * @param ko Kernel object metadata to update * @param thread The thread to grant permission */ -extern void z_thread_perms_set(struct _k_object *ko, struct k_thread *thread); +extern void z_thread_perms_set(struct z_object *ko, struct k_thread *thread); /** * Revoke a thread's permission to a kernel object @@ -105,7 +105,7 @@ extern void z_thread_perms_set(struct _k_object *ko, struct k_thread *thread); * @param ko Kernel object metadata to update * @param thread The thread to grant permission */ -extern void z_thread_perms_clear(struct _k_object *ko, struct k_thread *thread); +extern void z_thread_perms_clear(struct z_object *ko, struct k_thread *thread); /* * Revoke access to all objects for the provided thread @@ -393,10 +393,10 @@ extern int z_user_string_copy(char *dst, const char *src, size_t maxlen); #define Z_SYSCALL_MEMORY_ARRAY_WRITE(ptr, nmemb, size) \ Z_SYSCALL_MEMORY_ARRAY(ptr, nmemb, size, 1) -static inline int z_obj_validation_check(struct _k_object *ko, - void *obj, - enum k_objects otype, - enum _obj_init_check init) +static inline int z_obj_validation_check(struct z_object *ko, + void *obj, + enum k_objects otype, + enum _obj_init_check init) { int ret; diff --git a/kernel/futex.c b/kernel/futex.c index 8507cd2d01450..3ec3b05d2f6eb 100644 --- a/kernel/futex.c +++ b/kernel/futex.c @@ -14,7 +14,7 @@ static struct z_futex_data *k_futex_find_data(struct k_futex *futex) { - struct _k_object *obj; + struct z_object *obj; obj = z_object_find(futex); if (obj == NULL || obj->type != K_OBJ_FUTEX) { diff --git a/kernel/thread.c b/kernel/thread.c index 8feee474b47b3..2f8237b85aeec 100644 --- a/kernel/thread.c +++ b/kernel/thread.c @@ -305,7 +305,7 @@ static inline int z_vrfy_k_thread_name_copy(k_tid_t thread, { #ifdef CONFIG_THREAD_NAME size_t len; - struct _k_object *ko = z_object_find(thread); + struct z_object *ko = z_object_find(thread); /* Special case: we allow reading the names of initialized threads * even if we don't have permission on them @@ -620,7 +620,7 @@ k_tid_t z_vrfy_k_thread_create(struct k_thread *new_thread, int prio, u32_t options, s32_t delay) { size_t total_size; - struct _k_object *stack_object; + struct z_object *stack_object; /* The thread and stack objects *must* be in an uninitialized state */ Z_OOPS(Z_SYSCALL_OBJ_NEVER_INIT(new_thread, K_OBJ_THREAD)); diff --git a/kernel/userspace.c b/kernel/userspace.c index 26b5f5bed0849..cd5236bc3eb0e 100644 --- a/kernel/userspace.c +++ b/kernel/userspace.c @@ -58,7 +58,7 @@ static struct k_spinlock obj_lock; /* kobj struct data */ extern u8_t _thread_idx_map[CONFIG_MAX_THREAD_BYTES]; #endif -static void clear_perms_cb(struct _k_object *ko, void *ctx_ptr); +static void clear_perms_cb(struct z_object *ko, void *ctx_ptr); const char *otype_to_str(enum k_objects otype) { @@ -92,13 +92,13 @@ struct perm_ctx { #ifdef CONFIG_DYNAMIC_OBJECTS struct dyn_obj { - struct _k_object kobj; + struct z_object kobj; sys_dnode_t obj_list; struct rbnode node; /* must be immediately before data member */ u8_t data[]; /* The object itself */ }; -extern struct _k_object *z_object_gperf_find(void *obj); +extern struct z_object *z_object_gperf_find(void *obj); extern void z_object_gperf_wordlist_foreach(_wordlist_cb_func_t func, void *context); @@ -307,9 +307,9 @@ void k_object_free(void *obj) } } -struct _k_object *z_object_find(void *obj) +struct z_object *z_object_find(void *obj) { - struct _k_object *ret; + struct z_object *ret; ret = z_object_gperf_find(obj); @@ -342,7 +342,7 @@ void z_object_wordlist_foreach(_wordlist_cb_func_t func, void *context) static unsigned int thread_index_get(struct k_thread *thread) { - struct _k_object *ko; + struct z_object *ko; ko = z_object_find(thread); @@ -353,7 +353,7 @@ static unsigned int thread_index_get(struct k_thread *thread) return ko->data.thread_id; } -static void unref_check(struct _k_object *ko, uintptr_t index) +static void unref_check(struct z_object *ko, uintptr_t index) { k_spinlock_key_t key = k_spin_lock(&obj_lock); @@ -401,7 +401,7 @@ static void unref_check(struct _k_object *ko, uintptr_t index) k_spin_unlock(&obj_lock, key); } -static void wordlist_cb(struct _k_object *ko, void *ctx_ptr) +static void wordlist_cb(struct z_object *ko, void *ctx_ptr) { struct perm_ctx *ctx = (struct perm_ctx *)ctx_ptr; @@ -424,7 +424,7 @@ void z_thread_perms_inherit(struct k_thread *parent, struct k_thread *child) } } -void z_thread_perms_set(struct _k_object *ko, struct k_thread *thread) +void z_thread_perms_set(struct z_object *ko, struct k_thread *thread) { int index = thread_index_get(thread); @@ -433,7 +433,7 @@ void z_thread_perms_set(struct _k_object *ko, struct k_thread *thread) } } -void z_thread_perms_clear(struct _k_object *ko, struct k_thread *thread) +void z_thread_perms_clear(struct z_object *ko, struct k_thread *thread) { int index = thread_index_get(thread); @@ -443,7 +443,7 @@ void z_thread_perms_clear(struct _k_object *ko, struct k_thread *thread) } } -static void clear_perms_cb(struct _k_object *ko, void *ctx_ptr) +static void clear_perms_cb(struct z_object *ko, void *ctx_ptr) { uintptr_t id = (uintptr_t)ctx_ptr; @@ -459,7 +459,7 @@ void z_thread_perms_all_clear(struct k_thread *thread) } } -static int thread_perms_test(struct _k_object *ko) +static int thread_perms_test(struct z_object *ko) { int index; @@ -474,7 +474,7 @@ static int thread_perms_test(struct _k_object *ko) return 0; } -static void dump_permission_error(struct _k_object *ko) +static void dump_permission_error(struct z_object *ko) { int index = thread_index_get(_current); LOG_ERR("thread %p (%d) does not have permission on %s %p", @@ -483,7 +483,7 @@ static void dump_permission_error(struct _k_object *ko) LOG_HEXDUMP_ERR(ko->perms, sizeof(ko->perms), "permission bitmap"); } -void z_dump_object_error(int retval, void *obj, struct _k_object *ko, +void z_dump_object_error(int retval, void *obj, struct z_object *ko, enum k_objects otype) { switch (retval) { @@ -507,7 +507,7 @@ void z_dump_object_error(int retval, void *obj, struct _k_object *ko, void z_impl_k_object_access_grant(void *object, struct k_thread *thread) { - struct _k_object *ko = z_object_find(object); + struct z_object *ko = z_object_find(object); if (ko != NULL) { z_thread_perms_set(ko, thread); @@ -516,7 +516,7 @@ void z_impl_k_object_access_grant(void *object, struct k_thread *thread) void k_object_access_revoke(void *object, struct k_thread *thread) { - struct _k_object *ko = z_object_find(object); + struct z_object *ko = z_object_find(object); if (ko != NULL) { z_thread_perms_clear(ko, thread); @@ -530,14 +530,14 @@ void z_impl_k_object_release(void *object) void k_object_access_all_grant(void *object) { - struct _k_object *ko = z_object_find(object); + struct z_object *ko = z_object_find(object); if (ko != NULL) { ko->flags |= K_OBJ_FLAG_PUBLIC; } } -int z_object_validate(struct _k_object *ko, enum k_objects otype, +int z_object_validate(struct z_object *ko, enum k_objects otype, enum _obj_init_check init) { if (unlikely((ko == NULL) || @@ -572,7 +572,7 @@ int z_object_validate(struct _k_object *ko, enum k_objects otype, void z_object_init(void *obj) { - struct _k_object *ko; + struct z_object *ko; /* By the time we get here, if the caller was from userspace, all the * necessary checks have been done in z_object_validate(), which takes @@ -597,7 +597,7 @@ void z_object_init(void *obj) void z_object_recycle(void *obj) { - struct _k_object *ko = z_object_find(obj); + struct z_object *ko = z_object_find(obj); if (ko != NULL) { (void)memset(ko->perms, 0, sizeof(ko->perms)); @@ -608,7 +608,7 @@ void z_object_recycle(void *obj) void z_object_uninit(void *obj) { - struct _k_object *ko; + struct z_object *ko; /* See comments in z_object_init() */ ko = z_object_find(obj); diff --git a/kernel/userspace_handler.c b/kernel/userspace_handler.c index 054fa1724d838..96d375ced21b9 100644 --- a/kernel/userspace_handler.c +++ b/kernel/userspace_handler.c @@ -8,9 +8,9 @@ #include #include -static struct _k_object *validate_any_object(void *obj) +static struct z_object *validate_any_object(void *obj) { - struct _k_object *ko; + struct z_object *ko; int ret; ko = z_object_find(obj); @@ -39,7 +39,7 @@ static struct _k_object *validate_any_object(void *obj) static inline void z_vrfy_k_object_access_grant(void *object, struct k_thread *thread) { - struct _k_object *ko; + struct z_object *ko; Z_OOPS(Z_SYSCALL_OBJ_INIT(thread, K_OBJ_THREAD)); ko = validate_any_object(object); @@ -51,7 +51,7 @@ static inline void z_vrfy_k_object_access_grant(void *object, static inline void z_vrfy_k_object_release(void *object) { - struct _k_object *ko; + struct z_object *ko; ko = validate_any_object((void *)object); Z_OOPS(Z_SYSCALL_VERIFY_MSG(ko != NULL, "object %p access denied", diff --git a/lib/os/mutex.c b/lib/os/mutex.c index 0e952e8e940c3..43fc254a7acf9 100644 --- a/lib/os/mutex.c +++ b/lib/os/mutex.c @@ -11,7 +11,7 @@ static struct k_mutex *get_k_mutex(struct sys_mutex *mutex) { - struct _k_object *obj; + struct z_object *obj; obj = z_object_find(mutex); if (obj == NULL || obj->type != K_OBJ_SYS_MUTEX) { diff --git a/scripts/gen_kobject_list.py b/scripts/gen_kobject_list.py index 04e402b9a2d47..d698e389e7652 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -129,7 +129,7 @@ #include #include %} -struct _k_object; +struct z_object; """ # Different versions of gperf have different prototypes for the lookup @@ -137,7 +137,7 @@ # turned into a string, we told gperf to expect binary strings that are not # NULL-terminated. footer = """%% -struct _k_object *z_object_gperf_find(void *obj) +struct z_object *z_object_gperf_find(void *obj) { return z_object_lookup((const char *)obj, sizeof(void *)); } @@ -154,7 +154,7 @@ } #ifndef CONFIG_DYNAMIC_OBJECTS -struct _k_object *z_object_find(void *obj) +struct z_object *z_object_find(void *obj) ALIAS_OF(z_object_gperf_find); void z_object_wordlist_foreach(_wordlist_cb_func_t func, void *context) diff --git a/tests/kernel/mem_protect/userspace/src/main.c b/tests/kernel/mem_protect/userspace/src/main.c index d4b84ac7d193a..863602a5d5440 100644 --- a/tests/kernel/mem_protect/userspace/src/main.c +++ b/tests/kernel/mem_protect/userspace/src/main.c @@ -1123,7 +1123,7 @@ static struct k_sem recycle_sem; void test_object_recycle(void) { - struct _k_object *ko; + struct z_object *ko; int perms_count = 0; ko = z_object_find(&recycle_sem); From 7f05e8071677a8ef3cfad1c5c42eb560ee941a9d Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 18:26:20 -0700 Subject: [PATCH 05/19] kernel: fix k_object_free() spelling This was supposed to match definitions if dynamic objects are turned on. Signed-off-by: Andrew Boie --- include/kernel.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/kernel.h b/include/kernel.h index 6af9c51bc3eed..f8d3f13dbdfb7 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -379,7 +379,7 @@ static inline void *z_impl_k_object_alloc(enum k_objects otype) * * @param obj */ -static inline void k_obj_free(void *obj) +static inline void k_object_free(void *obj) { ARG_UNUSED(obj); } From 9193d9ff7984204569c45b75b46d7217d77839c6 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Thu, 28 May 2020 16:24:09 -0700 Subject: [PATCH 06/19] userspace: fix bad ssf pointer on bad/no syscall This was passing along _current->ssf, but these types of bad syscalls do not go through the z_mrsh mechanism and was passing stale data. We have the syscall stack frame already as an argument, propagate that so it works properly. Signed-off-by: Andrew Boie --- kernel/userspace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/userspace.c b/kernel/userspace.c index cd5236bc3eb0e..0b687cddb3d90 100644 --- a/kernel/userspace.c +++ b/kernel/userspace.c @@ -760,7 +760,7 @@ static uintptr_t handler_bad_syscall(uintptr_t bad_id, uintptr_t arg2, void *ssf) { LOG_ERR("Bad system call id %" PRIuPTR " invoked", bad_id); - arch_syscall_oops(_current->syscall_frame); + arch_syscall_oops(ssf); CODE_UNREACHABLE; /* LCOV_EXCL_LINE */ } @@ -769,7 +769,7 @@ static uintptr_t handler_no_syscall(uintptr_t arg1, uintptr_t arg2, uintptr_t arg5, uintptr_t arg6, void *ssf) { LOG_ERR("Unimplemented system call"); - arch_syscall_oops(_current->syscall_frame); + arch_syscall_oops(ssf); CODE_UNREACHABLE; /* LCOV_EXCL_LINE */ } From 7b43b0646072037bfd2485d37d6acc53e8c720c1 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Thu, 28 May 2020 11:48:54 -0700 Subject: [PATCH 07/19] userspace: add z_is_in_user_syscall() Certain types of system call validation may need to be pushed deeper in the implementation and not performed in the verification function. If such checks are only pertinent when the caller was from user mode, we need an API to detect this situation. This is implemented by having thread->syscall_frame be non-NULL only while a user system call is in progress. The template for the system call marshalling functions is changed to clear this value on exit. A test is added to prove that this works. Signed-off-by: Andrew Boie --- include/syscall_handler.h | 32 +++++++++++++++ kernel/thread.c | 2 + scripts/gen_syscalls.py | 4 ++ tests/kernel/mem_protect/syscalls/src/main.c | 41 ++++++++++++++++++- .../mem_protect/syscalls/src/test_syscalls.h | 2 + 5 files changed, 80 insertions(+), 1 deletion(-) diff --git a/include/syscall_handler.h b/include/syscall_handler.h index 60b14f8894179..722888b430e07 100644 --- a/include/syscall_handler.h +++ b/include/syscall_handler.h @@ -25,6 +25,38 @@ enum _obj_init_check { _OBJ_INIT_ANY = 1 }; +/** + * Return true if we are currently handling a system call from user mode + * + * Inside z_vrfy functions, we always know that we are handling + * a system call invoked from user context. + * + * However, some checks that are only relevant to user mode must + * instead be placed deeper within the implementation. This + * API is useful to conditionally make these checks. + * + * For performance reasons, whenever possible, checks should be placed + * in the relevant z_vrfy function since these are completely skipped + * when a syscall is invoked. + * + * This will return true only if we are handling a syscall for a + * user thread. If the system call was invoked from supervisor mode, + * or we are not handling a system call, this will return false. + * + * @return whether the current context is handling a syscall for a user + * mode thread + */ +static inline bool z_is_in_user_syscall(void) +{ + /* This gets set on entry to the syscall's generasted z_mrsh + * function and then cleared on exit. This code path is only + * encountered when a syscall is made from user mode, system + * calls from supervisor mode bypass everything directly to + * the implementation function. + */ + return !k_is_in_isr() && _current->syscall_frame != NULL; +} + /** * Ensure a system object is a valid object of the expected type * diff --git a/kernel/thread.c b/kernel/thread.c index 2f8237b85aeec..807922cc81092 100644 --- a/kernel/thread.c +++ b/kernel/thread.c @@ -500,6 +500,8 @@ void z_setup_new_thread(struct k_thread *new_thread, z_object_init(new_thread); z_object_init(stack); new_thread->stack_obj = stack; + new_thread->mem_domain_info.mem_domain = NULL; + new_thread->syscall_frame = NULL; /* Any given thread has access to itself */ k_object_access_grant(new_thread, new_thread); diff --git a/scripts/gen_syscalls.py b/scripts/gen_syscalls.py index ae7e250a8e587..30c12fe421915 100755 --- a/scripts/gen_syscalls.py +++ b/scripts/gen_syscalls.py @@ -284,15 +284,19 @@ def marshall_defs(func_name, func_type, args): if func_type == "void": mrsh += "\t" + "%s;\n" % vrfy_call + mrsh += "\t" + "_current->syscall_frame = NULL;\n" mrsh += "\t" + "return 0;\n" else: mrsh += "\t" + "%s ret = %s;\n" % (func_type, vrfy_call) + if need_split(func_type): ptr = "((u64_t *)%s)" % mrsh_rval(nmrsh - 1, nmrsh) mrsh += "\t" + "Z_OOPS(Z_SYSCALL_MEMORY_WRITE(%s, 8));\n" % ptr mrsh += "\t" + "*%s = ret;\n" % ptr + mrsh += "\t" + "_current->syscall_frame = NULL;\n" mrsh += "\t" + "return 0;\n" else: + mrsh += "\t" + "_current->syscall_frame = NULL;\n" mrsh += "\t" + "return (uintptr_t) ret;\n" mrsh += "}\n" diff --git a/tests/kernel/mem_protect/syscalls/src/main.c b/tests/kernel/mem_protect/syscalls/src/main.c index a73a6ac79cf63..5e8e36864bb1c 100644 --- a/tests/kernel/mem_protect/syscalls/src/main.c +++ b/tests/kernel/mem_protect/syscalls/src/main.c @@ -331,6 +331,44 @@ void test_syscall_torture(void) printk("\n"); } +bool z_impl_syscall_context(void) +{ + return z_is_in_user_syscall(); +} + +static inline bool z_vrfy_syscall_context(void) +{ + return z_impl_syscall_context(); +} +#include + +void test_syscall_context_user(void *p1, void *p2, void *p3) +{ + ARG_UNUSED(p1); + ARG_UNUSED(p2); + ARG_UNUSED(p3); + + zassert_true(syscall_context(), + "not reported in user syscall"); +} + +/* Show that z_is_in_syscall() works properly */ +void test_syscall_context(void) +{ + /* We're a regular supervisor thread. */ + zassert_false(z_is_in_user_syscall(), + "reported in user syscall when in supv. thread ctx"); + + /* Make a system call from supervisor mode. The check in the + * implementation function should return false. + */ + zassert_false(syscall_context(), + "reported in user syscall when called from supervisor"); + + /* Remainder of the test in user mode */ + k_thread_user_mode_enter(test_syscall_context_user, NULL, NULL, NULL); +} + K_MEM_POOL_DEFINE(test_pool, BUF_SIZE, BUF_SIZE, 4 * NR_THREADS, 4); void test_main(void) @@ -346,7 +384,8 @@ void test_main(void) ztest_user_unit_test(test_user_string_copy), ztest_user_unit_test(test_user_string_alloc_copy), ztest_user_unit_test(test_arg64), - ztest_unit_test(test_syscall_torture) + ztest_unit_test(test_syscall_torture), + ztest_unit_test(test_syscall_context) ); ztest_run_test_suite(syscalls); } diff --git a/tests/kernel/mem_protect/syscalls/src/test_syscalls.h b/tests/kernel/mem_protect/syscalls/src/test_syscalls.h index 51eaced52138f..7f107054a6ac5 100644 --- a/tests/kernel/mem_protect/syscalls/src/test_syscalls.h +++ b/tests/kernel/mem_protect/syscalls/src/test_syscalls.h @@ -21,6 +21,8 @@ __syscall int syscall_arg64(u64_t arg); __syscall u64_t syscall_arg64_big(u32_t arg1, u32_t arg2, u64_t arg3, u32_t arg4, u32_t arg5, u64_t arg6); +__syscall bool syscall_context(void); + #include #endif /* _TEST_SYSCALLS_H_ */ From 462358de75ea9e638756cb69e74ccad51408decf Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 13:22:20 -0700 Subject: [PATCH 08/19] scripts: gen_kobject_list: generalize obj alloc Instead of handling this ad hoc, generalize which kobject types can be allocated. Signed-off-by: Andrew Boie --- scripts/gen_kobject_list.py | 586 ++++++++++++++++++++++++++++++++++-- 1 file changed, 566 insertions(+), 20 deletions(-) diff --git a/scripts/gen_kobject_list.py b/scripts/gen_kobject_list.py index d698e389e7652..10b2526a6cff7 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -70,25 +70,28 @@ # # - The second item is a boolean indicating whether it is permissible for # the object to be located in user-accessible memory. +# +# - The third items is a boolean indicating whether this item can be +# dynamically allocated with k_object_alloc() # Regular dictionaries are ordered only with Python 3.6 and # above. Good summary and pointers to official documents at: # https://stackoverflow.com/questions/39980323/are-dictionaries-ordered-in-python-3-6 kobjects = OrderedDict([ - ("k_mem_slab", (None, False)), - ("k_msgq", (None, False)), - ("k_mutex", (None, False)), - ("k_pipe", (None, False)), - ("k_queue", (None, False)), - ("k_poll_signal", (None, False)), - ("k_sem", (None, False)), - ("k_stack", (None, False)), - ("k_thread", (None, False)), - ("k_timer", (None, False)), - ("_k_thread_stack_element", (None, False)), - ("device", (None, False)), - ("sys_mutex", (None, True)), - ("k_futex", (None, True)) + ("k_mem_slab", (None, False, True)), + ("k_msgq", (None, False, True)), + ("k_mutex", (None, False, True)), + ("k_pipe", (None, False, True)), + ("k_queue", (None, False, True)), + ("k_poll_signal", (None, False, True)), + ("k_sem", (None, False, True)), + ("k_stack", (None, False, True)), + ("k_thread", (None, False, True)), # But see # + ("k_timer", (None, False, True)), + ("_k_thread_stack_element", (None, False, False)), + ("device", (None, False, False)), + ("sys_mutex", (None, True, False)), + ("k_futex", (None, True, False)) ]) subsystems = [ @@ -118,6 +121,550 @@ "sample_driver_api" ] +def subsystem_to_enum(subsys): + return "K_OBJ_DRIVER_" + subsys[:-11].upper() + +# --- debug stuff --- + +scr = os.path.basename(sys.argv[0]) + +def debug(text): + if not args.verbose: + return + sys.stdout.write(scr + ": " + text + "\n") + +def error(text): + sys.exit("%s ERROR: %s" % (scr, text)) + +def debug_die(die, text): + lp_header = die.dwarfinfo.line_program_for_CU(die.cu).header + files = lp_header["file_entry"] + includes = lp_header["include_directory"] + + fileinfo = files[die.attributes["DW_AT_decl_file"].value - 1] + filename = fileinfo.name.decode("utf-8") + filedir = includes[fileinfo.dir_index - 1].decode("utf-8") + + path = os.path.join(filedir, filename) + lineno = die.attributes["DW_AT_decl_line"].value + + debug(str(die)) + debug("File '%s', line %d:" % (path, lineno)) + debug(" %s" % text) + +# -- ELF processing + +DW_OP_addr = 0x3 +DW_OP_fbreg = 0x91 +STACK_TYPE = "z_thread_stack_element" +thread_counter = 0 +sys_mutex_counter = 0 +futex_counter = 0 +stack_counter = 0 + +# Global type environment. Populated by pass 1. +type_env = {} +extern_env = {} + +class KobjectInstance: + def __init__(self, type_obj, addr): + global thread_counter + global sys_mutex_counter + global futex_counter + global stack_counter + + self.addr = addr + self.type_obj = type_obj + + # Type name determined later since drivers needs to look at the + # API struct address + self.type_name = None + + if self.type_obj.name == "k_thread": + # Assign an ID for this thread object, used to track its + # permissions to other kernel objects + self.data = thread_counter + thread_counter = thread_counter + 1 + elif self.type_obj.name == "sys_mutex": + self.data = "&kernel_mutexes[%d]" % sys_mutex_counter + sys_mutex_counter += 1 + elif self.type_obj.name == "k_futex": + self.data = "&futex_data[%d]" % futex_counter + futex_counter += 1 + elif self.type_obj.name == STACK_TYPE: + stack_counter += 1 + else: + self.data = 0 + + +class KobjectType: + def __init__(self, offset, name, size, api=False): + self.name = name + self.size = size + self.offset = offset + self.api = api + + def __repr__(self): + return "" % self.name + + @staticmethod + def has_kobject(): + return True + + def get_kobjects(self, addr): + return {addr: KobjectInstance(self, addr)} + + +class ArrayType: + def __init__(self, offset, elements, member_type): + self.elements = elements + self.member_type = member_type + self.offset = offset + + def __repr__(self): + return "" % self.member_type + + def has_kobject(self): + if self.member_type not in type_env: + return False + + return type_env[self.member_type].has_kobject() + + def get_kobjects(self, addr): + mt = type_env[self.member_type] + + # Stacks are arrays of _k_stack_element_t but we want to treat + # the whole array as one kernel object (a thread stack) + # Data value gets set to size of entire region + if isinstance(mt, KobjectType) and mt.name == STACK_TYPE: + # An array of stacks appears as a multi-dimensional array. + # The last size is the size of each stack. We need to track + # each stack within the array, not as one huge stack object. + *dimensions, stacksize = self.elements + num_members = 1 + for e in dimensions: + num_members = num_members * e + + ret = {} + for i in range(num_members): + a = addr + (i * stacksize) + o = mt.get_kobjects(a) + o[a].data = stacksize + ret.update(o) + return ret + + objs = {} + + # Multidimensional array flattened out + num_members = 1 + for e in self.elements: + num_members = num_members * e + + for i in range(num_members): + objs.update(mt.get_kobjects(addr + (i * mt.size))) + return objs + + +class AggregateTypeMember: + def __init__(self, offset, member_name, member_type, member_offset): + self.member_name = member_name + self.member_type = member_type + if isinstance(member_offset, list): + # DWARF v2, location encoded as set of operations + # only "DW_OP_plus_uconst" with ULEB128 argument supported + if member_offset[0] == 0x23: + self.member_offset = member_offset[1] & 0x7f + for i in range(1, len(member_offset)-1): + if member_offset[i] & 0x80: + self.member_offset += ( + member_offset[i+1] & 0x7f) << i*7 + else: + raise Exception("not yet supported location operation (%s:%d:%d)" % + (self.member_name, self.member_type, member_offset[0])) + else: + self.member_offset = member_offset + + def __repr__(self): + return "" % ( + self.member_name, self.member_type, self.member_offset) + + def has_kobject(self): + if self.member_type not in type_env: + return False + + return type_env[self.member_type].has_kobject() + + def get_kobjects(self, addr): + mt = type_env[self.member_type] + return mt.get_kobjects(addr + self.member_offset) + + +class ConstType: + def __init__(self, child_type): + self.child_type = child_type + + def __repr__(self): + return "" % self.child_type + + def has_kobject(self): + if self.child_type not in type_env: + return False + + return type_env[self.child_type].has_kobject() + + def get_kobjects(self, addr): + return type_env[self.child_type].get_kobjects(addr) + + +class AggregateType: + def __init__(self, offset, name, size): + self.name = name + self.size = size + self.offset = offset + self.members = [] + + def add_member(self, member): + self.members.append(member) + + def __repr__(self): + return "" % (self.name, self.members) + + def has_kobject(self): + result = False + + bad_members = [] + + for member in self.members: + if member.has_kobject(): + result = True + else: + bad_members.append(member) + # Don't need to consider this again, just remove it + + for bad_member in bad_members: + self.members.remove(bad_member) + + return result + + def get_kobjects(self, addr): + objs = {} + for member in self.members: + objs.update(member.get_kobjects(addr)) + return objs + + +# --- helper functions for getting data from DIEs --- + +def die_get_spec(die): + if 'DW_AT_specification' not in die.attributes: + return None + + spec_val = die.attributes["DW_AT_specification"].value + + # offset of the DW_TAG_variable for the extern declaration + offset = spec_val + die.cu.cu_offset + + return extern_env.get(offset) + + +def die_get_name(die): + if 'DW_AT_name' not in die.attributes: + die = die_get_spec(die) + if not die: + return None + + return die.attributes["DW_AT_name"].value.decode("utf-8") + + +def die_get_type_offset(die): + if 'DW_AT_type' not in die.attributes: + die = die_get_spec(die) + if not die: + return None + + return die.attributes["DW_AT_type"].value + die.cu.cu_offset + + +def die_get_byte_size(die): + if 'DW_AT_byte_size' not in die.attributes: + return 0 + + return die.attributes["DW_AT_byte_size"].value + + +def analyze_die_struct(die): + name = die_get_name(die) or "" + offset = die.offset + size = die_get_byte_size(die) + + # Incomplete type + if not size: + return + + if name in kobjects: + type_env[offset] = KobjectType(offset, name, size) + elif name in subsystems: + type_env[offset] = KobjectType(offset, name, size, api=True) + else: + at = AggregateType(offset, name, size) + type_env[offset] = at + + for child in die.iter_children(): + if child.tag != "DW_TAG_member": + continue + data_member_location = child.attributes.get("DW_AT_data_member_location") + if not data_member_location: + continue + + child_type = die_get_type_offset(child) + member_offset = data_member_location.value + cname = die_get_name(child) or "" + m = AggregateTypeMember(child.offset, cname, child_type, + member_offset) + at.add_member(m) + + return + + +def analyze_die_const(die): + type_offset = die_get_type_offset(die) + if not type_offset: + return + + type_env[die.offset] = ConstType(type_offset) + + +def analyze_die_array(die): + type_offset = die_get_type_offset(die) + elements = [] + + for child in die.iter_children(): + if child.tag != "DW_TAG_subrange_type": + continue + if "DW_AT_upper_bound" not in child.attributes: + continue + + ub = child.attributes["DW_AT_upper_bound"] + if not ub.form.startswith("DW_FORM_data"): + continue + + elements.append(ub.value + 1) + + if not elements: + if type_offset in type_env.keys(): + mt = type_env[type_offset] + if mt.has_kobject(): + if isinstance(mt, KobjectType) and mt.name == STACK_TYPE: + elements.append(1) + type_env[die.offset] = ArrayType(die.offset, elements, type_offset) + else: + type_env[die.offset] = ArrayType(die.offset, elements, type_offset) + + +def analyze_typedef(die): + type_offset = die_get_type_offset(die) + + if type_offset not in type_env.keys(): + return + + type_env[die.offset] = type_env[type_offset] + + +def unpack_pointer(elf, data, offset): + endian_code = "<" if elf.little_endian else ">" + if elf.elfclass == 32: + size_code = "I" + size = 4 + else: + size_code = "Q" + size = 8 + + return struct.unpack(endian_code + size_code, + data[offset:offset + size])[0] + + +def addr_deref(elf, addr): + for section in elf.iter_sections(): + start = section['sh_addr'] + end = start + section['sh_size'] + + if start <= addr < end: + data = section.data() + offset = addr - start + return unpack_pointer(elf, data, offset) + + return 0 + + +def device_get_api_addr(elf, addr): + # See include/device.h for a description of struct device + offset = 8 if elf.elfclass == 32 else 16 + return addr_deref(elf, addr + offset) + + +def find_kobjects(elf, syms): + if not elf.has_dwarf_info(): + sys.exit("ELF file has no DWARF information") + + app_smem_start = syms["_app_smem_start"] + app_smem_end = syms["_app_smem_end"] + + di = elf.get_dwarf_info() + + variables = [] + + # Step 1: collect all type information. + for CU in di.iter_CUs(): + for die in CU.iter_DIEs(): + # Unions are disregarded, kernel objects should never be union + # members since the memory is not dedicated to that object and + # could be something else + if die.tag == "DW_TAG_structure_type": + analyze_die_struct(die) + elif die.tag == "DW_TAG_const_type": + analyze_die_const(die) + elif die.tag == "DW_TAG_array_type": + analyze_die_array(die) + elif die.tag == "DW_TAG_typedef": + analyze_typedef(die) + elif die.tag == "DW_TAG_variable": + variables.append(die) + + # Step 2: filter type_env to only contain kernel objects, or structs + # and arrays of kernel objects + bad_offsets = [] + for offset, type_object in type_env.items(): + if not type_object.has_kobject(): + bad_offsets.append(offset) + + for offset in bad_offsets: + del type_env[offset] + + # Step 3: Now that we know all the types we are looking for, examine + # all variables + all_objs = {} + + for die in variables: + name = die_get_name(die) + if not name: + continue + + if name.startswith("__init_sys_init"): + # Boot-time initialization function; not an actual device + continue + + type_offset = die_get_type_offset(die) + + # Is this a kernel object, or a structure containing kernel + # objects? + if type_offset not in type_env: + continue + + if "DW_AT_declaration" in die.attributes: + # Extern declaration, only used indirectly + extern_env[die.offset] = die + continue + + if "DW_AT_location" not in die.attributes: + debug_die(die, + "No location information for object '%s'; possibly stack allocated" + % name) + continue + + loc = die.attributes["DW_AT_location"] + if loc.form != "DW_FORM_exprloc" and \ + loc.form != "DW_FORM_block1": + debug_die(die, "kernel object '%s' unexpected location format" % + name) + continue + + opcode = loc.value[0] + if opcode != DW_OP_addr: + + # Check if frame pointer offset DW_OP_fbreg + if opcode == DW_OP_fbreg: + debug_die(die, "kernel object '%s' found on stack" % name) + else: + debug_die(die, + "kernel object '%s' unexpected exprloc opcode %s" % + (name, hex(opcode))) + continue + + addr = (loc.value[1] | (loc.value[2] << 8) | + (loc.value[3] << 16) | (loc.value[4] << 24)) + + if addr == 0: + # Never linked; gc-sections deleted it + continue + + type_obj = type_env[type_offset] + objs = type_obj.get_kobjects(addr) + all_objs.update(objs) + + debug("symbol '%s' at %s contains %d object(s)" + % (name, hex(addr), len(objs))) + + # Step 4: objs is a dictionary mapping variable memory addresses to + # their associated type objects. Now that we have seen all variables + # and can properly look up API structs, convert this into a dictionary + # mapping variables to the C enumeration of what kernel object type it + # is. + ret = {} + for addr, ko in all_objs.items(): + # API structs don't get into the gperf table + if ko.type_obj.api: + continue + + _, user_ram_allowed, _ = kobjects[ko.type_obj.name] + if not user_ram_allowed and app_smem_start <= addr < app_smem_end: + debug("object '%s' found in invalid location %s" + % (ko.type_obj.name, hex(addr))) + continue + + if ko.type_obj.name != "device": + # Not a device struct so we immediately know its type + ko.type_name = kobject_to_enum(ko.type_obj.name) + ret[addr] = ko + continue + + # Device struct. Need to get the address of its API struct, + # if it has one. + apiaddr = device_get_api_addr(elf, addr) + if apiaddr not in all_objs: + if apiaddr == 0: + debug("device instance at 0x%x has no associated subsystem" + % addr) + else: + debug("device instance at 0x%x has unknown API 0x%x" + % (addr, apiaddr)) + # API struct does not correspond to a known subsystem, skip it + continue + + apiobj = all_objs[apiaddr] + ko.type_name = subsystem_to_enum(apiobj.type_obj.name) + ret[addr] = ko + + debug("found %d kernel object instances total" % len(ret)) + + # 1. Before python 3.7 dict order is not guaranteed. With Python + # 3.5 it doesn't seem random with *integer* keys but can't + # rely on that. + # 2. OrderedDict means _insertion_ order, so not enough because + # built from other (random!) dicts: need to _sort_ first. + # 3. Sorting memory address looks good. + return OrderedDict(sorted(ret.items())) + +def get_symbols(elf): + for section in elf.iter_sections(): + if isinstance(section, SymbolTableSection): + return {sym.name: sym.entry.st_value + for sym in section.iter_symbols()} + + raise LookupError("Could not find symbol table") + + +# -- GPERF generation logic + header = """%compare-lengths %define lookup-function-name z_object_lookup %language=ANSI-C @@ -282,7 +829,7 @@ def write_validation_output(fp): def write_kobj_types_output(fp): fp.write("/* Core kernel objects */\n") for kobj, obj_info in kobjects.items(): - dep, _ = obj_info + dep, _, _ = obj_info if kobj == "device": continue @@ -303,7 +850,7 @@ def write_kobj_types_output(fp): def write_kobj_otype_output(fp): fp.write("/* Core kernel objects */\n") for kobj, obj_info in kobjects.items(): - dep, _ = obj_info + dep, _, _ = obj_info if kobj == "device": continue @@ -327,10 +874,9 @@ def write_kobj_otype_output(fp): def write_kobj_size_output(fp): fp.write("/* Non device/stack objects */\n") for kobj, obj_info in kobjects.items(): - dep, _ = obj_info - # device handled by default case. Stacks are not currently handled, - # if they eventually are it will be a special case. - if kobj in {"device", "_k_thread_stack_element"}: + dep, _, alloc = obj_info + + if not alloc: continue if dep: From 3c63cdff11e222c5f6e348d3a1b3e7ccd0e43650 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 13:24:51 -0700 Subject: [PATCH 09/19] scripts: parse_syscalls: generalize struct tags Now we can build up lists of data structures matching a list of particular tags, with __subsystem being just one case. Relax searches to also look inside C files, since struct prototypes may be declared there as well. Signed-off-by: Andrew Boie --- CMakeLists.txt | 15 +++++----- scripts/gen_kobject_list.py | 2 +- scripts/parse_syscalls.py | 57 ++++++++++++++++++++++++------------- 3 files changed, 47 insertions(+), 27 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 670b784a2aed6..8caf9bac6b715 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -492,9 +492,9 @@ if(EXISTS ${CMAKE_BINARY_DIR}/zephyr_modules.txt) set(ZEPHYR_CURRENT_MODULE_DIR) endif() -set(syscall_list_h ${CMAKE_CURRENT_BINARY_DIR}/include/generated/syscall_list.h) -set(syscalls_json ${CMAKE_CURRENT_BINARY_DIR}/misc/generated/syscalls.json) -set(subsys_json ${CMAKE_CURRENT_BINARY_DIR}/misc/generated/subsystems.json) +set(syscall_list_h ${CMAKE_CURRENT_BINARY_DIR}/include/generated/syscall_list.h) +set(syscalls_json ${CMAKE_CURRENT_BINARY_DIR}/misc/generated/syscalls.json) +set(struct_tags_json ${CMAKE_CURRENT_BINARY_DIR}/misc/generated/struct_tags.json) # The syscalls subdirs txt file is constructed by python containing a list of folders to use for # dependency handling, including empty folders. @@ -586,20 +586,20 @@ endforeach() add_custom_command( OUTPUT ${syscalls_json} - ${subsys_json} + ${struct_tags_json} COMMAND ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/parse_syscalls.py --include ${ZEPHYR_BASE}/include # Read files from this dir ${parse_syscalls_include_args} # Read files from these dirs also --json-file ${syscalls_json} # Write this file - --subsystem-file ${subsys_json} # Write subsystem list to this file + --tag-struct-file ${struct_tags_json} # Write subsystem list to this file DEPENDS ${syscalls_subdirs_trigger} ${PARSE_SYSCALLS_HEADER_DEPENDS} ) add_custom_target(${SYSCALL_LIST_H_TARGET} DEPENDS ${syscall_list_h}) add_custom_target(${PARSE_SYSCALLS_TARGET} - DEPENDS ${syscalls_json} ${subsys_json}) + DEPENDS ${syscalls_json} ${struct_tags_json}) # 64-bit systems do not require special handling of 64-bit system call # parameters or return values, indicate this to the system call boilerplate @@ -623,7 +623,7 @@ add_custom_command(OUTPUT include/generated/syscall_dispatch.c ${syscall_list_h} ) # This is passed into all calls to the gen_kobject_list.py script. -set(gen_kobject_list_include_args --include ${subsys_json}) +set(gen_kobject_list_include_args --include ${struct_tags_json}) set(DRV_VALIDATION ${PROJECT_BINARY_DIR}/include/generated/driver-validation.h) add_custom_command( @@ -637,6 +637,7 @@ add_custom_command( DEPENDS ${ZEPHYR_BASE}/scripts/gen_kobject_list.py ${PARSE_SYSCALLS_TARGET} + ${struct_tags_json} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) add_custom_target(${DRIVER_VALIDATION_H_TARGET} DEPENDS ${DRV_VALIDATION}) diff --git a/scripts/gen_kobject_list.py b/scripts/gen_kobject_list.py index 10b2526a6cff7..d02f1fe46128a 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -891,7 +891,7 @@ def write_kobj_size_output(fp): def parse_subsystems_list_file(path): with open(path, "r") as fp: subsys_list = json.load(fp) - subsystems.extend(subsys_list) + subsystems.extend(subsys_list["__subsystem"]) def parse_args(): global args diff --git a/scripts/parse_syscalls.py b/scripts/parse_syscalls.py index 25c56223da84e..f3bddefc349e9 100644 --- a/scripts/parse_syscalls.py +++ b/scripts/parse_syscalls.py @@ -10,10 +10,14 @@ System calls require a great deal of boilerplate code in order to implement completely. This script is the first step in the build system's process of auto-generating this code by doing a text scan of directories containing -header files, and building up a database of system calls and their +C or header files, and building up a database of system calls and their function call prototypes. This information is emitted to a generated JSON file for further processing. +This script also scans for struct definitions such as __subsystem and +__net_socket, emitting a JSON dictionary mapping tags to all the struct +declarations found that were tagged with them. + If the output JSON file already exists, its contents are checked against what information this script would have outputted; if the result is that the file would be unchanged, it is not modified to prevent unnecessary @@ -26,24 +30,37 @@ import os import json +regex_flags = re.MULTILINE | re.VERBOSE + syscall_regex = re.compile(r''' __syscall\s+ # __syscall attribute, must be first ([^(]+) # type and name of system call (split later) [(] # Function opening parenthesis ([^)]*) # Arg list (split later) [)] # Closing parenthesis -''', re.MULTILINE | re.VERBOSE) +''', regex_flags) -subsys_regex = re.compile(r''' -__subsystem\s+ # __subsystem attribute, must be first +struct_tags = ["__subsystem"] + +tagged_struct_decl_template = r''' +%s\s+ # tag, must be first struct\s+ # struct keyword is next ([^{]+) # name of subsystem [{] # Open curly bracket -''', re.MULTILINE | re.VERBOSE) +''' + +def tagged_struct_update(target_list, tag, contents): + regex = re.compile(tagged_struct_decl_template % tag, regex_flags) + items = [mo.groups()[0].strip() for mo in regex.finditer(contents)] + target_list.extend(items) + def analyze_headers(multiple_directories): syscall_ret = [] - subsys_ret = [] + tagged_ret = {} + + for tag in struct_tags: + tagged_ret[tag] = [] for base_path in multiple_directories: for root, dirs, files in os.walk(base_path, topdown=True): @@ -51,10 +68,12 @@ def analyze_headers(multiple_directories): files.sort() for fn in files: - # toolchain/common.h has the definitions of __syscall and __subsystem which we + # toolchain/common.h has the definitions of these tagswhich we # don't want to trip over path = os.path.join(root, fn) - if not fn.endswith(".h") or path.endswith(os.path.join(os.sep, 'toolchain', 'common.h')): + if (not (path.endswith(".h") or path.endswith(".c")) or + path.endswith(os.path.join(os.sep, 'toolchain', + 'common.h'))): continue with open(path, "r", encoding="utf-8") as fp: @@ -63,16 +82,15 @@ def analyze_headers(multiple_directories): try: syscall_result = [(mo.groups(), fn) for mo in syscall_regex.finditer(contents)] - subsys_result = [mo.groups()[0].strip() - for mo in subsys_regex.finditer(contents)] + for tag in struct_tags: + tagged_struct_update(tagged_ret[tag], tag, contents) except Exception: sys.stderr.write("While parsing %s\n" % fn) raise syscall_ret.extend(syscall_result) - subsys_ret.extend(subsys_result) - return syscall_ret, subsys_ret + return syscall_ret, tagged_ret def update_file_if_changed(path, new): @@ -102,18 +120,19 @@ def parse_args(): "-j", "--json-file", required=True, help="Write system call prototype information as json to file") parser.add_argument( - "-s", "--subsystem-file", required=True, - help="Write subsystem name information as json to file") + "-t", "--tag-struct-file", required=True, + help="Write tagged struct name information as json to file") + args = parser.parse_args() def main(): parse_args() - syscalls, subsys = analyze_headers(args.include) + syscalls, tagged = analyze_headers(args.include) # Only write json files if they don't exist or have changes since - # they will force and incremental rebuild. + # they will force an incremental rebuild. syscalls_in_json = json.dumps( syscalls, @@ -122,12 +141,12 @@ def main(): ) update_file_if_changed(args.json_file, syscalls_in_json) - subsys_in_json = json.dumps( - subsys, + tagged_struct_in_json = json.dumps( + tagged, indent=4, sort_keys=True ) - update_file_if_changed(args.subsystem_file, subsys_in_json) + update_file_if_changed(args.tag_struct_file, tagged_struct_in_json) if __name__ == "__main__": From 5471834ae73d519272db0f10753a3041c8985a5b Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 13:30:19 -0700 Subject: [PATCH 10/19] userspace: net sockets are kernel objects Any data structure declaration tagged with __net_socket will end up in the kernel object table with type K_OBJ_NET_SOCKET. These all correspond to objects which are associated with socket file descriptors and can handle the socket vtable API. Signed-off-by: Andrew Boie --- include/toolchain/common.h | 10 ++++++++-- scripts/gen_kobject_list.py | 11 +++++++++++ scripts/parse_syscalls.py | 2 +- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/include/toolchain/common.h b/include/toolchain/common.h index 331ed433be059..2d9ca52106179 100644 --- a/include/toolchain/common.h +++ b/include/toolchain/common.h @@ -132,11 +132,17 @@ #define __syscall #endif /* #ifndef ZTEST_UNITTEST */ -/* Used as a sentinel by parse_syscalls.py to identify what API structs - * define driver subsystems. +/* Definitions for struct declaration tags. These are sentinel values used by + * parse_syscalls.py to gather a list of names of struct declarations that + * have these tags applied for them. */ + +/* Indicates this is a driver subsystem */ #define __subsystem +/* Indicates this is a network socket object */ +#define __net_socket + #ifndef BUILD_ASSERT /* compile-time assertion that makes the build fail */ #define BUILD_ASSERT(EXPR) \ diff --git a/scripts/gen_kobject_list.py b/scripts/gen_kobject_list.py index d02f1fe46128a..1ea24eafd6c96 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -73,6 +73,10 @@ # # - The third items is a boolean indicating whether this item can be # dynamically allocated with k_object_alloc() +# +# Key names in all caps do not correspond to a specific data type but instead +# indicate that objects of its type are of a family of compatible data +# structures # Regular dictionaries are ordered only with Python 3.6 and # above. Good summary and pointers to official documents at: @@ -90,6 +94,7 @@ ("k_timer", (None, False, True)), ("_k_thread_stack_element", (None, False, False)), ("device", (None, False, False)), + ("NET_SOCKET", (None, False, False)), ("sys_mutex", (None, True, False)), ("k_futex", (None, True, False)) ]) @@ -121,6 +126,9 @@ "sample_driver_api" ] +# Names of all structs tagged with __net_socket, found by parse_syscalls.py +net_sockets = [ ] + def subsystem_to_enum(subsys): return "K_OBJ_DRIVER_" + subsys[:-11].upper() @@ -405,6 +413,8 @@ def analyze_die_struct(die): type_env[offset] = KobjectType(offset, name, size) elif name in subsystems: type_env[offset] = KobjectType(offset, name, size, api=True) + elif name in net_sockets: + type_env[offset] = KobjectType(offset, "NET_SOCKET", size) else: at = AggregateType(offset, name, size) type_env[offset] = at @@ -892,6 +902,7 @@ def parse_subsystems_list_file(path): with open(path, "r") as fp: subsys_list = json.load(fp) subsystems.extend(subsys_list["__subsystem"]) + net_sockets.extend(subsys_list["__net_socket"]) def parse_args(): global args diff --git a/scripts/parse_syscalls.py b/scripts/parse_syscalls.py index f3bddefc349e9..e00474fe71b61 100644 --- a/scripts/parse_syscalls.py +++ b/scripts/parse_syscalls.py @@ -40,7 +40,7 @@ [)] # Closing parenthesis ''', regex_flags) -struct_tags = ["__subsystem"] +struct_tags = ["__subsystem", "__net_socket"] tagged_struct_decl_template = r''' %s\s+ # tag, must be first From 43e038682f28aed7e283d8c2f051522e9ea8777e Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 13:33:12 -0700 Subject: [PATCH 11/19] net: tag net socket objects Used for permission validation when accessing the associated file descriptors from user mode. There often get defined in implementation code, expand the search to look in drivers/ and subsys/net/. Signed-off-by: Andrew Boie --- CMakeLists.txt | 2 ++ drivers/modem/modem_socket.h | 2 +- include/net/net_context.h | 2 +- subsys/net/lib/sockets/sockets_net_mgmt.c | 2 +- subsys/net/lib/websocket/websocket_internal.h | 4 +++- 5 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8caf9bac6b715..c873e2576ee26 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -591,6 +591,8 @@ add_custom_command( ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/parse_syscalls.py --include ${ZEPHYR_BASE}/include # Read files from this dir + --include ${ZEPHYR_BASE}/drivers # For net sockets + --include ${ZEPHYR_BASE}/subsys/net # More net sockets ${parse_syscalls_include_args} # Read files from these dirs also --json-file ${syscalls_json} # Write this file --tag-struct-file ${struct_tags_json} # Write subsystem list to this file diff --git a/drivers/modem/modem_socket.h b/drivers/modem/modem_socket.h index 055c7b997c518..0efa38378f495 100644 --- a/drivers/modem/modem_socket.h +++ b/drivers/modem/modem_socket.h @@ -23,7 +23,7 @@ extern "C" { #endif -struct modem_socket { +__net_socket struct modem_socket { sa_family_t family; enum net_sock_type type; enum net_ip_protocol ip_proto; diff --git a/include/net/net_context.h b/include/net/net_context.h index c2c266ed8c7c4..308b4669582a9 100644 --- a/include/net/net_context.h +++ b/include/net/net_context.h @@ -196,7 +196,7 @@ struct tls_context; * If there is no such source address there, the packet cannot be sent * anyway. This saves 12 bytes / context in IPv6. */ -struct net_context { +__net_socket struct net_context { /** User data. * * First member of the structure to let users either have user data diff --git a/subsys/net/lib/sockets/sockets_net_mgmt.c b/subsys/net/lib/sockets/sockets_net_mgmt.c index 499601bc6274d..1ae9131316004 100644 --- a/subsys/net/lib/sockets/sockets_net_mgmt.c +++ b/subsys/net/lib/sockets/sockets_net_mgmt.c @@ -23,7 +23,7 @@ LOG_MODULE_REGISTER(net_sock_mgmt, CONFIG_NET_SOCKETS_LOG_LEVEL); #define MSG_ALLOC_TIMEOUT K_MSEC(100) -struct net_mgmt_socket { +__net_socket struct net_mgmt_socket { /* Network interface related to this socket */ struct net_if *iface; diff --git a/subsys/net/lib/websocket/websocket_internal.h b/subsys/net/lib/websocket/websocket_internal.h index 036d4ffafa66e..00ba18b51bf6e 100644 --- a/subsys/net/lib/websocket/websocket_internal.h +++ b/subsys/net/lib/websocket/websocket_internal.h @@ -10,6 +10,8 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include + #define WS_SHA1_OUTPUT_LEN 20 /* Min Websocket header length */ @@ -24,7 +26,7 @@ /** * Websocket connection information */ -struct websocket_context { +__net_socket struct websocket_context { union { /** User data. */ From e64cd690c9318f55d60882ad15a0e8ab8b876475 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Thu, 28 May 2020 14:36:36 -0700 Subject: [PATCH 12/19] net: sockets: add API to fetch an fd's ctx object Zephyr running on MPU devices have a different memory model than process-oriented OSes like Linux and require a method to set kernel object permissions on a file descriptor's underlying context object. Add this, and a test to show that it is working. Signed-off-by: Andrew Boie --- include/net/socket.h | 43 ++++++++++++++++++++++++++++++++ subsys/net/lib/sockets/sockets.c | 17 +++++++++++++ 2 files changed, 60 insertions(+) diff --git a/include/net/socket.h b/include/net/socket.h index 6384ed667388a..48e721e992099 100644 --- a/include/net/socket.h +++ b/include/net/socket.h @@ -145,6 +145,44 @@ struct zsock_addrinfo { char _ai_canonname[DNS_MAX_NAME_SIZE + 1]; }; +/** + * @brief Obtain a file descriptor's associated net context + * + * With CONFIG_USERSPACE enabled, the kernel's object permission system + * must apply to socket file descriptors. When a socket is opened, by default + * only the caller has permission, access by other threads will fail unless + * they have been specifically granted permission. + * + * This is achieved by tagging data structure definitions that implement the + * underlying object associated with a network socket file descriptor with + * '__net_socket`. All pointers to instances of these will be known to the + * kernel as kernel objects with type K_OBJ_NET_SOCKET. + * + * This API is intended for threads that need to grant access to the object + * associated with a particular file descriptor to another thread. The + * returned pointer represents the underlying K_OBJ_NET_SOCKET and + * may be passed to APIs like k_object_access_grant(). + * + * In a system like Linux which has the notion of threads running in processes + * in a shared virtual address space, this sort of management is unnecessary as + * the scope of file descriptors is implemented at the process level. + * + * However in Zephyr the file descriptor scope is global, and MPU-based systems + * are not able to implement a process-like model due to the lack of memory + * virtualization hardware. They use discrete object permissions and memory + * domains instead to define thread access scope. + * + * User threads will have no direct access to the returned object + * and will fault if they try to access its memory; the pointer can only be + * used to make permission assignment calls, which follow exactly the rules + * for other kernel objects like device drivers and IPC. + * + * @param sock file descriptor + * @return pointer to associated network socket object, or NULL if the + * file descriptor wasn't valid or the caller had no access permission + */ +__syscall void *zsock_get_context_object(int sock); + /** * @brief Create a network socket * @@ -156,6 +194,11 @@ struct zsock_addrinfo { * This function is also exposed as ``socket()`` * if :option:`CONFIG_NET_SOCKETS_POSIX_NAMES` is defined. * @endrst + * + * If CONFIG_USERSPACE is enabled, the caller will be granted access to the + * context object associated with the returned file descriptor. + * @see zsock_get_context_object() + * */ __syscall int zsock_socket(int family, int type, int proto); diff --git a/subsys/net/lib/sockets/sockets.c b/subsys/net/lib/sockets/sockets.c index 3c12e1439c6ac..11ffd1b322bf5 100644 --- a/subsys/net/lib/sockets/sockets.c +++ b/subsys/net/lib/sockets/sockets.c @@ -49,6 +49,23 @@ static inline void *get_sock_vtable( (const struct fd_op_vtable **)vtable); } +void *z_impl_zsock_get_context_object(int sock) +{ + const struct socket_op_vtable *ignored; + + return get_sock_vtable(sock, &ignored); +} + +#ifdef CONFIG_USERSPACE +void *z_vrfy_zsock_get_context_object(int sock) +{ + /* All checking done in implementation */ + return z_impl_zsock_get_context_object(sock); +} + +#include +#endif + static void zsock_received_cb(struct net_context *ctx, struct net_pkt *pkt, union net_ip_header *ip_hdr, From 50096c3af61c0cc046a9c91525c16154c3da9378 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 13:37:49 -0700 Subject: [PATCH 13/19] tests: net_mgmt: grant socket access This socket is shared by all the test cases which run in different threads. Just make it a global object here. Signed-off-by: Andrew Boie --- tests/net/socket/net_mgmt/src/main.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/net/socket/net_mgmt/src/main.c b/tests/net/socket/net_mgmt/src/main.c index d3473c5aaa211..9b80eeddb967d 100644 --- a/tests/net/socket/net_mgmt/src/main.c +++ b/tests/net/socket/net_mgmt/src/main.c @@ -350,6 +350,16 @@ static void test_net_mgmt_setup(void) fd = socket(AF_NET_MGMT, SOCK_DGRAM, NET_MGMT_EVENT_PROTO); zassert_false(fd < 0, "Cannot create net_mgmt socket (%d)", errno); +#ifdef CONFIG_USERSPACE + /* Set the underlying net_context to global access scope so that + * other scenario threads may use it + */ + void *ctx = zsock_get_context_object(fd); + + zassert_not_null(ctx, "null net_context"); + k_object_access_all_grant(ctx); +#endif /* CONFIG_USERSPACE */ + memset(&sockaddr, 0, sizeof(sockaddr)); sockaddr.nm_family = AF_NET_MGMT; From 966b5386534005155d98ce95ee79fb0b0b4c7630 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 13:57:21 -0700 Subject: [PATCH 14/19] tests: net: tcp: test zsock_get_context_object Add test case to prove that this new API works. Signed-off-by: Andrew Boie --- tests/net/socket/tcp/src/main.c | 69 ++++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/tests/net/socket/tcp/src/main.c b/tests/net/socket/tcp/src/main.c index 72903cb0ee966..9fcf9d880ef40 100644 --- a/tests/net/socket/tcp/src/main.c +++ b/tests/net/socket/tcp/src/main.c @@ -446,8 +446,73 @@ void test_v4_accept_timeout(void) k_sleep(TCP_TEARDOWN_TIMEOUT); } +#ifdef CONFIG_USERSPACE +#define CHILD_STACK_SZ (2048 + CONFIG_TEST_EXTRA_STACKSIZE) +struct k_thread child_thread; +K_THREAD_STACK_DEFINE(child_stack, CHILD_STACK_SZ); +ZTEST_BMEM volatile int result; + +static void child_entry(void *p1, void *p2, void *p3) +{ + int sock = (int)p1; + + result = close(sock); +} + +static void spawn_child(int sock) +{ + k_thread_create(&child_thread, child_stack, + K_THREAD_STACK_SIZEOF(child_stack), child_entry, + (void *)sock, NULL, NULL, 0, K_USER, + K_FOREVER); +} +#endif + +void test_socket_permission(void) +{ +#ifdef CONFIG_USERSPACE + int sock; + struct sockaddr_in saddr; + struct net_context *ctx; + + prepare_sock_tcp_v4(CONFIG_NET_CONFIG_MY_IPV4_ADDR, ANY_PORT, + &sock, &saddr); + + ctx = zsock_get_context_object(sock); + zassert_not_null(ctx, "zsock_get_context_object() failed"); + + /* Spawn a child thread which doesn't inherit our permissions, + * it will try to perform a socket operation and fail due to lack + * of permissions on it. + */ + spawn_child(sock); + k_thread_start(&child_thread); + k_thread_join(&child_thread, K_FOREVER); + + zassert_not_equal(result, 0, "child succeeded with no permission"); + + /* Now spawn the same child thread again, but this time we grant + * permission on the net_context before we start it, and the + * child should now succeed. + */ + spawn_child(sock); + k_object_access_grant(ctx, &child_thread); + k_thread_start(&child_thread); + k_thread_join(&child_thread, K_FOREVER); + + zassert_equal(result, 0, "child failed with permissions"); +#else + ztest_test_skip(); +#endif /* CONFIG_USERSPACE */ +} + void test_main(void) { +#ifdef CONFIG_USERSPACE + /* ztest thread inherit permissions from main */ + k_thread_access_grant(k_current_get(), &child_thread, child_stack); +#endif + ztest_test_suite( socket_tcp, ztest_user_unit_test(test_v4_send_recv), @@ -457,7 +522,9 @@ void test_main(void) ztest_user_unit_test(test_v4_sendto_recvfrom_null_dest), ztest_user_unit_test(test_v6_sendto_recvfrom_null_dest), ztest_unit_test(test_open_close_immediately), - ztest_user_unit_test(test_v4_accept_timeout)); + ztest_user_unit_test(test_v4_accept_timeout), + ztest_user_unit_test(test_socket_permission) + ); ztest_run_test_suite(socket_tcp); } From ce21e4f64fc7c40630954df3244b37765f60029d Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 14:52:26 -0700 Subject: [PATCH 15/19] fdtable: init fd context objects Anytime a file descriptor context object is updated, we need to reset its access permissions and initialization state. This is the most centralized place to do it. Signed-off-by: Andrew Boie --- lib/os/fdtable.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/os/fdtable.c b/lib/os/fdtable.c index ef2c0433021be..53dcbecf15379 100644 --- a/lib/os/fdtable.c +++ b/lib/os/fdtable.c @@ -18,6 +18,7 @@ #include #include #include +#include struct fd_entry { void *obj; @@ -132,6 +133,16 @@ int z_reserve_fd(void) void z_finalize_fd(int fd, void *obj, const struct fd_op_vtable *vtable) { /* Assumes fd was already bounds-checked. */ +#ifdef CONFIG_USERSPACE + /* descriptor context objects are inserted into the table when they + * are ready for use. Mark the object as initialized and grant the + * caller (and only the caller) access. + * + * This call is a no-op if obj is invalid or points to something + * not a kernel object. + */ + z_object_recycle(obj); +#endif fdtable[fd].obj = obj; fdtable[fd].vtable = vtable; } From 96b0552dd8005ad16f1c6ccc264f57fcdaec9289 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 14:41:13 -0700 Subject: [PATCH 16/19] net: lib: remove socket-specific recycle calls This is just done in common code now. Signed-off-by: Andrew Boie --- subsys/net/lib/sockets/sockets_can.c | 7 ------- subsys/net/lib/sockets/sockets_net_mgmt.c | 7 ------- subsys/net/lib/sockets/sockets_packet.c | 8 -------- subsys/net/lib/sockets/sockets_tls.c | 11 ----------- subsys/net/lib/websocket/websocket.c | 8 -------- 5 files changed, 41 deletions(-) diff --git a/subsys/net/lib/sockets/sockets_can.c b/subsys/net/lib/sockets/sockets_can.c index 27688e328d734..c4e560acb929d 100644 --- a/subsys/net/lib/sockets/sockets_can.c +++ b/subsys/net/lib/sockets/sockets_can.c @@ -70,13 +70,6 @@ int zcan_socket(int family, int type, int proto) k_fifo_init(&ctx->recv_q); -#ifdef CONFIG_USERSPACE - /* Set net context object as initialized and grant access to the - * calling thread (and only the calling thread) - */ - z_object_recycle(ctx); -#endif - z_finalize_fd(fd, ctx, (const struct fd_op_vtable *)&can_sock_fd_op_vtable); diff --git a/subsys/net/lib/sockets/sockets_net_mgmt.c b/subsys/net/lib/sockets/sockets_net_mgmt.c index 1ae9131316004..db67bde4ad9a7 100644 --- a/subsys/net/lib/sockets/sockets_net_mgmt.c +++ b/subsys/net/lib/sockets/sockets_net_mgmt.c @@ -80,13 +80,6 @@ int znet_mgmt_socket(int family, int type, int proto) mgmt->alloc_timeout = MSG_ALLOC_TIMEOUT; mgmt->wait_timeout = K_FOREVER; -#if defined(CONFIG_USERSPACE) - /* Set net context object as initialized and grant access to the - * calling thread (and only the calling thread) - */ - z_object_recycle(mgmt); -#endif - z_finalize_fd(fd, mgmt, (const struct fd_op_vtable *)&net_mgmt_sock_fd_op_vtable); diff --git a/subsys/net/lib/sockets/sockets_packet.c b/subsys/net/lib/sockets/sockets_packet.c index 029796045ce7a..c8667d34ddb4f 100644 --- a/subsys/net/lib/sockets/sockets_packet.c +++ b/subsys/net/lib/sockets/sockets_packet.c @@ -67,14 +67,6 @@ static int zpacket_socket(int family, int type, int proto) /* recv_q and accept_q are in union */ k_fifo_init(&ctx->recv_q); - -#ifdef CONFIG_USERSPACE - /* Set net context object as initialized and grant access to the - * calling thread (and only the calling thread) - */ - z_object_recycle(ctx); -#endif - z_finalize_fd(fd, ctx, (const struct fd_op_vtable *)&packet_sock_fd_op_vtable); diff --git a/subsys/net/lib/sockets/sockets_tls.c b/subsys/net/lib/sockets/sockets_tls.c index 1db3e4f1d537d..d841fd1986f1a 100644 --- a/subsys/net/lib/sockets/sockets_tls.c +++ b/subsys/net/lib/sockets/sockets_tls.c @@ -1154,13 +1154,6 @@ static int ztls_socket(int family, int type, int proto) /* recv_q and accept_q are in union */ k_fifo_init(&ctx->recv_q); -#ifdef CONFIG_USERSPACE - /* Set net context object as initialized and grant access to the - * calling thread (and only the calling thread) - */ - z_object_recycle(ctx); -#endif - if (tls_proto != 0) { /* If TLS protocol is used, allocate TLS context */ ctx->tls = tls_alloc(); @@ -1276,10 +1269,6 @@ int ztls_accept_ctx(struct net_context *parent, struct sockaddr *addr, child = k_fifo_get(&parent->accept_q, K_FOREVER); - #ifdef CONFIG_USERSPACE - z_object_recycle(child); - #endif - if (addr != NULL && addrlen != NULL) { int len = MIN(*addrlen, sizeof(child->remote)); diff --git a/subsys/net/lib/websocket/websocket.c b/subsys/net/lib/websocket/websocket.c index fa3046be68e40..979eb77084b5d 100644 --- a/subsys/net/lib/websocket/websocket.c +++ b/subsys/net/lib/websocket/websocket.c @@ -365,14 +365,6 @@ int websocket_connect(int sock, struct websocket_request *wreq, } ctx->sock = fd; - -#ifdef CONFIG_USERSPACE - /* Set net context object as initialized and grant access to the - * calling thread (and only the calling thread) - */ - z_object_recycle(ctx); -#endif - z_finalize_fd(fd, ctx, (const struct fd_op_vtable *)&websocket_fd_op_vtable); From 5b270b138112a3ba0d81718a09e5938a792ecf16 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 17:49:02 -0700 Subject: [PATCH 17/19] userspace: improve dynamic object allocation We now have a low-level function z_dynamic_object_create() which is not a system call and is used for installing kernel objects that are not supported by k_object_alloc(). Checking for valid object type enumeration values moved completely to the implementation function. A few debug messages and comments were improved. Futexes and sys_mutexes are now properly excluded from dynamic generation. Signed-off-by: Andrew Boie --- include/kernel.h | 29 ++++++++++++ kernel/userspace.c | 88 ++++++++++++++++++++++++++----------- kernel/userspace_handler.c | 4 -- scripts/gen_kobject_list.py | 3 +- 4 files changed, 94 insertions(+), 30 deletions(-) diff --git a/include/kernel.h b/include/kernel.h index f8d3f13dbdfb7..a01b743de7c25 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -356,6 +356,27 @@ void k_object_access_all_grant(void *object); __syscall void *k_object_alloc(enum k_objects otype); #ifdef CONFIG_DYNAMIC_OBJECTS +/** + * Allocate memory and install as a generic kernel object + * + * This is a low-level function to allocate some memory, and register that + * allocated memory in the kernel object lookup tables with type K_OBJ_ANY. + * Initialization state and thread permissions will be cleared. The + * returned z_object's data value will be uninitialized. + * + * Most users will want to use k_object_alloc() instead. + * + * Memory allocated will be drawn from the calling thread's reasource pool + * and may be freed later by passing the actual object pointer (found + * in the returned z_object's 'name' member) to k_object_free(). + * + * @param size Size of the allocated object + * @return NULL on insufficient memory + * @return A pointer to the associated z_object that is installed in the + * kernel object tables + */ +struct z_object *z_dynamic_object_create(size_t size); + /** * Free a kernel object previously allocated with k_object_alloc() * @@ -374,6 +395,14 @@ static inline void *z_impl_k_object_alloc(enum k_objects otype) return NULL; } + +static inline struct z_object *z_dynamic_object_create(size_t size) +{ + ARG_UNUSED(size); + + return NULL; +} + /** * @brief Free an object * diff --git a/kernel/userspace.c b/kernel/userspace.c index 0b687cddb3d90..8e53ef50f6091 100644 --- a/kernel/userspace.c +++ b/kernel/userspace.c @@ -72,6 +72,9 @@ const char *otype_to_str(enum k_objects otype) /* otype-to-str.h is generated automatically during build by * gen_kobject_list.py */ + case K_OBJ_ANY: + ret = "generic"; + break; #include default: ret = "?"; @@ -233,51 +236,80 @@ static void thread_idx_free(uintptr_t tidx) sys_bitfield_set_bit((mem_addr_t)_thread_idx_map, tidx); } -void *z_impl_k_object_alloc(enum k_objects otype) +struct z_object *z_dynamic_object_create(size_t size) { struct dyn_obj *dyn_obj; - uintptr_t tidx; - - /* Stacks are not supported, we don't yet have mem pool APIs - * to request memory that is aligned - */ - __ASSERT(otype > K_OBJ_ANY && otype < K_OBJ_LAST && - otype != K_OBJ__THREAD_STACK_ELEMENT, - "bad object type requested"); - dyn_obj = z_thread_malloc(sizeof(*dyn_obj) + obj_size_get(otype)); + dyn_obj = z_thread_malloc(sizeof(*dyn_obj) + size); if (dyn_obj == NULL) { - LOG_WRN("could not allocate kernel object"); + LOG_ERR("could not allocate kernel object, out of memory"); return NULL; } - dyn_obj->kobj.name = (char *)&dyn_obj->data; - dyn_obj->kobj.type = otype; - dyn_obj->kobj.flags = K_OBJ_FLAG_ALLOC; + dyn_obj->kobj.name = &dyn_obj->data; + dyn_obj->kobj.type = K_OBJ_ANY; + dyn_obj->kobj.flags = 0; (void)memset(dyn_obj->kobj.perms, 0, CONFIG_MAX_THREAD_BYTES); - /* Need to grab a new thread index for k_thread */ - if (otype == K_OBJ_THREAD) { + k_spinlock_key_t key = k_spin_lock(&lists_lock); + + rb_insert(&obj_rb_tree, &dyn_obj->node); + sys_dlist_append(&obj_list, &dyn_obj->obj_list); + k_spin_unlock(&lists_lock, key); + + return &dyn_obj->kobj; +} + +void *z_impl_k_object_alloc(enum k_objects otype) +{ + struct z_object *zo; + uintptr_t tidx; + + if (otype <= K_OBJ_ANY || otype >= K_OBJ_LAST) { + LOG_ERR("bad object type %d requested", otype); + return NULL; + } + + switch (otype) { + case K_OBJ_THREAD: if (!thread_idx_alloc(&tidx)) { - k_free(dyn_obj); + LOG_ERR("out of free thread indexes"); return NULL; } + break; + /* The following are currently not allowed at all */ + case K_OBJ_FUTEX: /* Lives in user memory */ + case K_OBJ_SYS_MUTEX: /* Lives in user memory */ + case K_OBJ_NET_SOCKET: /* Indeterminate size */ + LOG_ERR("forbidden object type '%s' requested", + otype_to_str(otype)); + return NULL; + default: + /* Remainder within bounds are permitted */ + break; + } - dyn_obj->kobj.data.thread_id = tidx; + zo = z_dynamic_object_create(obj_size_get(otype)); + if (zo == NULL) { + return NULL; + } + zo->type = otype; + + if (otype == K_OBJ_THREAD) { + zo->data.thread_id = tidx; } /* The allocating thread implicitly gets permission on kernel objects * that it allocates */ - z_thread_perms_set(&dyn_obj->kobj, _current); + z_thread_perms_set(zo, _current); - k_spinlock_key_t key = k_spin_lock(&lists_lock); - - rb_insert(&obj_rb_tree, &dyn_obj->node); - sys_dlist_append(&obj_list, &dyn_obj->obj_list); - k_spin_unlock(&lists_lock, key); + /* Activates reference counting logic for automatic disposal when + * all permissions have been revoked + */ + zo->flags |= K_OBJ_FLAG_ALLOC; - return dyn_obj->kobj.name; + return zo->name; } void k_object_free(void *obj) @@ -489,6 +521,12 @@ void z_dump_object_error(int retval, void *obj, struct z_object *ko, switch (retval) { case -EBADF: LOG_ERR("%p is not a valid %s", obj, otype_to_str(otype)); + if (ko == NULL) { + LOG_ERR("address is not a known kernel object"); + } else { + LOG_ERR("address is actually a %s", + otype_to_str(ko->type)); + } break; case -EPERM: dump_permission_error(ko); diff --git a/kernel/userspace_handler.c b/kernel/userspace_handler.c index 96d375ced21b9..199da520a5f92 100644 --- a/kernel/userspace_handler.c +++ b/kernel/userspace_handler.c @@ -62,10 +62,6 @@ static inline void z_vrfy_k_object_release(void *object) static inline void *z_vrfy_k_object_alloc(enum k_objects otype) { - Z_OOPS(Z_SYSCALL_VERIFY_MSG(otype > K_OBJ_ANY && otype < K_OBJ_LAST && - otype != K_OBJ__THREAD_STACK_ELEMENT, - "bad object type %d requested", otype)); - return z_impl_k_object_alloc(otype); } #include diff --git a/scripts/gen_kobject_list.py b/scripts/gen_kobject_list.py index 1ea24eafd6c96..9080b9ddf39fe 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -72,7 +72,8 @@ # the object to be located in user-accessible memory. # # - The third items is a boolean indicating whether this item can be -# dynamically allocated with k_object_alloc() +# dynamically allocated with k_object_alloc(). Keep this in sync with +# the switch statement in z_impl_k_object_alloc(). # # Key names in all caps do not correspond to a specific data type but instead # indicate that objects of its type are of a family of compatible data From 83ba2b186e3e68776b2c13937187b0a8f1ff96cc Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Thu, 28 May 2020 12:31:11 -0700 Subject: [PATCH 18/19] net: check permissions on net contexts The original sockets system calls used file descriptors which were actually net_context pointers. For all socket system calls, any calls from user mode would check if the caller had permission to use the net context. This was later changed to not stuff net_context pointers into file descriptors, but all the permission checking was unintentionally lost, allowing all threads on the system to read/write all socket file descriptors in the system at will, with no way to isolate applications running on the same microcontroller from each other's network activity. This patch restores the permission checks on network context objects for socket system calls that originated from user mode. The call to z_object_recycle() was never removed from zsock_socket_internal(); this is again leveraged to grant the caller who opened the socket permission on the net_context associated with the returned file descriptor. To ensure that all socket calls do this checking, all uses of z_get_fd_obj_and_vtable() have been routed through get_sock_vtable(). Objects have initialization state set and thread permissions reset to just the caller in common zsock_socket() code. Signed-off-by: Andrew Boie --- subsys/net/lib/sockets/sockets.c | 73 ++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/subsys/net/lib/sockets/sockets.c b/subsys/net/lib/sockets/sockets.c index 11ffd1b322bf5..1fe59d31d85d7 100644 --- a/subsys/net/lib/sockets/sockets.c +++ b/subsys/net/lib/sockets/sockets.c @@ -45,8 +45,36 @@ const struct socket_op_vtable sock_fd_op_vtable; static inline void *get_sock_vtable( int sock, const struct socket_op_vtable **vtable) { - return z_get_fd_obj_and_vtable(sock, - (const struct fd_op_vtable **)vtable); + void *ctx; + + ctx = z_get_fd_obj_and_vtable(sock, + (const struct fd_op_vtable **)vtable); + +#ifdef CONFIG_USERSPACE + if (z_is_in_user_syscall()) { + struct z_object *zo; + int ret; + + zo = z_object_find(ctx); + ret = z_object_validate(zo, K_OBJ_NET_SOCKET, _OBJ_INIT_TRUE); + + if (ret != 0) { + z_dump_object_error(ret, ctx, zo, K_OBJ_NET_SOCKET); + /* Invalidate the context, the caller doesn't have + * sufficient permission or there was some other + * problem with the net socket object + */ + ctx = NULL; + } + } +#endif /* CONFIG_USERSPACE */ + + if (ctx == NULL) { + NET_ERR("invalid access on sock %d by thread %p", sock, + _current); + } + + return ctx; } void *z_impl_zsock_get_context_object(int sock) @@ -139,13 +167,6 @@ int zsock_socket_internal(int family, int type, int proto) /* recv_q and accept_q are in union */ k_fifo_init(&ctx->recv_q); -#ifdef CONFIG_USERSPACE - /* Set net context object as initialized and grant access to the - * calling thread (and only the calling thread) - */ - z_object_recycle(ctx); -#endif - /* TCP context is effectively owned by both application * and the stack: stack may detect that peer closed/aborted * connection, but it must not dispose of the context behind @@ -202,9 +223,6 @@ static inline int z_vrfy_zsock_socket(int family, int type, int proto) int zsock_close_ctx(struct net_context *ctx) { -#ifdef CONFIG_USERSPACE - z_object_uninit(ctx); -#endif /* Reset callbacks to avoid any race conditions while * flushing queues. No need to check return values here, * as these are fail-free operations and we're closing @@ -225,8 +243,8 @@ int zsock_close_ctx(struct net_context *ctx) int z_impl_zsock_close(int sock) { - const struct fd_op_vtable *vtable; - void *ctx = z_get_fd_obj_and_vtable(sock, &vtable); + const struct socket_op_vtable *vtable; + void *ctx = get_sock_vtable(sock, &vtable); if (ctx == NULL) { return -1; @@ -236,7 +254,8 @@ int z_impl_zsock_close(int sock) NET_DBG("close: ctx=%p, fd=%d", ctx, sock); - return z_fdtable_call_ioctl(vtable, ctx, ZFD_IOCTL_CLOSE); + return z_fdtable_call_ioctl((const struct fd_op_vtable *)vtable, + ctx, ZFD_IOCTL_CLOSE); } #ifdef CONFIG_USERSPACE @@ -466,9 +485,6 @@ int zsock_accept_ctx(struct net_context *parent, struct sockaddr *addr, net_context_set_accepting(ctx, false); -#ifdef CONFIG_USERSPACE - z_object_recycle(ctx); -#endif if (addr != NULL && addrlen != NULL) { int len = MIN(*addrlen, sizeof(ctx->remote)); @@ -982,15 +998,16 @@ ssize_t z_vrfy_zsock_recvfrom(int sock, void *buf, size_t max_len, int flags, */ int z_impl_zsock_fcntl(int sock, int cmd, int flags) { - const struct fd_op_vtable *vtable; + const struct socket_op_vtable *vtable; void *obj; - obj = z_get_fd_obj_and_vtable(sock, &vtable); + obj = get_sock_vtable(sock, &vtable); if (obj == NULL) { return -1; } - return z_fdtable_call_ioctl(vtable, obj, cmd, flags); + return z_fdtable_call_ioctl((const struct fd_op_vtable *)vtable, + obj, cmd, flags); } #ifdef CONFIG_USERSPACE @@ -1086,7 +1103,8 @@ int z_impl_zsock_poll(struct zsock_pollfd *fds, int nfds, int timeout) continue; } - ctx = z_get_fd_obj_and_vtable(pfd->fd, &vtable); + ctx = get_sock_vtable(pfd->fd, + (const struct socket_op_vtable **)&vtable); if (ctx == NULL) { /* Will set POLLNVAL in return loop */ continue; @@ -1143,7 +1161,8 @@ int z_impl_zsock_poll(struct zsock_pollfd *fds, int nfds, int timeout) continue; } - ctx = z_get_fd_obj_and_vtable(pfd->fd, &vtable); + ctx = get_sock_vtable(pfd->fd, + (const struct socket_op_vtable **)&vtable); if (ctx == NULL) { pfd->revents = ZSOCK_POLLNVAL; ret++; @@ -1498,8 +1517,8 @@ int zsock_getsockname_ctx(struct net_context *ctx, struct sockaddr *addr, int z_impl_zsock_getsockname(int sock, struct sockaddr *addr, socklen_t *addrlen) { - const struct fd_op_vtable *vtable; - void *ctx = z_get_fd_obj_and_vtable(sock, &vtable); + const struct socket_op_vtable *vtable; + void *ctx = get_sock_vtable(sock, &vtable); if (ctx == NULL) { return -1; @@ -1507,8 +1526,8 @@ int z_impl_zsock_getsockname(int sock, struct sockaddr *addr, NET_DBG("getsockname: ctx=%p, fd=%d", ctx, sock); - return z_fdtable_call_ioctl(vtable, ctx, ZFD_IOCTL_GETSOCKNAME, - addr, addrlen); + return z_fdtable_call_ioctl((const struct fd_op_vtable *)vtable, ctx, + ZFD_IOCTL_GETSOCKNAME, addr, addrlen); } #ifdef CONFIG_USERSPACE From 1c24936dcf78022a1a7f035e457716db1d958c79 Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Mon, 27 Jul 2020 13:55:30 +0300 Subject: [PATCH 19/19] scripts: elf_helper: Fix number of parameters on kobject dict The kobject dict has three parameters now after the dynamic object patches. Signed-off-by: Jukka Rissanen --- scripts/elf_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/elf_helper.py b/scripts/elf_helper.py index f5842d2f50f44..6f4892d1d0ac4 100644 --- a/scripts/elf_helper.py +++ b/scripts/elf_helper.py @@ -529,7 +529,7 @@ def find_kobjects(self, syms): if ko.type_obj.api: continue - _, user_ram_allowed = kobjects[ko.type_obj.name] + _, user_ram_allowed, _ = kobjects[ko.type_obj.name] if not user_ram_allowed and app_smem_start <= addr < app_smem_end: self.debug_die(die, "object '%s' found in invalid location %s"