kernel: fix CONFIG_THREAD_NAME from user mode.
This mechanism had multiple problems: - Missing parameter documentation strings. - Multiple calls to k_thread_name_set() from user mode would leak memory, since the copied string was never freed - k_thread_name_get() returns memory to user mode with no guarantees on whether user mode can actually read it; in the case where the string was in thread resource pool memory (which happens when k_thread_name_set() is called from user mode) it would never be readable. - There was no test case coverage for these functions from user mode. To properly fix this, thread objects now have a buffer region reserved specifically for the thread name. Setting the thread name copies the string into the buffer. Getting the thread name with k_thread_name_get() still returns a pointer, but the system call has been removed. A new API k_thread_name_copy() is introduced to copy the thread name into a destination buffer, and a system call has been provided for that instead. We now have full test case coverge for these APIs in both user and supervisor mode. Some of the code has been cleaned up to place system call handler functions in proximity with their implementations. Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
This commit is contained in:
Andrew Boie
Andrew Boie
parent
1ee017050b
commit
38129ce1a6
|
|
@ -538,7 +538,7 @@ struct k_thread {
|
|||
|
||||
#if defined(CONFIG_THREAD_NAME)
|
||||
/* Thread name */
|
||||
const char *name;
|
||||
char name[CONFIG_THREAD_MAX_NAME_LEN];
|
||||
#endif
|
||||
|
||||
#ifdef CONFIG_THREAD_CUSTOM_DATA
|
||||
|
|
@ -1304,8 +1304,14 @@ __syscall void *k_thread_custom_data_get(void);
|
|||
* Set the name of the thread to be used when THREAD_MONITOR is enabled for
|
||||
* tracing and debugging.
|
||||
*
|
||||
* @param thread_id Thread to set name, or NULL to set the current thread
|
||||
* @param value Name string
|
||||
* @retval 0 on success
|
||||
* @retval -EFAULT Memory access error with supplied string
|
||||
* @retval -ENOSYS Thread name configuration option not enabled
|
||||
* @retval -EINVAL Thread name too long
|
||||
*/
|
||||
__syscall void k_thread_name_set(k_tid_t thread_id, const char *value);
|
||||
__syscall int k_thread_name_set(k_tid_t thread_id, const char *value);
|
||||
|
||||
/**
|
||||
* @brief Get thread name
|
||||
|
|
@ -1313,9 +1319,23 @@ __syscall void k_thread_name_set(k_tid_t thread_id, const char *value);
|
|||
* Get the name of a thread
|
||||
*
|
||||
* @param thread_id Thread ID
|
||||
*
|
||||
* @retval Thread name, or NULL if configuration not enabled
|
||||
*/
|
||||
__syscall const char *k_thread_name_get(k_tid_t thread_id);
|
||||
const char *k_thread_name_get(k_tid_t thread_id);
|
||||
|
||||
/**
|
||||
* @brief Copy the thread name into a supplied buffer
|
||||
*
|
||||
* @param thread_id Thread to obtain name information
|
||||
* @param buf Destination buffer
|
||||
* @param size Destinatiomn buffer size
|
||||
* @retval -ENOSPC Destination buffer too small
|
||||
* @retval -EFAULT Memory access error
|
||||
* @retval -ENOSYS Thread name feature not enabled
|
||||
* @retval 0 Success
|
||||
*/
|
||||
__syscall int k_thread_name_copy(k_tid_t thread_id, char *buf,
|
||||
size_t size);
|
||||
|
||||
/**
|
||||
* @}
|
||||
|
|
|
|||
|
|
@ -351,6 +351,16 @@ config THREAD_NAME
|
|||
bool "Thread name [EXPERIMENTAL]"
|
||||
help
|
||||
This option allows to set a name for a thread.
|
||||
|
||||
config THREAD_MAX_NAME_LEN
|
||||
int "Max length of a thread name"
|
||||
default 32
|
||||
range 8 128
|
||||
depends on THREAD_NAME
|
||||
help
|
||||
Thread names get stored in the k_thread struct. Indicate the max
|
||||
name length, including the terminating NULL byte. Reduce this value
|
||||
to conserve memory.
|
||||
endmenu
|
||||
|
||||
menu "Work Queue Options"
|
||||
|
|
|
|||
|
|
@ -241,7 +241,7 @@ static ALWAYS_INLINE void z_new_thread_init(struct k_thread *thread,
|
|||
#endif
|
||||
|
||||
#ifdef CONFIG_THREAD_NAME
|
||||
thread->name = NULL;
|
||||
thread->name[0] = '\0';
|
||||
#endif
|
||||
|
||||
#if defined(CONFIG_USERSPACE)
|
||||
|
|
|
|||
140
kernel/thread.c
140
kernel/thread.c
|
|
@ -133,11 +133,22 @@ void z_impl_k_thread_custom_data_set(void *value)
|
|||
_current->custom_data = value;
|
||||
}
|
||||
|
||||
#ifdef CONFIG_USERSPACE
|
||||
Z_SYSCALL_HANDLER(k_thread_custom_data_set, data)
|
||||
{
|
||||
z_impl_k_thread_custom_data_set((void *)data);
|
||||
return 0;
|
||||
}
|
||||
#endif
|
||||
|
||||
void *z_impl_k_thread_custom_data_get(void)
|
||||
{
|
||||
return _current->custom_data;
|
||||
}
|
||||
|
||||
#ifdef CONFIG_USERSPACE
|
||||
Z_SYSCALL_HANDLER0_SIMPLE(k_thread_custom_data_get);
|
||||
#endif /* CONFIG_USERSPACE */
|
||||
#endif /* CONFIG_THREAD_CUSTOM_DATA */
|
||||
|
||||
#if defined(CONFIG_THREAD_MONITOR)
|
||||
|
|
@ -167,61 +178,109 @@ void z_thread_monitor_exit(struct k_thread *thread)
|
|||
}
|
||||
#endif
|
||||
|
||||
int z_impl_k_thread_name_set(struct k_thread *thread, const char *value)
|
||||
{
|
||||
#ifdef CONFIG_THREAD_NAME
|
||||
void z_impl_k_thread_name_set(struct k_thread *thread, const char *value)
|
||||
{
|
||||
if (thread == NULL) {
|
||||
_current->name = value;
|
||||
} else {
|
||||
thread->name = value;
|
||||
thread = _current;
|
||||
}
|
||||
}
|
||||
|
||||
const char *z_impl_k_thread_name_get(struct k_thread *thread)
|
||||
{
|
||||
return (const char *)thread->name;
|
||||
}
|
||||
|
||||
strncpy(thread->name, value, CONFIG_THREAD_MAX_NAME_LEN);
|
||||
thread->name[CONFIG_THREAD_MAX_NAME_LEN - 1] = '\0';
|
||||
return 0;
|
||||
#else
|
||||
void z_impl_k_thread_name_set(k_tid_t thread_id, const char *value)
|
||||
{
|
||||
ARG_UNUSED(thread_id);
|
||||
ARG_UNUSED(thread);
|
||||
ARG_UNUSED(value);
|
||||
}
|
||||
|
||||
const char *z_impl_k_thread_name_get(k_tid_t thread_id)
|
||||
{
|
||||
ARG_UNUSED(thread_id);
|
||||
return NULL;
|
||||
}
|
||||
return -ENOSYS;
|
||||
#endif /* CONFIG_THREAD_NAME */
|
||||
}
|
||||
|
||||
#ifdef CONFIG_USERSPACE
|
||||
|
||||
#if defined(CONFIG_THREAD_NAME)
|
||||
Z_SYSCALL_HANDLER(k_thread_name_set, thread, data)
|
||||
Z_SYSCALL_HANDLER(k_thread_name_set, thread, str_param)
|
||||
{
|
||||
char *name_copy = NULL;
|
||||
#ifdef CONFIG_THREAD_NAME
|
||||
struct k_thread *t = (struct k_thread *)thread;
|
||||
size_t len;
|
||||
int err;
|
||||
const char *str = (const char *)str_param;
|
||||
|
||||
name_copy = z_user_string_alloc_copy((char *)data, 64);
|
||||
z_impl_k_thread_name_set((struct k_thread *)thread, name_copy);
|
||||
return 0;
|
||||
if (t != NULL) {
|
||||
if (Z_SYSCALL_OBJ(t, K_OBJ_THREAD) != 0) {
|
||||
return -EINVAL;
|
||||
}
|
||||
}
|
||||
|
||||
len = z_user_string_nlen(str, CONFIG_THREAD_MAX_NAME_LEN, &err);
|
||||
if (err != 0) {
|
||||
return -EFAULT;
|
||||
}
|
||||
if (Z_SYSCALL_MEMORY_READ(str, len) != 0) {
|
||||
return -EFAULT;
|
||||
}
|
||||
|
||||
return z_impl_k_thread_name_set(t, str);
|
||||
#else
|
||||
return -ENOSYS;
|
||||
#endif /* CONFIG_THREAD_NAME */
|
||||
}
|
||||
#endif /* CONFIG_USERSPACE */
|
||||
|
||||
const char *k_thread_name_get(struct k_thread *thread)
|
||||
{
|
||||
#ifdef CONFIG_THREAD_NAME
|
||||
return (const char *)thread->name;
|
||||
#else
|
||||
ARG_UNUSED(thread);
|
||||
return NULL;
|
||||
#endif /* CONFIG_THREAD_NAME */
|
||||
}
|
||||
|
||||
Z_SYSCALL_HANDLER1_SIMPLE(k_thread_name_get, K_OBJ_THREAD, k_tid_t);
|
||||
#endif
|
||||
|
||||
#ifdef CONFIG_THREAD_CUSTOM_DATA
|
||||
Z_SYSCALL_HANDLER(k_thread_custom_data_set, data)
|
||||
int z_impl_k_thread_name_copy(k_tid_t thread_id, char *buf, size_t size)
|
||||
{
|
||||
z_impl_k_thread_custom_data_set((void *)data);
|
||||
#ifdef CONFIG_THREAD_NAME
|
||||
strncpy(buf, thread_id->name, size);
|
||||
return 0;
|
||||
#else
|
||||
ARG_UNUSED(thread_id);
|
||||
ARG_UNUSED(buf);
|
||||
ARG_UNUSED(size);
|
||||
return -ENOSYS;
|
||||
#endif /* CONFIG_THREAD_NAME */
|
||||
}
|
||||
|
||||
Z_SYSCALL_HANDLER0_SIMPLE(k_thread_custom_data_get);
|
||||
#endif /* CONFIG_THREAD_CUSTOM_DATA */
|
||||
#ifdef CONFIG_USERSPACE
|
||||
Z_SYSCALL_HANDLER(k_thread_name_copy, thread_id, buf, size)
|
||||
{
|
||||
#ifdef CONFIG_THREAD_NAME
|
||||
size_t len;
|
||||
struct k_thread *t = (struct k_thread *)thread_id;
|
||||
struct _k_object *ko = z_object_find(t);
|
||||
|
||||
/* Special case: we allow reading the names of initialized threads
|
||||
* even if we don't have permission on them
|
||||
*/
|
||||
if (t == NULL || ko->type != K_OBJ_THREAD ||
|
||||
(ko->flags & K_OBJ_FLAG_INITIALIZED) == 0) {
|
||||
return -EINVAL;
|
||||
}
|
||||
if (Z_SYSCALL_MEMORY_WRITE(buf, size) != 0) {
|
||||
return -EFAULT;
|
||||
}
|
||||
len = strlen(t->name);
|
||||
if (len + 1 > size) {
|
||||
return -ENOSPC;
|
||||
}
|
||||
|
||||
return z_user_to_copy((void *)buf, t->name, len + 1);
|
||||
#else
|
||||
ARG_UNUSED(thread_id);
|
||||
ARG_UNUSED(buf);
|
||||
ARG_UNUSED(size);
|
||||
return -ENOSYS;
|
||||
#endif /* CONFIG_THREAD_NAME */
|
||||
}
|
||||
#endif /* CONFIG_USERSPACE */
|
||||
|
||||
#endif
|
||||
|
||||
#ifdef CONFIG_STACK_SENTINEL
|
||||
/* Check that the stack sentinel is still present
|
||||
|
|
@ -381,7 +440,12 @@ void z_setup_new_thread(struct k_thread *new_thread,
|
|||
k_spin_unlock(&lock, key);
|
||||
#endif
|
||||
#ifdef CONFIG_THREAD_NAME
|
||||
new_thread->name = name;
|
||||
if (name != NULL) {
|
||||
strncpy(new_thread->name, name,
|
||||
CONFIG_THREAD_MAX_NAME_LEN - 1);
|
||||
/* Ensure NULL termination, truncate if longer */
|
||||
new_thread->name[CONFIG_THREAD_MAX_NAME_LEN - 1] = '\0';
|
||||
}
|
||||
#endif
|
||||
#ifdef CONFIG_USERSPACE
|
||||
z_object_init(new_thread);
|
||||
|
|
|
|||
|
|
@ -70,10 +70,6 @@ void test_systhreads_idle(void)
|
|||
K_IDLE_PRIO, NULL);
|
||||
}
|
||||
|
||||
static void thread_name_entry(void)
|
||||
{
|
||||
}
|
||||
|
||||
static void customdata_entry(void *p1, void *p2, void *p3)
|
||||
{
|
||||
long data = 1U;
|
||||
|
|
@ -107,41 +103,115 @@ void test_customdata_get_set_coop(void)
|
|||
k_thread_abort(tid);
|
||||
}
|
||||
|
||||
static void thread_name_entry(void *p1, void *p2, void *p3)
|
||||
{
|
||||
/* Do nothing and exit */
|
||||
}
|
||||
|
||||
/**
|
||||
* @ingroup kernel_thread_tests
|
||||
* @brief test thread name get/set from preempt thread
|
||||
* @see k_thread_name_get(), k_thread_name_set()
|
||||
*/
|
||||
* @ingroup kernel_thread_tests
|
||||
* @brief test thread name get/set from supervisor thread
|
||||
* @see k_thread_name_get(), k_thread_name_copy(), k_thread_name_set()
|
||||
*/
|
||||
void test_thread_name_get_set(void)
|
||||
{
|
||||
int ret;
|
||||
const char *thread_name;
|
||||
char thread_buf[CONFIG_THREAD_MAX_NAME_LEN];
|
||||
|
||||
/* Set and get current thread's name */
|
||||
k_thread_name_set(NULL, "parent_thread");
|
||||
ret = k_thread_name_set(NULL, "parent_thread");
|
||||
zassert_equal(ret, 0, "k_thread_name_set() failed");
|
||||
thread_name = k_thread_name_get(k_current_get());
|
||||
|
||||
zassert_true(thread_name != NULL, "thread name was null");
|
||||
ret = strcmp(thread_name, "parent_thread");
|
||||
zassert_equal(ret, 0, "parent thread name does not match");
|
||||
|
||||
/* Set and get child thread's name */
|
||||
k_tid_t tid = k_thread_create(&tdata_name, tstack_name, STACK_SIZE,
|
||||
(k_thread_entry_t)thread_name_entry,
|
||||
NULL, NULL, NULL,
|
||||
K_PRIO_COOP(1), 0, 0);
|
||||
thread_name_entry, NULL, NULL, NULL,
|
||||
K_PRIO_PREEMPT(1), 0, 0);
|
||||
|
||||
k_thread_name_set(tid, "customdata");
|
||||
ret = k_thread_name_set(tid, "customdata");
|
||||
zassert_equal(ret, 0, "k_thread_name_set() failed");
|
||||
ret = k_thread_name_copy(tid, thread_buf, sizeof(thread_buf));
|
||||
zassert_equal(ret, 0, "couldn't get copied thread name");
|
||||
ret = strcmp(thread_buf, "customdata");
|
||||
zassert_equal(ret, 0, "child thread name does not match");
|
||||
|
||||
k_sleep(500);
|
||||
/* cleanup environment */
|
||||
k_thread_abort(tid);
|
||||
}
|
||||
|
||||
thread_name = k_thread_name_get(tid);
|
||||
#ifdef CONFIG_USERSPACE
|
||||
static char unreadable_string[64];
|
||||
static char not_my_buffer[CONFIG_THREAD_MAX_NAME_LEN];
|
||||
ZTEST_BMEM struct k_thread *main_thread_ptr;
|
||||
struct k_sem sem;
|
||||
#endif /* CONFIG_USERSPACE */
|
||||
|
||||
/**
|
||||
* @ingroup kernel_thread_tests
|
||||
* @brief test thread name get/set from user thread
|
||||
* @see k_thread_name_copy(), k_thread_name_set()
|
||||
*/
|
||||
void test_thread_name_user_get_set(void)
|
||||
{
|
||||
#ifdef CONFIG_USERSPACE
|
||||
int ret;
|
||||
char thread_name[CONFIG_THREAD_MAX_NAME_LEN];
|
||||
char too_small[2];
|
||||
|
||||
/* Some memory-related error cases for k_thread_name_set() */
|
||||
ret = k_thread_name_set(NULL, (const char *)0xFFFFFFF0);
|
||||
zassert_equal(ret, -EFAULT, "accepted nonsense string (%d)", ret);
|
||||
ret = k_thread_name_set(NULL, unreadable_string);
|
||||
zassert_equal(ret, -EFAULT, "accepted unreadable string");
|
||||
ret = k_thread_name_set((struct k_thread *)&sem, "some name");
|
||||
zassert_equal(ret, -EINVAL, "accepted non-thread object");
|
||||
ret = k_thread_name_set(main_thread_ptr, "some name");
|
||||
zassert_equal(ret, -EINVAL, "no permission on thread object");
|
||||
|
||||
/* Set and get current thread's name */
|
||||
ret = k_thread_name_set(NULL, "parent_thread");
|
||||
zassert_equal(ret, 0, "k_thread_name_set() failed");
|
||||
ret = k_thread_name_copy(k_current_get(), thread_name,
|
||||
sizeof(thread_name));
|
||||
zassert_equal(ret, 0, "k_thread_name_copy() failed");
|
||||
ret = strcmp(thread_name, "parent_thread");
|
||||
zassert_equal(ret, 0, "parent thread name does not match");
|
||||
|
||||
/* memory-related cases for k_thread_name_get() */
|
||||
ret = k_thread_name_copy(k_current_get(), too_small,
|
||||
sizeof(too_small));
|
||||
zassert_equal(ret, -ENOSPC, "wrote to too-small buffer");
|
||||
ret = k_thread_name_copy(k_current_get(), not_my_buffer,
|
||||
sizeof(not_my_buffer));
|
||||
zassert_equal(ret, -EFAULT, "wrote to buffer without permission");
|
||||
ret = k_thread_name_copy((struct k_thread *)&sem, thread_name,
|
||||
sizeof(thread_name));
|
||||
zassert_equal(ret, -EINVAL, "not a thread object");
|
||||
ret = k_thread_name_copy(main_thread_ptr, thread_name,
|
||||
sizeof(thread_name));
|
||||
zassert_equal(ret, 0, "couldn't get main thread name");
|
||||
printk("Main thread name is '%s'\n", thread_name);
|
||||
|
||||
/* Set and get child thread's name */
|
||||
k_tid_t tid = k_thread_create(&tdata_name, tstack_name, STACK_SIZE,
|
||||
thread_name_entry, NULL, NULL, NULL,
|
||||
K_PRIO_PREEMPT(1), K_USER, 0);
|
||||
ret = k_thread_name_set(tid, "customdata");
|
||||
zassert_equal(ret, 0, "k_thread_name_set() failed");
|
||||
ret = k_thread_name_copy(tid, thread_name, sizeof(thread_name));
|
||||
zassert_equal(ret, 0, "couldn't get copied thread name");
|
||||
ret = strcmp(thread_name, "customdata");
|
||||
zassert_equal(ret, 0, "child thread name does not match");
|
||||
|
||||
/* cleanup environment */
|
||||
k_thread_abort(tid);
|
||||
#else
|
||||
ztest_test_skip();
|
||||
#endif /* CONFIG_USERSPACE */
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -200,12 +270,20 @@ void test_user_mode(void)
|
|||
}
|
||||
#endif
|
||||
|
||||
extern const k_tid_t _main_thread;
|
||||
|
||||
void test_main(void)
|
||||
{
|
||||
k_thread_access_grant(k_current_get(), &tdata, tstack);
|
||||
k_thread_access_grant(k_current_get(), &tdata_custom, tstack_custom);
|
||||
k_thread_access_grant(k_current_get(), &tdata_name, tstack_name);
|
||||
main_prio = k_thread_priority_get(k_current_get());
|
||||
#ifdef CONFIG_USERSPACE
|
||||
strncpy(unreadable_string, "unreadable string",
|
||||
sizeof(unreadable_string));
|
||||
/* Copy so user mode can get at it */
|
||||
main_thread_ptr = _main_thread;
|
||||
#endif
|
||||
|
||||
ztest_test_suite(threads_lifecycle,
|
||||
ztest_user_unit_test(test_threads_spawn_params),
|
||||
|
|
@ -228,6 +306,7 @@ void test_main(void)
|
|||
ztest_user_unit_test(test_customdata_get_set_preempt),
|
||||
ztest_unit_test(test_k_thread_foreach),
|
||||
ztest_unit_test(test_thread_name_get_set),
|
||||
ztest_user_unit_test(test_thread_name_user_get_set),
|
||||
ztest_unit_test(test_user_mode),
|
||||
ztest_unit_test(test_threads_cpu_mask)
|
||||
);
|
||||
|
|
|
|||
|
|
@ -1,3 +1,3 @@
|
|||
tests:
|
||||
kernel.threads:
|
||||
tags: kernel threads userspace
|
||||
tags: kernel threads userspace ignore_faults
|
||||
|
|
|
|||
Loading…
Reference in a new issue