From 0fe789ff2ea89b8d9b847b9d6bceae84d3beb144 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Thu, 12 Apr 2018 18:35:56 -0700 Subject: [PATCH] kernel: add k_msgq_alloc_init() User mode can't be trusted to provide a memory buffer to k_msgq_init(). Introduce k_msgq_alloc_init() which allocates the buffer out of the calling thread's resource pool and expose that as a system call instead. Signed-off-by: Andrew Boie --- include/kernel.h | 34 +++++++- kernel/msg_q.c | 51 +++++++++-- kernel/userspace.c | 3 + tests/kernel/msgq/msgq_api/prj.conf | 2 + tests/kernel/msgq/msgq_api/src/main.c | 45 ++++++++-- .../msgq/msgq_api/src/test_msgq_attrs.c | 64 +++++++++----- .../msgq/msgq_api/src/test_msgq_contexts.c | 26 ++++++ .../kernel/msgq/msgq_api/src/test_msgq_fail.c | 84 +++++++++++++------ .../msgq/msgq_api/src/test_msgq_purge.c | 60 ++++++++----- 9 files changed, 283 insertions(+), 86 deletions(-) diff --git a/include/kernel.h b/include/kernel.h index 21a0b3a33c0..098bdd7fadd 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -2946,6 +2946,7 @@ struct k_msgq { u32_t used_msgs; _OBJECT_TRACING_NEXT_PTR(k_msgq); + u8_t flags; }; #define _K_MSGQ_INITIALIZER(obj, q_buffer, q_msg_size, q_max_msgs) \ @@ -2963,6 +2964,8 @@ struct k_msgq { #define K_MSGQ_INITIALIZER DEPRECATED_MACRO _K_MSGQ_INITIALIZER +#define K_MSGQ_FLAG_ALLOC BIT(0) + struct k_msgq_attrs { size_t msg_size; u32_t max_msgs; @@ -2999,7 +3002,7 @@ struct k_msgq_attrs { * @param q_align Alignment of the message queue's ring buffer. */ #define K_MSGQ_DEFINE(q_name, q_msg_size, q_max_msgs, q_align) \ - static char __noinit __aligned(q_align) \ + static char __kernel_noinit __aligned(q_align) \ _k_fifo_buf_##q_name[(q_max_msgs) * (q_msg_size)]; \ struct k_msgq q_name \ __in_section(_k_msgq, static, q_name) = \ @@ -3024,8 +3027,33 @@ struct k_msgq_attrs { * * @return N/A */ -__syscall void k_msgq_init(struct k_msgq *q, char *buffer, - size_t msg_size, u32_t max_msgs); +void k_msgq_init(struct k_msgq *q, char *buffer, size_t msg_size, + u32_t max_msgs); + +/** + * @brief Initialize a message queue. + * + * This routine initializes a message queue object, prior to its first use, + * allocating its internal ring buffer from the calling thread's resource + * pool. + * + * Memory allocated for the ring buffer can be released by calling + * k_msgq_cleanup(), or if userspace is enabled and the msgq object loses + * all of its references. + * + * @param q Address of the message queue. + * @param msg_size Message size (in bytes). + * @param max_msgs Maximum number of messages that can be queued. + * + * @return 0 on success, -ENOMEM if there was insufficient memory in the + * thread's resource pool, or -EINVAL if the size parameters cause + * an integer overflow. + */ +__syscall int k_msgq_alloc_init(struct k_msgq *q, size_t msg_size, + u32_t max_msgs); + + +void k_msgq_cleanup(struct k_msgq *q); /** * @brief Send a message to a message queue. diff --git a/kernel/msg_q.c b/kernel/msg_q.c index c92f6319bd7..8c388a216bf 100644 --- a/kernel/msg_q.c +++ b/kernel/msg_q.c @@ -47,8 +47,8 @@ SYS_INIT(init_msgq_module, PRE_KERNEL_1, CONFIG_KERNEL_INIT_PRIORITY_OBJECTS); #endif /* CONFIG_OBJECT_TRACING */ -void _impl_k_msgq_init(struct k_msgq *q, char *buffer, - size_t msg_size, u32_t max_msgs) +void k_msgq_init(struct k_msgq *q, char *buffer, size_t msg_size, + u32_t max_msgs) { q->msg_size = msg_size; q->max_msgs = max_msgs; @@ -57,24 +57,57 @@ void _impl_k_msgq_init(struct k_msgq *q, char *buffer, q->read_ptr = buffer; q->write_ptr = buffer; q->used_msgs = 0; + q->flags = 0; sys_dlist_init(&q->wait_q); SYS_TRACING_OBJ_INIT(k_msgq, q); _k_object_init(q); } -#ifdef CONFIG_USERSPACE -_SYSCALL_HANDLER(k_msgq_init, q, buffer, msg_size, max_msgs) +int _impl_k_msgq_alloc_init(struct k_msgq *q, size_t msg_size, + u32_t max_msgs) { - _SYSCALL_OBJ_INIT(q, K_OBJ_MSGQ); - _SYSCALL_MEMORY_ARRAY_WRITE(buffer, max_msgs, msg_size); + void *buffer; + int ret; + size_t total_size; - _impl_k_msgq_init((struct k_msgq *)q, (char *)buffer, msg_size, - max_msgs); - return 0; + if (__builtin_umul_overflow((u32_t)msg_size, max_msgs, + (u32_t *)&total_size)) { + ret = -EINVAL; + } else { + buffer = z_thread_malloc(total_size); + if (buffer) { + k_msgq_init(q, buffer, msg_size, max_msgs); + q->flags = K_MSGQ_FLAG_ALLOC; + ret = 0; + } else { + ret = -ENOMEM; + } + } + + return ret; +} + +#ifdef CONFIG_USERSPACE +_SYSCALL_HANDLER(k_msgq_alloc_init, q, msg_size, max_msgs) +{ + _SYSCALL_OBJ_NEVER_INIT(q, K_OBJ_MSGQ); + + return _impl_k_msgq_alloc_init((struct k_msgq *)q, msg_size, max_msgs); } #endif +void k_msgq_cleanup(struct k_msgq *q) +{ + __ASSERT_NO_MSG(sys_dlist_is_empty(&q->wait_q)); + + if (q->flags & K_MSGQ_FLAG_ALLOC) { + k_free(q->buffer_start); + q->flags &= ~K_MSGQ_FLAG_ALLOC; + } +} + + int _impl_k_msgq_put(struct k_msgq *q, void *data, s32_t timeout) { __ASSERT(!_is_in_isr() || timeout == K_NO_WAIT, ""); diff --git a/kernel/userspace.c b/kernel/userspace.c index bcd196f25a6..0b3ee297d6e 100644 --- a/kernel/userspace.c +++ b/kernel/userspace.c @@ -261,6 +261,9 @@ static void unref_check(struct _k_object *ko) case K_OBJ_PIPE: k_pipe_cleanup((struct k_pipe *)ko->name); break; + case K_OBJ_MSGQ: + k_msgq_cleanup((struct k_msgq *)ko->name); + break; default: break; } diff --git a/tests/kernel/msgq/msgq_api/prj.conf b/tests/kernel/msgq/msgq_api/prj.conf index 9a75212e89d..117448d7f07 100644 --- a/tests/kernel/msgq/msgq_api/prj.conf +++ b/tests/kernel/msgq/msgq_api/prj.conf @@ -1,2 +1,4 @@ CONFIG_ZTEST=y CONFIG_IRQ_OFFLOAD=y +CONFIG_USERSPACE=y +CONFIG_DYNAMIC_OBJECTS=y diff --git a/tests/kernel/msgq/msgq_api/src/main.c b/tests/kernel/msgq/msgq_api/src/main.c index 9de895dcfad..e857af96fc9 100644 --- a/tests/kernel/msgq/msgq_api/src/main.c +++ b/tests/kernel/msgq/msgq_api/src/main.c @@ -19,6 +19,29 @@ extern void test_msgq_put_fail(void); extern void test_msgq_get_fail(void); extern void test_msgq_purge_when_put(void); extern void test_msgq_attrs_get(void); +#ifdef CONFIG_USERSPACE +extern void test_msgq_user_thread(void); +extern void test_msgq_user_thread_overflow(void); +extern void test_msgq_user_put_fail(void); +extern void test_msgq_user_get_fail(void); +extern void test_msgq_user_attrs_get(void); +extern void test_msgq_user_purge_when_put(void); + +K_MEM_POOL_DEFINE(test_pool, 128, 128, 2, 4); +#else +#define dummy_test(_name) \ + static void _name(void) \ + { \ + ztest_test_skip(); \ + } + +dummy_test(test_msgq_user_thread); +dummy_test(test_msgq_user_thread_overflow); +dummy_test(test_msgq_user_put_fail); +dummy_test(test_msgq_user_get_fail); +dummy_test(test_msgq_user_attrs_get); +dummy_test(test_msgq_user_purge_when_put); +#endif /* CONFIG_USERSPACE */ extern struct k_msgq kmsgq; extern struct k_msgq msgq; @@ -32,13 +55,23 @@ void test_main(void) k_thread_access_grant(k_current_get(), &kmsgq, &msgq, &end_sema, &tdata, &tstack, NULL); +#ifdef CONFIG_USERSPACE + k_thread_resource_pool_assign(k_current_get(), &test_pool); +#endif + ztest_test_suite(msgq_api, - ztest_user_unit_test(test_msgq_thread), - ztest_user_unit_test(test_msgq_thread_overflow), + ztest_unit_test(test_msgq_thread), + ztest_unit_test(test_msgq_thread_overflow), + ztest_user_unit_test(test_msgq_user_thread), + ztest_user_unit_test(test_msgq_user_thread_overflow), ztest_unit_test(test_msgq_isr), - ztest_user_unit_test(test_msgq_put_fail), - ztest_user_unit_test(test_msgq_get_fail), - ztest_user_unit_test(test_msgq_attrs_get), - ztest_user_unit_test(test_msgq_purge_when_put)); + ztest_unit_test(test_msgq_put_fail), + ztest_unit_test(test_msgq_get_fail), + ztest_user_unit_test(test_msgq_user_put_fail), + ztest_user_unit_test(test_msgq_user_get_fail), + ztest_unit_test(test_msgq_attrs_get), + ztest_user_unit_test(test_msgq_user_attrs_get), + ztest_unit_test(test_msgq_purge_when_put), + ztest_user_unit_test(test_msgq_user_purge_when_put)); ztest_run_test_suite(msgq_api); } diff --git a/tests/kernel/msgq/msgq_api/src/test_msgq_attrs.c b/tests/kernel/msgq/msgq_api/src/test_msgq_attrs.c index e490fdb1254..7ef7aa340c6 100644 --- a/tests/kernel/msgq/msgq_api/src/test_msgq_attrs.c +++ b/tests/kernel/msgq/msgq_api/src/test_msgq_attrs.c @@ -10,38 +10,56 @@ static char __aligned(4) tbuffer[MSG_SIZE * MSGQ_LEN]; static u32_t send_buf[MSGQ_LEN] = { MSG0, MSG1 }; static u32_t rec_buf[MSGQ_LEN] = { MSG0, MSG1 }; +static void attrs_get(struct k_msgq *q) +{ + int ret; + struct k_msgq_attrs attrs; + + k_msgq_get_attrs(q, &attrs); + zassert_equal(attrs.used_msgs, 0, NULL); + + /*fill the queue to full*/ + for (int i = 0; i < MSGQ_LEN; i++) { + ret = k_msgq_put(q, (void *)&send_buf[i], K_NO_WAIT); + zassert_equal(ret, 0, NULL); + } + + k_msgq_get_attrs(q, &attrs); + zassert_equal(attrs.used_msgs, MSGQ_LEN, NULL); + + for (int i = 0; i < MSGQ_LEN; i++) { + ret = k_msgq_get(q, (void *)&rec_buf[i], K_NO_WAIT); + zassert_equal(ret, 0, NULL); + } + + k_msgq_get_attrs(q, &attrs); + zassert_equal(attrs.used_msgs, 0, NULL); +} + /** * @brief Verify zephyr msgq get attributes API. * @addtogroup kernel_message_queue * @{ */ + void test_msgq_attrs_get(void) { - int ret; - struct k_msgq_attrs attrs; - k_msgq_init(&msgq, tbuffer, MSG_SIZE, MSGQ_LEN); - - k_msgq_get_attrs(&msgq, &attrs); - zassert_equal(attrs.used_msgs, 0, NULL); - - /*fill the queue to full*/ - for (int i = 0; i < MSGQ_LEN; i++) { - ret = k_msgq_put(&msgq, (void *)&send_buf[i], K_NO_WAIT); - zassert_equal(ret, 0, NULL); - } - - k_msgq_get_attrs(&msgq, &attrs); - zassert_equal(attrs.used_msgs, MSGQ_LEN, NULL); - - for (int i = 0; i < MSGQ_LEN; i++) { - ret = k_msgq_get(&msgq, (void *)&rec_buf[i], K_NO_WAIT); - zassert_equal(ret, 0, NULL); - } - - k_msgq_get_attrs(&msgq, &attrs); - zassert_equal(attrs.used_msgs, 0, NULL); + attrs_get(&msgq); } + +#ifdef CONFIG_USERSPACE +void test_msgq_user_attrs_get(void) +{ + struct k_msgq *q; + + q = k_object_alloc(K_OBJ_MSGQ); + zassert_not_null(q, "couldn't alloc message queue"); + zassert_false(k_msgq_alloc_init(q, MSG_SIZE, MSGQ_LEN), NULL); + attrs_get(q); +} +#endif + /** * @} */ diff --git a/tests/kernel/msgq/msgq_api/src/test_msgq_contexts.c b/tests/kernel/msgq/msgq_api/src/test_msgq_contexts.c index 7717df3a039..b6dbd7d7be1 100644 --- a/tests/kernel/msgq/msgq_api/src/test_msgq_contexts.c +++ b/tests/kernel/msgq/msgq_api/src/test_msgq_contexts.c @@ -156,6 +156,32 @@ void test_msgq_thread_overflow(void) msgq_thread_overflow(&kmsgq); } +#ifdef CONFIG_USERSPACE +void test_msgq_user_thread(void) +{ + struct k_msgq *q; + + q = k_object_alloc(K_OBJ_MSGQ); + zassert_not_null(q, "couldn't alloc message queue"); + zassert_false(k_msgq_alloc_init(q, MSG_SIZE, MSGQ_LEN), NULL); + k_sem_init(&end_sema, 0, 1); + + msgq_thread(q); +} + +void test_msgq_user_thread_overflow(void) +{ + struct k_msgq *q; + + q = k_object_alloc(K_OBJ_MSGQ); + zassert_not_null(q, "couldn't alloc message queue"); + zassert_false(k_msgq_alloc_init(q, MSG_SIZE, 1), NULL); + k_sem_init(&end_sema, 0, 1); + + msgq_thread_overflow(q); +} +#endif /* CONFIG_USERSPACE */ + void test_msgq_isr(void) { struct k_msgq stack_msgq; diff --git a/tests/kernel/msgq/msgq_api/src/test_msgq_fail.c b/tests/kernel/msgq/msgq_api/src/test_msgq_fail.c index 44c8a688e58..3d715d834c5 100644 --- a/tests/kernel/msgq/msgq_api/src/test_msgq_fail.c +++ b/tests/kernel/msgq/msgq_api/src/test_msgq_fail.c @@ -9,43 +9,77 @@ static char __aligned(4) tbuffer[MSG_SIZE * MSGQ_LEN]; static u32_t data[MSGQ_LEN] = { MSG0, MSG1 }; extern struct k_msgq msgq; +static void put_fail(struct k_msgq *q) +{ + int ret = k_msgq_put(q, (void *)&data[0], K_NO_WAIT); + + zassert_false(ret, NULL); + ret = k_msgq_put(q, (void *)&data[0], K_NO_WAIT); + zassert_false(ret, NULL); + /**TESTPOINT: msgq put returns -ENOMSG*/ + ret = k_msgq_put(q, (void *)&data[1], K_NO_WAIT); + zassert_equal(ret, -ENOMSG, NULL); + /**TESTPOINT: msgq put returns -EAGAIN*/ + ret = k_msgq_put(q, (void *)&data[0], TIMEOUT); + zassert_equal(ret, -EAGAIN, NULL); + + k_msgq_purge(q); +} + +static void get_fail(struct k_msgq *q) +{ + u32_t rx_data; + + /**TESTPOINT: msgq get returns -ENOMSG*/ + int ret = k_msgq_get(q, &rx_data, K_NO_WAIT); + + zassert_equal(ret, -ENOMSG, NULL); + /**TESTPOINT: msgq get returns -EAGAIN*/ + ret = k_msgq_get(q, &rx_data, TIMEOUT); + zassert_equal(ret, -EAGAIN, NULL); +} + /** * @brief Verify zephyr msgq return code under negative tests * @addtogroup kernel_message_queue * @{ */ -void test_msgq_put_fail(void *p1, void *p2, void *p3) + +void test_msgq_put_fail(void) { k_msgq_init(&msgq, tbuffer, MSG_SIZE, MSGQ_LEN); - - int ret = k_msgq_put(&msgq, (void *)&data[0], K_NO_WAIT); - - zassert_false(ret, NULL); - ret = k_msgq_put(&msgq, (void *)&data[0], K_NO_WAIT); - zassert_false(ret, NULL); - /**TESTPOINT: msgq put returns -ENOMSG*/ - ret = k_msgq_put(&msgq, (void *)&data[1], K_NO_WAIT); - zassert_equal(ret, -ENOMSG, NULL); - /**TESTPOINT: msgq put returns -EAGAIN*/ - ret = k_msgq_put(&msgq, (void *)&data[0], TIMEOUT); - zassert_equal(ret, -EAGAIN, NULL); - - k_msgq_purge(&msgq); + put_fail(&msgq); } -void test_msgq_get_fail(void *p1, void *p2, void *p3) +#ifdef CONFIG_USERSPACE +void test_msgq_user_put_fail(void) { - u32_t rx_data; + struct k_msgq *q; - k_msgq_init(&msgq, tbuffer, MSG_SIZE, MSGQ_LEN); - /**TESTPOINT: msgq get returns -ENOMSG*/ - int ret = k_msgq_get(&msgq, &rx_data, K_NO_WAIT); - - zassert_equal(ret, -ENOMSG, NULL); - /**TESTPOINT: msgq get returns -EAGAIN*/ - ret = k_msgq_get(&msgq, &rx_data, TIMEOUT); - zassert_equal(ret, -EAGAIN, NULL); + q = k_object_alloc(K_OBJ_MSGQ); + zassert_not_null(q, "couldn't alloc message queue"); + zassert_false(k_msgq_alloc_init(q, MSG_SIZE, MSGQ_LEN), NULL); + put_fail(q); } +#endif /* CONFIG_USERSPACE */ + +void test_msgq_get_fail(void) +{ + k_msgq_init(&msgq, tbuffer, MSG_SIZE, MSGQ_LEN); + get_fail(&msgq); +} + +#ifdef CONFIG_USERSPACE +void test_msgq_user_get_fail(void) +{ + struct k_msgq *q; + + q = k_object_alloc(K_OBJ_MSGQ); + zassert_not_null(q, "couldn't alloc message queue"); + zassert_false(k_msgq_alloc_init(q, MSG_SIZE, MSGQ_LEN), NULL); + get_fail(q); +} +#endif /* CONFIG_USERSPACE */ /** * @} diff --git a/tests/kernel/msgq/msgq_api/src/test_msgq_purge.c b/tests/kernel/msgq/msgq_api/src/test_msgq_purge.c index 342dcf739c2..3dd635168ed 100644 --- a/tests/kernel/msgq/msgq_api/src/test_msgq_purge.c +++ b/tests/kernel/msgq/msgq_api/src/test_msgq_purge.c @@ -19,36 +19,56 @@ static void tThread_entry(void *p1, void *p2, void *p3) zassert_equal(ret, -ENOMSG, NULL); } +static void purge_when_put(struct k_msgq *q) +{ + int ret; + + /*fill the queue to full*/ + for (int i = 0; i < MSGQ_LEN; i++) { + ret = k_msgq_put(q, (void *)&data[i], K_NO_WAIT); + zassert_equal(ret, 0, NULL); + } + /*create another thread waiting to put msg*/ + k_thread_create(&tdata, tstack, STACK_SIZE, + tThread_entry, q, NULL, NULL, + K_PRIO_PREEMPT(0), K_USER | K_INHERIT_PERMS, 0); + k_sleep(TIMEOUT >> 1); + /**TESTPOINT: msgq purge while another thread waiting to put msg*/ + k_msgq_purge(q); + + /*verify msg put after purge*/ + for (int i = 0; i < MSGQ_LEN; i++) { + ret = k_msgq_put(q, (void *)&data[i], K_NO_WAIT); + zassert_equal(ret, 0, NULL); + } +} + /** * @brief Verify zephyr msgq purge under different scenario * @addtogroup kernel_message_queue * @{ */ + void test_msgq_purge_when_put(void) { - int ret; - k_msgq_init(&msgq, tbuffer, MSG_SIZE, MSGQ_LEN); - /*fill the queue to full*/ - for (int i = 0; i < MSGQ_LEN; i++) { - ret = k_msgq_put(&msgq, (void *)&data[i], K_NO_WAIT); - zassert_equal(ret, 0, NULL); - } - /*create another thread waiting to put msg*/ - k_thread_create(&tdata, tstack, STACK_SIZE, - tThread_entry, &msgq, NULL, NULL, - K_PRIO_PREEMPT(0), K_USER | K_INHERIT_PERMS, 0); - k_sleep(TIMEOUT >> 1); - /**TESTPOINT: msgq purge while another thread waiting to put msg*/ - k_msgq_purge(&msgq); - - /*verify msg put after purge*/ - for (int i = 0; i < MSGQ_LEN; i++) { - ret = k_msgq_put(&msgq, (void *)&data[i], K_NO_WAIT); - zassert_equal(ret, 0, NULL); - } + purge_when_put(&msgq); } + +#ifdef CONFIG_USERSPACE +void test_msgq_user_purge_when_put(void) +{ + struct k_msgq *q; + + q = k_object_alloc(K_OBJ_MSGQ); + zassert_not_null(q, "couldn't alloc message queue"); + zassert_false(k_msgq_alloc_init(q, MSG_SIZE, MSGQ_LEN), NULL); + + purge_when_put(q); +} +#endif + /** * @} */