Skip to content
Permalink
Browse files

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>
  • Loading branch information...
andrewboie committed Jun 25, 2019
1 parent 1ee0170 commit 38129ce1a6510debe7e4557f0726928d75ae4290
@@ -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,18 +1304,38 @@ __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
*
* Get the name of a thread
*
* @param thread_id Thread ID
* @retval Thread name, or NULL if configuration not enabled
*/
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 const char *k_thread_name_get(k_tid_t thread_id);
__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)
@@ -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

#ifdef CONFIG_THREAD_NAME
void z_impl_k_thread_name_set(struct k_thread *thread, const char *value)
int z_impl_k_thread_name_set(struct k_thread *thread, const char *value)
{
#ifdef CONFIG_THREAD_NAME
if (thread == NULL) {
_current->name = value;
} else {
thread->name = value;
thread = _current;
}

strncpy(thread->name, value, CONFIG_THREAD_MAX_NAME_LEN);
thread->name[CONFIG_THREAD_MAX_NAME_LEN - 1] = '\0';
return 0;
#else
ARG_UNUSED(thread);
ARG_UNUSED(value);
return -ENOSYS;
#endif /* CONFIG_THREAD_NAME */
}

const char *z_impl_k_thread_name_get(struct k_thread *thread)
#ifdef CONFIG_USERSPACE
Z_SYSCALL_HANDLER(k_thread_name_set, thread, str_param)
{
return (const char *)thread->name;
}
#ifdef CONFIG_THREAD_NAME
struct k_thread *t = (struct k_thread *)thread;
size_t len;
int err;
const char *str = (const char *)str_param;

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
void z_impl_k_thread_name_set(k_tid_t thread_id, const char *value)
{
ARG_UNUSED(thread_id);
ARG_UNUSED(value);
return -ENOSYS;
#endif /* CONFIG_THREAD_NAME */
}
#endif /* CONFIG_USERSPACE */

const char *z_impl_k_thread_name_get(k_tid_t thread_id)
const char *k_thread_name_get(struct k_thread *thread)
{
ARG_UNUSED(thread_id);
#ifdef CONFIG_THREAD_NAME
return (const char *)thread->name;
#else
ARG_UNUSED(thread);
return NULL;
}
#endif /* CONFIG_THREAD_NAME */
}

#ifdef CONFIG_USERSPACE

#if defined(CONFIG_THREAD_NAME)
Z_SYSCALL_HANDLER(k_thread_name_set, thread, data)
int z_impl_k_thread_name_copy(k_tid_t thread_id, char *buf, size_t size)
{
char *name_copy = NULL;

name_copy = z_user_string_alloc_copy((char *)data, 64);
z_impl_k_thread_name_set((struct k_thread *)thread, name_copy);
#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_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)
#ifdef CONFIG_USERSPACE
Z_SYSCALL_HANDLER(k_thread_name_copy, thread_id, buf, size)
{
z_impl_k_thread_custom_data_set((void *)data);
return 0;
}
#ifdef CONFIG_THREAD_NAME
size_t len;
struct k_thread *t = (struct k_thread *)thread_id;
struct _k_object *ko = z_object_find(t);

Z_SYSCALL_HANDLER0_SIMPLE(k_thread_custom_data_get);
#endif /* CONFIG_THREAD_CUSTOM_DATA */
/* 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);

0 comments on commit 38129ce

Please sign in to comment.
You can’t perform that action at this time.