From e9cfc54d00bb469ec7eefd43ffaec410446a094e Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 13 Apr 2018 13:15:28 -0700 Subject: [PATCH] kernel: remove k_object_access_revoke() as syscall Forthcoming patches will dual-purpose an object's permission bitfield as also reference tracking for kernel objects, used to handle automatic freeing of resources. We do not want to allow user thread A to revoke thread B's access to some object O if B is in the middle of an API call using O. However we do want to allow threads to revoke their own access to an object, so introduce a new API and syscall for that. Signed-off-by: Andrew Boie --- doc/kernel/usermode/kernelobjects.rst | 7 ++-- include/kernel.h | 17 ++++++++-- kernel/userspace.c | 7 +++- kernel/userspace_handler.c | 6 ++-- .../mem_protect/mem_protect/src/kobject.c | 4 +-- .../kernel/mem_protect/mem_protect/src/main.c | 4 +-- tests/kernel/mem_protect/userspace/src/main.c | 34 ++----------------- 7 files changed, 33 insertions(+), 46 deletions(-) diff --git a/doc/kernel/usermode/kernelobjects.rst b/doc/kernel/usermode/kernelobjects.rst index efd45196145..3786e3cde78 100644 --- a/doc/kernel/usermode/kernelobjects.rst +++ b/doc/kernel/usermode/kernelobjects.rst @@ -152,9 +152,10 @@ ways to do this. access to a set of kernel objects at boot time. Once a thread has been granted access to an object, such access may be -removed with the :c:func:`k_object_access_revoke()` API. User threads using -this API must have permission on both the object in question, and the thread -object that is having access revoked. +removed with the :c:func:`k_object_access_revoke()` API. This API is not +available to user threads, however user threads may use +:c:func:`k_object_release()` to relinquish their own permissions on an +object. API calls from supervisor mode to set permissions on kernel objects that are not being tracked by the kernel will be no-ops. Doing the same from user mode diff --git a/include/kernel.h b/include/kernel.h index 27fa8ea5da9..aba1d7b9553 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -223,13 +223,21 @@ static inline void _impl_k_object_access_grant(void *object, /** * @internal */ -static inline void _impl_k_object_access_revoke(void *object, - struct k_thread *thread) +static inline void k_object_access_revoke(void *object, + struct k_thread *thread) { ARG_UNUSED(object); ARG_UNUSED(thread); } +/** + * @internal + */ +static inline void _impl_k_object_release(void *object) +{ + ARG_UNUSED(object); +} + static inline void k_object_access_all_grant(void *object) { ARG_UNUSED(object); @@ -258,7 +266,10 @@ __syscall void k_object_access_grant(void *object, struct k_thread *thread); * @param object Address of kernel object * @param thread Thread to remove access to the object */ -__syscall void k_object_access_revoke(void *object, struct k_thread *thread); +void k_object_access_revoke(void *object, struct k_thread *thread); + + +__syscall void k_object_release(void *object); /** * grant all present and future threads access to an object diff --git a/kernel/userspace.c b/kernel/userspace.c index af78e3f58a8..7f6d3683a7a 100644 --- a/kernel/userspace.c +++ b/kernel/userspace.c @@ -350,7 +350,7 @@ void _impl_k_object_access_grant(void *object, struct k_thread *thread) } } -void _impl_k_object_access_revoke(void *object, struct k_thread *thread) +void k_object_access_revoke(void *object, struct k_thread *thread) { struct _k_object *ko = _k_object_find(object); @@ -359,6 +359,11 @@ void _impl_k_object_access_revoke(void *object, struct k_thread *thread) } } +void _impl_k_object_release(void *object) +{ + k_object_access_revoke(object, _current); +} + void k_object_access_all_grant(void *object) { struct _k_object *ko = _k_object_find(object); diff --git a/kernel/userspace_handler.c b/kernel/userspace_handler.c index f78e17f78ec..6b45ec4e7af 100644 --- a/kernel/userspace_handler.c +++ b/kernel/userspace_handler.c @@ -6,6 +6,7 @@ #include #include +#include static struct _k_object *validate_any_object(void *obj) { @@ -47,14 +48,13 @@ _SYSCALL_HANDLER(k_object_access_grant, object, thread) return 0; } -_SYSCALL_HANDLER(k_object_access_revoke, object, thread) +_SYSCALL_HANDLER(k_object_release, object) { struct _k_object *ko; - _SYSCALL_OBJ_INIT(thread, K_OBJ_THREAD); ko = validate_any_object((void *)object); _SYSCALL_VERIFY_MSG(ko, "object %p access denied", (void *)object); - _thread_perms_clear(ko, (struct k_thread *)thread); + _thread_perms_clear(ko, _current); return 0; } diff --git a/tests/kernel/mem_protect/mem_protect/src/kobject.c b/tests/kernel/mem_protect/mem_protect/src/kobject.c index 4bc00b0dbcc..77ff268bf1c 100644 --- a/tests/kernel/mem_protect/mem_protect/src/kobject.c +++ b/tests/kernel/mem_protect/mem_protect/src/kobject.c @@ -238,7 +238,7 @@ void kobject_user_test7(void *p1, void *p2, void *p3) USERSPACE_BARRIER; k_sem_give(&kobject_sem); - k_object_access_revoke(&kobject_sem, k_current_get()); + k_object_release(&kobject_sem); valid_fault = true; USERSPACE_BARRIER; @@ -247,7 +247,7 @@ void kobject_user_test7(void *p1, void *p2, void *p3) } /* Test revoke permission of a k object from userspace. */ -void test_kobject_revoke_access_from_user(void *p1, void *p2, void *p3) +void test_kobject_release_from_user(void *p1, void *p2, void *p3) { k_thread_access_grant(k_current_get(), &kobject_sem, NULL); diff --git a/tests/kernel/mem_protect/mem_protect/src/main.c b/tests/kernel/mem_protect/mem_protect/src/main.c index 1527a5a3b40..c2be04defaa 100644 --- a/tests/kernel/mem_protect/mem_protect/src/main.c +++ b/tests/kernel/mem_protect/mem_protect/src/main.c @@ -32,7 +32,7 @@ extern void test_thread_without_kobject_permission(void); extern void test_kobject_revoke_access(void); extern void test_kobject_grant_access_kobj(void); extern void test_kobject_grant_access_kobj_invalid(void); -extern void test_kobject_revoke_access_from_user(void); +extern void test_kobject_release_from_user(void); extern void test_kobject_access_all_grant(void); extern void test_thread_has_residual_permissions(void); extern void test_kobject_access_grant_to_invalid_thread(void); @@ -73,7 +73,7 @@ void test_main(void) ztest_unit_test(test_kobject_revoke_access), ztest_unit_test(test_kobject_grant_access_kobj), ztest_unit_test(test_kobject_grant_access_kobj_invalid), - ztest_unit_test(test_kobject_revoke_access_from_user), + ztest_unit_test(test_kobject_release_from_user), ztest_unit_test(test_kobject_access_all_grant), ztest_unit_test(test_thread_has_residual_permissions), ztest_unit_test(test_kobject_access_grant_to_invalid_thread), diff --git a/tests/kernel/mem_protect/userspace/src/main.c b/tests/kernel/mem_protect/userspace/src/main.c index 40151870ad5..026a4477368 100644 --- a/tests/kernel/mem_protect/userspace/src/main.c +++ b/tests/kernel/mem_protect/userspace/src/main.c @@ -418,7 +418,7 @@ static void revoke_noperms_object(void) expect_fault = true; expected_reason = REASON_KERNEL_OOPS; BARRIER(); - k_object_access_revoke(&ksem, k_current_get()); + k_object_release(&ksem); zassert_unreachable("Revoke access to unauthorized object " "did not fault\n"); @@ -426,7 +426,7 @@ static void revoke_noperms_object(void) static void access_after_revoke(void) { - k_object_access_revoke(&test_revoke_sem, k_current_get()); + k_object_release(&test_revoke_sem); /* Try to access an object after revoking access to it */ expect_fault = true; @@ -437,35 +437,6 @@ static void access_after_revoke(void) zassert_unreachable("Using revoked object did not fault\n"); } -static void revoke_from_parent(k_tid_t parentThread) -{ - /* The following should cause a fault */ - expect_fault = true; - expected_reason = REASON_KERNEL_OOPS; - BARRIER(); - k_object_access_revoke(&test_revoke_sem, parentThread); - - zassert_unreachable("Revoking from unauthorized thread did " - "not fault\n"); -} - -static void revoke_other_thread(void) -{ - /* Create user mode thread */ - k_thread_create(&uthread_thread, uthread_stack, STACKSIZE, - (k_thread_entry_t)revoke_from_parent, k_current_get(), - NULL, NULL, -1, K_USER | K_INHERIT_PERMS, K_NO_WAIT); - - /* - * Abort ztest thread so that it does not return to the caller - * and incorrectly signal a passing test. The thread created above - * will handle calling ztest_test_pass() or ztest_test_fail() - * to complete the test, either directly or from - * _SysFatalErrorHandler(). - */ - k_thread_abort(k_current_get()); -} - static void umode_enter_func(void) { if (_is_user_context()) { @@ -560,7 +531,6 @@ void test_main(void) ztest_user_unit_test(write_other_stack), ztest_user_unit_test(revoke_noperms_object), ztest_user_unit_test(access_after_revoke), - ztest_user_unit_test(revoke_other_thread), ztest_unit_test(user_mode_enter), ztest_user_unit_test(write_kobject_user_pipe), ztest_user_unit_test(read_kobject_user_pipe)