From 7e3d3d782fb7b074d8701c99127a8bcccb8a86af Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Tue, 10 Oct 2017 09:31:32 -0700 Subject: [PATCH] kernel: userspace.c code cleanup - Dumping error messages split from _k_object_validate(), to avoid spam in test cases that are expected to have failure result. - _k_object_find() prototype moved to syscall_handler.h - Clean up k_object_access() implementation to avoid double object lookup and use single validation function - Added comments, minor whitespace changes Signed-off-by: Andrew Boie --- include/kernel.h | 2 + kernel/include/syscall_handler.h | 68 ++++++++++- kernel/userspace.c | 114 ++++++------------ kernel/userspace_handler.c | 36 +++++- .../mem_protect/obj_validation/src/main.c | 5 +- 5 files changed, 141 insertions(+), 84 deletions(-) diff --git a/include/kernel.h b/include/kernel.h index d396acf0437..14eadb40dca 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -131,6 +131,8 @@ struct k_mem_partition; * function in kernel/userspace.c */ enum k_objects { + K_OBJ_ANY, + /* Core kernel objects */ K_OBJ_ALERT, K_OBJ_MSGQ, diff --git a/kernel/include/syscall_handler.h b/kernel/include/syscall_handler.h index a2cea20c694..2a44d654640 100644 --- a/kernel/include/syscall_handler.h +++ b/kernel/include/syscall_handler.h @@ -28,8 +28,9 @@ extern const _k_syscall_handler_t _k_syscall_table[K_SYSCALL_LIMIT]; * call handlers to validate kernel object pointers passed in from * userspace. * - * @param obj Address of the kernel object - * @param otype Expected type of the kernel object + * @param ko Kernel object metadata pointer, or NULL + * @param otype Expected type of the kernel object, or K_OBJ_ANY if type + * doesn't matter * @param init If true, this is for an init function and we will not error * out if the object is not initialized * @return 0 If the object is valid @@ -37,7 +38,45 @@ extern const _k_syscall_handler_t _k_syscall_table[K_SYSCALL_LIMIT]; * -EPERM If the caller does not have permissions * -EINVAL Object is not initialized */ -int _k_object_validate(void *obj, enum k_objects otype, int init); +int _k_object_validate(struct _k_object *ko, enum k_objects otype, int init); + +/** + * Dump out error information on failed _k_object_validate() call + * + * @param retval Return value from _k_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 otype Expected type of the kernel object + */ +extern void _dump_object_error(int retval, void *obj, struct _k_object *ko, + enum k_objects otype); + +/** + * Kernel object validation function + * + * Retrieve metadata for a kernel object. This function is implemented in + * the gperf script footer, see gen_kobject_list.py + * + * @param obj Address of kernel object to get metadata + * @return Kernel object's metadata, or NULL if the parameter wasn't the + * memory address of a kernel object + */ +extern struct _k_object *_k_object_find(void *obj); + +/** + * Grant a thread permission to a kernel object + * + * @param ko Kernel object metadata to update + * @param thread The thread to grant permission + */ +extern void _thread_perms_set(struct _k_object *ko, struct k_thread *thread); + +/** + * Grant all current and future threads access to a kernel object + * + * @param ko Kernel object metadata to update + */ +extern void _thread_perms_all_set(struct _k_object *ko); /** * @brief Runtime expression check for system call arguments @@ -157,9 +196,28 @@ int _k_object_validate(void *obj, enum k_objects otype, int init); #define _SYSCALL_MEMORY_ARRAY_WRITE(ptr, nmemb, size, ssf) \ _SYSCALL_MEMORY_ARRAY(ptr, nmemb, size, 1, ssf) +static inline int _obj_validation_check(void *obj, enum k_objects otype, + int init) +{ + struct _k_object *ko; + int ret; + + ko = _k_object_find(obj); + ret = _k_object_validate(ko, otype, init); + +#ifdef CONFIG_PRINTK + if (ret) { + _dump_object_error(ret, obj, ko, otype); + } +#endif + + return ret; +} + + #define _SYSCALL_IS_OBJ(ptr, type, init, ssf) \ - _SYSCALL_VERIFY_MSG(!_k_object_validate((void *)ptr, type, init), ssf, \ - "object %p access denied", (void *)(ptr)) + _SYSCALL_VERIFY_MSG(!_obj_validation_check((void *)ptr, type, init), \ + ssf, "object %p access denied", (void *)(ptr)) /** * @brief Runtime check kernel object pointer for non-init functions diff --git a/kernel/userspace.c b/kernel/userspace.c index e0e9a972a7d..a8c6ef7b962 100644 --- a/kernel/userspace.c +++ b/kernel/userspace.c @@ -12,18 +12,7 @@ #include #include #include - -/** - * Kernel object validation function - * - * Retrieve metadata for a kernel object. This function is implemented in - * the gperf script footer, see gen_kobject_list.py - * - * @param obj Address of kernel object to get metadata - * @return Kernel object's metadata, or NULL if the parameter wasn't the - * memory address of a kernel object - */ -extern struct _k_object *_k_object_find(void *obj); +#include const char *otype_to_str(enum k_objects otype) { @@ -103,9 +92,7 @@ const char *otype_to_str(enum k_objects otype) #endif } -/* Stub functions, to be filled in forthcoming patch sets */ - -static void set_thread_perms(struct _k_object *ko, struct k_thread *thread) +void _thread_perms_set(struct _k_object *ko, struct k_thread *thread) { if (thread->base.perm_index < 8 * CONFIG_MAX_THREAD_BYTES) { sys_bitfield_set_bit((mem_addr_t)&ko->perms, @@ -113,7 +100,7 @@ static void set_thread_perms(struct _k_object *ko, struct k_thread *thread) } } -static int test_thread_perms(struct _k_object *ko) +static int thread_perms_test(struct _k_object *ko) { if (_current->base.perm_index < 8 * CONFIG_MAX_THREAD_BYTES) { return sys_bitfield_test_bit((mem_addr_t)&ko->perms, @@ -122,89 +109,66 @@ static int test_thread_perms(struct _k_object *ko) return 0; } -/** - * Kernek object permission modification check - * - * Check that the caller has sufficient perms to modify access permissions for - * a particular kernel object. oops() if a user thread is trying to something - * forbidden. - * - * @param object to be modified - * @return NULL if the caller is a kernel thread and the object was not found - */ -static struct _k_object *access_check(void *object) +void _thread_perms_all_set(struct _k_object *ko) { - struct _k_object *ko = _k_object_find(object); + memset(ko->perms, 0xFF, CONFIG_MAX_THREAD_BYTES); +} - if (!ko) { - if (_is_thread_user()) { - printk("granting access to non-existent kernel object %p\n", - object); - k_oops(); - } else { - /* Supervisor threads may at times instantiate objects - * that ignore rules on where they can live. Such - * objects won't ever be usable from userspace, but - * we shouldn't explode. - */ - return NULL; - } +static void dump_permission_error(struct _k_object *ko) +{ + printk("thread %p (%d) does not have permission on %s %p [", + _current, _current->base.perm_index, + otype_to_str(ko->type), ko->name); + for (int i = CONFIG_MAX_THREAD_BYTES - 1; i >= 0; i--) { + printk("%02x", ko->perms[i]); } + printk("]\n"); +} - /* userspace can't grant access to objects unless it already has - * access to that object - */ - if (_is_thread_user() && !test_thread_perms(ko)) { - printk("insufficient permissions in current thread %p\n", - _current); - printk("Cannot grant access to %s %p\n", - otype_to_str(ko->type), object); - k_oops(); +void _dump_object_error(int retval, void *obj, struct _k_object *ko, + enum k_objects otype) +{ + switch (retval) { + case -EBADF: + printk("%p is not a valid %s\n", obj, otype_to_str(otype)); + break; + case -EPERM: + dump_permission_error(ko); + break; + case -EINVAL: + printk("%p used before initialization\n", obj); + break; } - - return ko; } void _impl_k_object_access_grant(void *object, struct k_thread *thread) { - struct _k_object *ko = access_check(object); + struct _k_object *ko = _k_object_find(object); if (ko) { - set_thread_perms(ko, thread); + _thread_perms_set(ko, thread); } } void _impl_k_object_access_all_grant(void *object) { - struct _k_object *ko = access_check(object); + struct _k_object *ko = _k_object_find(object); if (ko) { - memset(ko->perms, 0xFF, CONFIG_MAX_THREAD_BYTES); + _thread_perms_all_set(ko); } } -int _k_object_validate(void *obj, enum k_objects otype, int init) +int _k_object_validate(struct _k_object *ko, enum k_objects otype, int init) { - struct _k_object *ko; - - ko = _k_object_find(obj); - - if (!ko || ko->type != otype) { - printk("%p is not a %s\n", obj, otype_to_str(otype)); + if (!ko || (otype != K_OBJ_ANY && ko->type != otype)) { return -EBADF; } /* Manipulation of any kernel objects by a user thread requires that * thread be granted access first, even for uninitialized objects */ - if (_is_thread_user() && !test_thread_perms(ko)) { - printk("thread %p (%d) does not have permission on %s %p [", - _current, _current->base.perm_index, otype_to_str(otype), - obj); - for (int i = CONFIG_MAX_THREAD_BYTES - 1; i >= 0; i--) { - printk("%02x", ko->perms[i]); - } - printk("]\n"); + if (!thread_perms_test(ko)) { return -EPERM; } @@ -212,14 +176,12 @@ int _k_object_validate(void *obj, enum k_objects otype, int init) * initialized, we should freak out */ if (!init && !(ko->flags & K_OBJ_FLAG_INITIALIZED)) { - printk("%p used before initialization\n", obj); return -EINVAL; } return 0; } - void _k_object_init(void *object) { struct _k_object *ko; @@ -241,9 +203,13 @@ void _k_object_init(void *object) return; } + /* Initializing an object implicitly grants access to the calling + * thread and nobody else + */ memset(ko->perms, 0, CONFIG_MAX_THREAD_BYTES); - set_thread_perms(ko, _current); + _thread_perms_set(ko, _current); + /* Allows non-initialization system calls to be made on this object */ ko->flags |= K_OBJ_FLAG_INITIALIZED; } diff --git a/kernel/userspace_handler.c b/kernel/userspace_handler.c index 0dbd8601327..562f41b1f70 100644 --- a/kernel/userspace_handler.c +++ b/kernel/userspace_handler.c @@ -7,9 +7,33 @@ #include #include +static struct _k_object *validate_any_object(void *obj) +{ + struct _k_object *ko; + int ret; + + ko = _k_object_find(obj); + + /* This can be any kernel object and it doesn't have to be + * initialized + */ + ret = _k_object_validate(ko, K_OBJ_ANY, 1); + if (ret) { +#ifdef CONFIG_PRINTK + _dump_object_error(ret, obj, ko, K_OBJ_ANY); +#endif + return NULL; + } + + return ko; +} + /* Normally these would be included in userspace.c, but the way * syscall_dispatch.c declares weak handlers results in build errors if these * are located in userspace.c. Just put in a separate file. + * + * To avoid double _k_object_find() lookups, we don't call the implementation + * function, but call a level deeper. */ u32_t _handler_k_object_access_grant(u32_t object, u32_t thread, u32_t arg3, @@ -17,9 +41,13 @@ u32_t _handler_k_object_access_grant(u32_t object, u32_t thread, u32_t arg3, void *ssf) { _SYSCALL_ARG2; + struct _k_object *ko; _SYSCALL_OBJ(thread, K_OBJ_THREAD, ssf); - _impl_k_object_access_grant((void *)object, (struct k_thread *)thread); + ko = validate_any_object((void *)object); + _SYSCALL_VERIFY_MSG(ko, ssf, "object %p access denied", (void *)object); + _thread_perms_set(ko, (struct k_thread *)thread); + return 0; } @@ -28,7 +56,11 @@ u32_t _handler_k_object_access_all_grant(u32_t object, u32_t arg2, u32_t arg3, void *ssf) { _SYSCALL_ARG1; + struct _k_object *ko; + + ko = validate_any_object((void *)object); + _SYSCALL_VERIFY_MSG(ko, ssf, "object %p access denied", (void *)object); + _thread_perms_all_set(ko); - _impl_k_object_access_all_grant((void *)object); return 0; } diff --git a/tests/kernel/mem_protect/obj_validation/src/main.c b/tests/kernel/mem_protect/obj_validation/src/main.c index 78e23369e20..d578f99c405 100644 --- a/tests/kernel/mem_protect/obj_validation/src/main.c +++ b/tests/kernel/mem_protect/obj_validation/src/main.c @@ -20,13 +20,11 @@ static __kernel char bad_sem[sizeof(struct k_sem)]; static struct k_sem sem3; #endif - static int test_bad_object(struct k_sem *sem) { - return !_k_object_validate(sem, K_OBJ_SEM, 0); + return !_k_object_validate(_k_object_find(sem), K_OBJ_SEM, 0); } - void main(void) { struct k_sem stack_sem; int i, rv; @@ -44,6 +42,7 @@ void main(void) { #endif /* sem1 should already be ready to go */ + k_object_access_grant(&sem1, k_current_get()); rv |= !test_bad_object(&sem1); /* sem2 has to be initialized at runtime */