From 5737e3f76339a515c1eb7497e59496e057f21f68 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 18:26:20 -0700 Subject: [PATCH 01/15] 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 697c9c1a25195..2787174b92a83 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -375,7 +375,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 f5c40e578f73e5908d58e51dd3054dc605936d70 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Thu, 28 May 2020 16:24:09 -0700 Subject: [PATCH 02/15] 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 281f072c8858a..98872881c46ab 100644 --- a/kernel/userspace.c +++ b/kernel/userspace.c @@ -785,7 +785,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 */ } @@ -794,7 +794,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 074bdc046dbcf90543f6c3a5c9419f30fab28309 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Thu, 28 May 2020 11:48:54 -0700 Subject: [PATCH 03/15] 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 | 1 + 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, 79 insertions(+), 1 deletion(-) diff --git a/include/syscall_handler.h b/include/syscall_handler.h index 7185a995c4ce9..cd6e0b50c801b 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 a18086f2c89fe..a1288aead24f0 100644 --- a/kernel/thread.c +++ b/kernel/thread.c @@ -466,6 +466,7 @@ void z_setup_new_thread(struct k_thread *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 fcc81577536bb..bd58e15dc0f80 100644 --- a/tests/kernel/mem_protect/syscalls/src/main.c +++ b/tests/kernel/mem_protect/syscalls/src/main.c @@ -337,6 +337,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) @@ -352,7 +390,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 02beeeacf0e9723c214b3a7298ebc2753b40c4fd Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 13:22:20 -0700 Subject: [PATCH 04/15] 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 | 44 +++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/scripts/gen_kobject_list.py b/scripts/gen_kobject_list.py index 1d574f420b95d..bf16135203f24 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -77,25 +77,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)), - ("z_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)), + ("z_thread_stack_element", (None, False, False)), + ("device", (None, False, False)), + ("sys_mutex", (None, True, False)), + ("k_futex", (None, True, False)) ]) def kobject_to_enum(kobj): @@ -609,7 +612,7 @@ def find_kobjects(elf, 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: debug("object '%s' found in invalid location %s" % (ko.type_obj.name, hex(addr))) @@ -853,7 +856,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 @@ -874,7 +877,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 @@ -898,10 +901,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", STACK_TYPE}: + dep, _, alloc = obj_info + + if not alloc: continue if dep: From 522e2c34bb6d65644a2ca110941e1b14cb20e807 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 13:24:51 -0700 Subject: [PATCH 05/15] 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 | 16 +++++------ scripts/gen_kobject_list.py | 2 +- scripts/parse_syscalls.py | 57 ++++++++++++++++++++++++------------- 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6ee341157ce45..02e8ba23d69c8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -495,9 +495,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. @@ -589,20 +589,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 @@ -632,7 +632,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( @@ -646,7 +646,7 @@ add_custom_command( DEPENDS ${ZEPHYR_BASE}/scripts/gen_kobject_list.py ${PARSE_SYSCALLS_TARGET} - ${subsys_json} + ${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 bf16135203f24..54a4f0da0984c 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -918,7 +918,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 41fcb9cd90a1d9e233903f1fbec65688d8cf5548 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 13:30:19 -0700 Subject: [PATCH 06/15] 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 0df20ee2de963..4ec2f9088571d 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 to fail. * Common implementation swallows the message. diff --git a/scripts/gen_kobject_list.py b/scripts/gen_kobject_list.py index 54a4f0da0984c..01574b074d96c 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -80,6 +80,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: @@ -97,6 +101,7 @@ ("k_timer", (None, False, True)), ("z_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)) ]) @@ -118,6 +123,9 @@ def kobject_to_enum(kobj): #}; ] +# 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() @@ -402,6 +410,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 @@ -919,6 +929,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 14ce08a772d44818a538501bdd32c55aa887986e Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 13:33:12 -0700 Subject: [PATCH 07/15] 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/socketpair.c | 2 +- subsys/net/lib/sockets/sockets_net_mgmt.c | 2 +- subsys/net/lib/websocket/websocket_internal.h | 4 +++- 6 files changed, 9 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 02e8ba23d69c8..e0923ed40cf00 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -594,6 +594,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 0ad683741af7b..9d8d39b1ca39d 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/socketpair.c b/subsys/net/lib/sockets/socketpair.c index be648a246016f..0bf9733753225 100644 --- a/subsys/net/lib/sockets/socketpair.c +++ b/subsys/net/lib/sockets/socketpair.c @@ -48,7 +48,7 @@ enum { * - write operations may block if the remote @a recv_q is full * - each endpoint may be blocking or non-blocking */ -struct spair { +__net_socket struct spair { int remote; /**< the remote endpoint file descriptor */ u32_t flags; /**< status and option bits */ struct k_sem sem; /**< semaphore for exclusive structure access */ diff --git a/subsys/net/lib/sockets/sockets_net_mgmt.c b/subsys/net/lib/sockets/sockets_net_mgmt.c index cb0c5383c45b0..97b22df83d1f8 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 150beb3b54489..42a10331723f8 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 9b79f5f8a6d3ee036227aab558efd552122003c2 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Thu, 28 May 2020 14:36:36 -0700 Subject: [PATCH 08/15] 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 4011f6fd2e0c8..1f68b2308d32f 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 2ac6e5b3eb4c5..4fae0a83cc1a9 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 b3c0b78747c5f782041a2d6f962aee26dba5e0f7 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 13:37:49 -0700 Subject: [PATCH 09/15] 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 a6df63d98fac7..2f9e87bc9f115 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 11568ceddc3e9586b61580f29ed0c4aea42efcbb Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 13:57:21 -0700 Subject: [PATCH 10/15] 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 7f41a68f5b166f2fb24c4c72d99327228e4e84f9 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 14:52:26 -0700 Subject: [PATCH 11/15] 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 5d95ae3c80e82b9ac6c0c8b7c9d24431cdcabdf3 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 14:41:13 -0700 Subject: [PATCH 12/15] 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 b652bfe29e870..e549bd73eb45a 100644 --- a/subsys/net/lib/sockets/sockets_can.c +++ b/subsys/net/lib/sockets/sockets_can.c @@ -71,13 +71,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 97b22df83d1f8..d9e0346097c4c 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 369eec019b44f..ae6a0b71d7532 100644 --- a/subsys/net/lib/sockets/sockets_packet.c +++ b/subsys/net/lib/sockets/sockets_packet.c @@ -68,14 +68,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 ecb5d21d71987..67697d31857f9 100644 --- a/subsys/net/lib/sockets/sockets_tls.c +++ b/subsys/net/lib/sockets/sockets_tls.c @@ -1159,13 +1159,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(); @@ -1285,10 +1278,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 4190dec876bae..a0c2b0ff29331 100644 --- a/subsys/net/lib/websocket/websocket.c +++ b/subsys/net/lib/websocket/websocket.c @@ -364,14 +364,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 08172566c96c7ec5bd25d427134d66c8014785c4 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 17:49:02 -0700 Subject: [PATCH 13/15] 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 | 89 ++++++++++++++++++++++++++----------- kernel/userspace_handler.c | 4 -- scripts/gen_kobject_list.py | 3 +- 4 files changed, 95 insertions(+), 30 deletions(-) diff --git a/include/kernel.h b/include/kernel.h index 2787174b92a83..f4a22a26f5adf 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -352,6 +352,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() * @@ -370,6 +391,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 98872881c46ab..04f32398676fc 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 = "?"; @@ -252,51 +255,81 @@ 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_THREAD_STACK_ELEMENT: /* No aligned allocator */ + 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) @@ -508,6 +541,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 dc4602ca9110d..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 01574b074d96c..f404efc332df8 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -79,7 +79,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 6ce48231f35d0ab914aabebe474791b4d8fb0bb5 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 29 May 2020 17:52:35 -0700 Subject: [PATCH 14/15] net: socketpair: fix user mode access The socketpair file descriptor context objects are heap allocated and not drawn from a static pool. Register these as kernel objects when we create them if user mode is enabled. Signed-off-by: Andrew Boie --- subsys/net/lib/sockets/socketpair.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/subsys/net/lib/sockets/socketpair.c b/subsys/net/lib/sockets/socketpair.c index 0bf9733753225..199803e805c82 100644 --- a/subsys/net/lib/sockets/socketpair.c +++ b/subsys/net/lib/sockets/socketpair.c @@ -193,8 +193,11 @@ static void spair_delete(struct spair *spair) /* ensure no private information is released to the memory pool */ memset(spair, 0, sizeof(*spair)); - +#ifdef CONFIG_USERSPACE + k_object_free(spair); +#else k_free(spair); +#endif if (remote != NULL && have_remote_sem) { k_sem_give(&remote->sem); @@ -214,7 +217,18 @@ static struct spair *spair_new(void) { struct spair *spair; +#ifdef CONFIG_USERSPACE + struct z_object *zo = z_dynamic_object_create(sizeof(*spair)); + + if (zo == NULL) { + spair = NULL; + } else { + spair = zo->name; + zo->type = K_OBJ_NET_SOCKET; + } +#else spair = k_malloc(sizeof(*spair)); +#endif if (spair == NULL) { errno = ENOMEM; goto out; From 9c936e606eb2e0bc9984e71e0b832aa0054d88b5 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Thu, 28 May 2020 12:31:11 -0700 Subject: [PATCH 15/15] 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 4fae0a83cc1a9..ad3173f2038bc 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) @@ -140,13 +168,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 @@ -203,9 +224,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 @@ -226,8 +244,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; @@ -237,7 +255,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 @@ -467,9 +486,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)); @@ -1067,15 +1083,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 @@ -1177,7 +1194,8 @@ int z_impl_zsock_poll(struct zsock_pollfd *fds, int nfds, int poll_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; @@ -1243,7 +1261,8 @@ int z_impl_zsock_poll(struct zsock_pollfd *fds, int nfds, int poll_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++; @@ -1600,8 +1619,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; @@ -1609,8 +1628,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