kernel: pipes: add k_pipe_alloc_init()

User mode can't be trusted to provide the kernel buffers for
internal use. The syscall for k_pipe_init() has been removed
in favor of a new API to draw the buffer memory from the
calling thread's resource pool.

K_PIPE_DEFINE() now properly locates the allocated buffer into
kernel memory.

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>

include/kernel.h

@@ -3347,6 +3347,8 @@ extern int k_mbox_data_block_get(struct k_mbox_msg *rx_msg,
  * @cond INTERNAL_HIDDEN
  */
 
+#define K_PIPE_FLAG_ALLOC	BIT(0)	/* Buffer was allocated */
+
 struct k_pipe {
 	unsigned char *buffer;          /* Pipe buffer: may be NULL */
 	size_t         size;            /* Buffer size */
@@ -3360,6 +3362,7 @@ struct k_pipe {
 	} wait_q;
 
 	_OBJECT_TRACING_NEXT_PTR(k_pipe);
+	u8_t	       flags;		/* Flags */
 };
 
 #define _K_PIPE_INITIALIZER(obj, pipe_buffer, pipe_buffer_size)        \
@@ -3399,7 +3402,7 @@ struct k_pipe {
  * @param pipe_align Alignment of the pipe's ring buffer (power of 2).
  */
 #define K_PIPE_DEFINE(name, pipe_buffer_size, pipe_align)		\
-	static unsigned char __noinit __aligned(pipe_align)   \
+	static unsigned char __kernel_noinit __aligned(pipe_align)	\
 		_k_pipe_buf_##name[pipe_buffer_size];			\
 	struct k_pipe name						\
 		__in_section(_k_pipe, static, name) =			\
@@ -3418,8 +3421,35 @@ struct k_pipe {
  *
  * @return N/A
  */
-__syscall void k_pipe_init(struct k_pipe *pipe, unsigned char *buffer,
+void k_pipe_init(struct k_pipe *pipe, unsigned char *buffer, size_t size);
-			   size_t size);
+
+/**
+ * @brief Release a pipe's allocated buffer
+ *
+ * If a pipe object was given a dynamically allocated buffer via
+ * k_pipe_alloc_init(), this will free it. This function does nothing
+ * if the buffer wasn't dynamically allocated.
+ *
+ * @param pipe Address of the pipe.
+ */
+void k_pipe_cleanup(struct k_pipe *pipe);
+
+/**
+ * @brief Initialize a pipe and allocate a buffer for it
+ *
+ * Storage for the buffer region will be allocated from the calling thread's
+ * resource pool. This memory will be released if k_pipe_cleanup() is called,
+ * or userspace is enabled and the pipe object loses all references to it.
+ *
+ * This function should only be called on uninitialized pipe objects.
+ *
+ * @param pipe Address of the pipe.
+ * @param size Size of the pipe's ring buffer (in bytes), or zero if no ring
+ *             buffer is used.
+ * @retval 0 on success
+ * @retval -ENOMEM if memory couln't be allocated
+ */
+__syscall int k_pipe_alloc_init(struct k_pipe *pipe, size_t size);
 
 /**
  * @brief Write data to a pipe.

kernel/pipes.c

@@ -19,6 +19,7 @@
 #include <misc/dlist.h>
 #include <init.h>
 #include <syscall_handler.h>
+#include <misc/__assert.h>
 
 struct k_pipe_desc {
 	unsigned char *buffer;           /* Position in src/dest buffer */
@@ -127,31 +128,63 @@ SYS_INIT(init_pipes_module, PRE_KERNEL_1, CONFIG_KERNEL_INIT_PRIORITY_OBJECTS);
 
 #endif /* CONFIG_NUM_PIPE_ASYNC_MSGS or CONFIG_OBJECT_TRACING */
 
-void _impl_k_pipe_init(struct k_pipe *pipe, unsigned char *buffer, size_t size)
+void k_pipe_init(struct k_pipe *pipe, unsigned char *buffer, size_t size)
 {
 	pipe->buffer = buffer;
 	pipe->size = size;
 	pipe->bytes_used = 0;
 	pipe->read_index = 0;
 	pipe->write_index = 0;
+	pipe->flags = 0;
 	sys_dlist_init(&pipe->wait_q.writers);
 	sys_dlist_init(&pipe->wait_q.readers);
 	SYS_TRACING_OBJ_INIT(k_pipe, pipe);
 	_k_object_init(pipe);
 }
 
-#ifdef CONFIG_USERSPACE
+int _impl_k_pipe_alloc_init(struct k_pipe *pipe, size_t size)
-_SYSCALL_HANDLER(k_pipe_init, pipe, buffer, size)
 {
-	_SYSCALL_OBJ_INIT(pipe, K_OBJ_PIPE);
+	void *buffer;
-	_SYSCALL_MEMORY_WRITE(buffer, size);
+	int ret;
 
-	_impl_k_pipe_init((struct k_pipe *)pipe, (unsigned char *)buffer,
+	if (size) {
-			  size);
+		buffer = z_thread_malloc(size);
-	return 0;
+		if (buffer) {
+			k_pipe_init(pipe, buffer, size);
+			pipe->flags = K_PIPE_FLAG_ALLOC;
+			ret = 0;
+		} else {
+			ret = -ENOMEM;
+		}
+	} else {
+		k_pipe_init(pipe, NULL, 0);
+		ret = 0;
+	}
+
+	return ret;
+}
+
+#ifdef CONFIG_USERSPACE
+_SYSCALL_HANDLER(k_pipe_alloc_init, pipe, size)
+{
+	_SYSCALL_OBJ_NEVER_INIT(pipe, K_OBJ_PIPE);
+
+	return _impl_k_pipe_alloc_init((struct k_pipe *)pipe, size);
 }
 #endif
 
+void k_pipe_cleanup(struct k_pipe *pipe)
+{
+	__ASSERT_NO_MSG(sys_dlist_is_empty(&pipe->wait_q.readers));
+	__ASSERT_NO_MSG(sys_dlist_is_empty(&pipe->wait_q.writers));
+
+	if (pipe->flags & K_PIPE_FLAG_ALLOC) {
+		k_free(pipe->buffer);
+		pipe->buffer = NULL;
+		pipe->flags &= ~K_PIPE_FLAG_ALLOC;
+	}
+}
+
 /**
  * @brief Copy bytes from @a src to @a dest
  *

kernel/userspace.c

@@ -258,6 +258,9 @@ static void unref_check(struct _k_object *ko)
 	 * specifically needs to happen depends on the object type.
 	 */
 	switch (ko->type) {
+	case K_OBJ_PIPE:
+		k_pipe_cleanup((struct k_pipe *)ko->name);
+		break;
 	default:
 		break;
 	}

tests/kernel/pipe/pipe_api/prj.conf

@@ -1,3 +1,4 @@
 CONFIG_ZTEST=y
 CONFIG_IRQ_OFFLOAD=y
 CONFIG_TEST_USERSPACE=y
+CONFIG_DYNAMIC_OBJECTS=y

tests/kernel/pipe/pipe_api/src/main.c

@@ -13,28 +13,56 @@
 
 #include <ztest.h>
 extern void test_pipe_thread2thread(void);
+
 extern void test_pipe_put_fail(void);
 extern void test_pipe_get_fail(void);
 extern void test_pipe_block_put(void);
 extern void test_pipe_block_put_sema(void);
 extern void test_pipe_get_put(void);
+#ifdef CONFIG_USERSPACE
+extern void test_pipe_user_thread2thread(void);
+extern void test_pipe_user_put_fail(void);
+extern void test_pipe_user_get_fail(void);
+extern void test_resource_pool_auto_free(void);
+#endif
 
 /* k objects */
 extern struct k_pipe pipe, kpipe, put_get_pipe;
 extern struct k_sem end_sema;
 extern struct k_stack tstack;
 extern struct k_thread tdata;
+extern struct k_mem_pool test_pool;
+
+#ifndef CONFIG_USERSPACE
+#define dummy_test(_name) \
+	static void _name(void) \
+	{ \
+		ztest_test_skip(); \
+	}
+
+dummy_test(test_pipe_user_thread2thread);
+dummy_test(test_pipe_user_put_fail);
+dummy_test(test_pipe_user_get_fail);
+dummy_test(test_resource_pool_auto_free);
+#endif /* !CONFIG_USERSPACE */
+
 /*test case main entry*/
 void test_main(void)
 {
-	k_thread_access_grant(k_current_get(),
+	k_thread_access_grant(k_current_get(), &pipe,
-			      &kpipe, &pipe, &end_sema, &tdata, &tstack,
+			      &kpipe, &end_sema, &tdata, &tstack,
 			      &put_get_pipe, NULL);
 
+	k_thread_resource_pool_assign(k_current_get(), &test_pool);
+
 	ztest_test_suite(pipe_api,
-			 ztest_user_unit_test(test_pipe_thread2thread),
+			 ztest_unit_test(test_pipe_thread2thread),
-			 ztest_user_unit_test(test_pipe_put_fail),
+			 ztest_user_unit_test(test_pipe_user_thread2thread),
-			 ztest_user_unit_test(test_pipe_get_fail),
+			 ztest_user_unit_test(test_pipe_user_put_fail),
+			 ztest_user_unit_test(test_pipe_user_get_fail),
+			 ztest_unit_test(test_resource_pool_auto_free),
+			 ztest_unit_test(test_pipe_put_fail),
+			 ztest_unit_test(test_pipe_get_fail),
 			 ztest_unit_test(test_pipe_block_put),
 			 ztest_unit_test(test_pipe_block_put_sema),
 			 ztest_unit_test(test_pipe_get_put));

tests/kernel/pipe/pipe_api/src/test_pipe_contexts.c

@@ -33,6 +33,12 @@ K_THREAD_STACK_DEFINE(tstack, STACK_SIZE);
 __kernel struct k_thread tdata;
 K_SEM_DEFINE(end_sema, 0, 1);
 
+/* By design, only two blocks. We should never need more than that, one
+ * to allocate the pipe object, one for its buffer. Both should be auto-
+ * released when the thread exits
+ */
+K_MEM_POOL_DEFINE(test_pool, 128, 128, 2, 4);
+
 static void tpipe_put(struct k_pipe *ppipe)
 {
 	size_t to_wt, wt_byte = 0;
@@ -127,6 +133,22 @@ void test_pipe_thread2thread(void)
 	tpipe_thread_thread(&kpipe);
 }
 
+#ifdef CONFIG_USERSPACE
+void test_pipe_user_thread2thread(void)
+{
+	/**TESTPOINT: test k_pipe_init pipe*/
+
+	struct k_pipe *p = k_object_alloc(K_OBJ_PIPE);
+
+	zassert_true(p != NULL, NULL);
+	zassert_false(k_pipe_alloc_init(p, PIPE_LEN), NULL);
+	tpipe_thread_thread(&pipe);
+
+	/**TESTPOINT: test K_PIPE_DEFINE pipe*/
+	tpipe_thread_thread(&kpipe);
+}
+#endif
+
 void test_pipe_block_put(void)
 {
 
@@ -172,3 +194,13 @@ void test_pipe_get_put(void)
 	k_thread_abort(tid);
 }
 
+#ifdef CONFIG_USERSPACE
+void test_resource_pool_auto_free(void)
+{
+	/* Pool has 2 blocks, both should succeed if kernel object and pipe
+	 * buffer are auto-freed when the allocating threads exit
+	 */
+	zassert_true(k_mem_pool_malloc(&test_pool, 64) != NULL, NULL);
+	zassert_true(k_mem_pool_malloc(&test_pool, 64) != NULL, NULL);
+}
+#endif

tests/kernel/pipe/pipe_api/src/test_pipe_fail.c

@@ -24,38 +24,74 @@ static unsigned char __aligned(4) data[] = "abcd1234";
 
 __kernel struct k_pipe put_get_pipe;
 
-/*test cases*/
-void test_pipe_put_fail(void *p1, void *p2, void *p3)
-{
 
+static void put_fail(struct k_pipe *p)
+{
 	size_t wt_byte = 0;
 
-	k_pipe_init(&put_get_pipe, data, PIPE_LEN);
+	zassert_false(k_pipe_put(p, data, PIPE_LEN, &wt_byte,
-	zassert_false(k_pipe_put(&put_get_pipe, data, PIPE_LEN, &wt_byte,
 				 1, K_FOREVER), NULL);
 	/**TESTPOINT: pipe put returns -EIO*/
-	zassert_equal(k_pipe_put(&put_get_pipe, data, PIPE_LEN, &wt_byte,
+	zassert_equal(k_pipe_put(p, data, PIPE_LEN, &wt_byte,
 				 1, K_NO_WAIT), -EIO, NULL);
 	zassert_false(wt_byte, NULL);
 	/**TESTPOINT: pipe put returns -EAGAIN*/
-	zassert_equal(k_pipe_put(&put_get_pipe, data, PIPE_LEN, &wt_byte,
+	zassert_equal(k_pipe_put(p, data, PIPE_LEN, &wt_byte,
 				 1, TIMEOUT), -EAGAIN, NULL);
 	zassert_true(wt_byte < 1, NULL);
 }
 
-void test_pipe_get_fail(void *p1, void *p2, void *p3)
+/*test cases*/
+void test_pipe_put_fail(void)
 {
+	k_pipe_init(&put_get_pipe, data, PIPE_LEN);
 
+	put_fail(&put_get_pipe);
+}
+
+#ifdef CONFIG_USERSPACE
+void test_pipe_user_put_fail(void)
+{
+	struct k_pipe *p = k_object_alloc(K_OBJ_PIPE);
+
+	zassert_true(p != NULL, NULL);
+	zassert_false(k_pipe_alloc_init(p, PIPE_LEN), NULL);
+
+	put_fail(p);
+}
+#endif
+
+static void get_fail(struct k_pipe *p)
+{
 	unsigned char rx_data[PIPE_LEN];
 	size_t rd_byte = 0;
 
-	k_pipe_init(&put_get_pipe, data, PIPE_LEN);
 	/**TESTPOINT: pipe put returns -EIO*/
-	zassert_equal(k_pipe_get(&put_get_pipe, rx_data, PIPE_LEN, &rd_byte, 1,
+	zassert_equal(k_pipe_get(p, rx_data, PIPE_LEN, &rd_byte, 1,
 				 K_NO_WAIT), -EIO, NULL);
 	zassert_false(rd_byte, NULL);
 	/**TESTPOINT: pipe put returns -EAGAIN*/
-	zassert_equal(k_pipe_get(&put_get_pipe, rx_data, PIPE_LEN, &rd_byte, 1,
+	zassert_equal(k_pipe_get(p, rx_data, PIPE_LEN, &rd_byte, 1,
 				 TIMEOUT), -EAGAIN, NULL);
 	zassert_true(rd_byte < 1, NULL);
 }
+
+
+void test_pipe_get_fail(void)
+{
+	k_pipe_init(&put_get_pipe, data, PIPE_LEN);
+
+	get_fail(&put_get_pipe);
+}
+
+#ifdef CONFIG_USERSPACE
+void test_pipe_user_get_fail(void)
+{
+	struct k_pipe *p = k_object_alloc(K_OBJ_PIPE);
+
+	zassert_true(p != NULL, NULL);
+	zassert_false(k_pipe_alloc_init(p, PIPE_LEN), NULL);
+
+	get_fail(p);
+}
+#endif