Syscall handler cleanups

include/kernel.h

@@ -178,28 +178,6 @@ struct _k_object {
 } __packed;
 
 #define K_OBJ_FLAG_INITIALIZED	BIT(0)
-/**
- * Ensure a system object is a valid object of the expected type
- *
- * Searches for the object and ensures that it is indeed an object
- * of the expected type, that the caller has the right permissions on it,
- * and that the object has been initialized.
- *
- * This function is intended to be called on the kernel-side system
- * call handlers to validate kernel object pointers passed in from
- * userspace.
- *
- * @param obj Address of the kernel object
- * @param otype Expected type of the kernel object
- * @param init If true, this is for an init function and we will not error
- *	   out if the object is not initialized
- * @return 0 If the object is valid
- *         -EBADF if not a valid object of the specified type
- *         -EPERM If the caller does not have permissions
- *         -EINVAL Object is not initialized
- */
-int _k_object_validate(void *obj, enum k_objects otype, int init);
-
 
 /**
  * Lookup a kernel object and init its metadata if it exists
@@ -212,15 +190,6 @@ int _k_object_validate(void *obj, enum k_objects otype, int init);
  */
 void _k_object_init(void *obj);
 #else
-static inline int _k_object_validate(void *obj, enum k_objects otype, int init)
-{
-	ARG_UNUSED(obj);
-	ARG_UNUSED(otype);
-	ARG_UNUSED(init);
-
-	return 0;
-}
-
 static inline void _k_object_init(void *obj)
 {
 	ARG_UNUSED(obj);

kernel/alert.c

@@ -97,7 +97,7 @@ u32_t _handler_k_alert_send(u32_t alert, u32_t arg2, u32_t arg3,
 {
 	_SYSCALL_ARG1;
 
-	_SYSCALL_IS_OBJ(alert, K_OBJ_ALERT, 0, ssf);
+	_SYSCALL_OBJ(alert, K_OBJ_ALERT, ssf);
 	_impl_k_alert_send((struct k_alert *)alert);
 
 	return 0;
@@ -115,7 +115,7 @@ u32_t _handler_k_alert_recv(u32_t alert, u32_t timeout, u32_t arg3,
 {
 	_SYSCALL_ARG2;
 
-	_SYSCALL_IS_OBJ(alert, K_OBJ_ALERT, 0, ssf);
+	_SYSCALL_OBJ(alert, K_OBJ_ALERT, ssf);
 	return _impl_k_alert_recv((struct k_alert *)alert, timeout);
 }
 #endif

kernel/include/syscall_handler.h

@@ -17,55 +17,178 @@
 
 extern const _k_syscall_handler_t _k_syscall_table[K_SYSCALL_LIMIT];
 
+/**
+ * Ensure a system object is a valid object of the expected type
+ *
+ * Searches for the object and ensures that it is indeed an object
+ * of the expected type, that the caller has the right permissions on it,
+ * and that the object has been initialized.
+ *
+ * This function is intended to be called on the kernel-side system
+ * call handlers to validate kernel object pointers passed in from
+ * userspace.
+ *
+ * @param obj Address of the kernel object
+ * @param otype Expected type of the kernel object
+ * @param init If true, this is for an init function and we will not error
+ *	   out if the object is not initialized
+ * @return 0 If the object is valid
+ *         -EBADF if not a valid object of the specified type
+ *         -EPERM If the caller does not have permissions
+ *         -EINVAL Object is not initialized
+ */
+int _k_object_validate(void *obj, enum k_objects otype, int init);
+
 /**
  * @brief Runtime expression check for system call arguments
  *
  * Used in handler functions to perform various runtime checks on arguments,
- * and generate a kernel oops if anything is not expected
+ * and generate a kernel oops if anything is not expected, printing a custom
+ * message.
  *
  * @param expr Boolean expression to verify, a false result will trigger an
  *             oops
  * @param ssf Syscall stack frame argument passed to the handler function
+ * @param fmt Printf-style format string (followed by appropriate variadic
+ *            arguments) to print on verification failure
  */
-#define _SYSCALL_VERIFY(expr, ssf) \
+#define _SYSCALL_VERIFY_MSG(expr, ssf, fmt, ...) \
 	do { \
 		if (!(expr)) { \
-			printk("FATAL: syscall failed check: " #expr "\n"); \
+			printk("FATAL: syscall %s failed check: " fmt "\n", \
+			       __func__, ##__VA_ARGS__); \
 			_arch_syscall_oops(ssf); \
 		} \
 	} while (0)
 
 /**
- * @brief Runtime check that a user thread has proper access to a memory area
+ * @brief Runtime expression check for system call arguments
  *
- * Checks that the particular memory area is readable or writable by the
+ * Used in handler functions to perform various runtime checks on arguments,
+ * and generate a kernel oops if anything is not expected.
+ *
+ * @param expr Boolean expression to verify, a false result will trigger an
+ *             oops. A stringified version of this expression will be printed.
+ * @param ssf Syscall stack frame argument passed to the handler function
+ *            arguments) to print on verification failure
+ */
+#define _SYSCALL_VERIFY(expr, ssf) _SYSCALL_VERIFY_MSG(expr, ssf, #expr)
+
+#define _SYSCALL_MEMORY(ptr, size, write, ssf) \
+	_SYSCALL_VERIFY_MSG(!_arch_buffer_validate((void *)ptr, size, write), \
+			    ssf, "Memory region %p (size %u) %s access denied", \
+			    (void *)(ptr), (u32_t)(size), \
+			    write ? "write" : "read")
+
+/**
+ * @brief Runtime check that a user thread has read permission to a memory area
+ *
+ * Checks that the particular memory area is readable by the currently running
+ * thread if the CPU was in user mode, and generates a kernel oops if it
+ * wasn't. Prevents userspace from getting the kernel to read memory the thread
+ * does not have access to, or passing in garbage pointers that would
+ * crash/pagefault the kernel if dereferenced.
+ *
+ * @param ptr Memory area to examine
+ * @param size Size of the memory area
+ * @param write If the thread should be able to write to this memory, not just
+ *		read it
+ * @param ssf Syscall stack frame argument passed to the handler function
+ */
+#define _SYSCALL_MEMORY_READ(ptr, size, ssf) \
+	_SYSCALL_MEMORY(ptr, size, 0, ssf)
+
+/**
+ * @brief Runtime check that a user thread has write permission to a memory area
+ *
+ * Checks that the particular memory area is readable and writable by the
  * currently running thread if the CPU was in user mode, and generates a kernel
  * oops if it wasn't. Prevents userspace from getting the kernel to read or
  * modify memory the thread does not have access to, or passing in garbage
- * pointers that would crash/pagefault the kernel if accessed.
+ * pointers that would crash/pagefault the kernel if dereferenced.
  *
  * @param ptr Memory area to examine
  * @param size Size of the memory area
  * @param write If the thread should be able to write to this memory, not just
  *		read it
  * @param ssf Syscall stack frame argument passed to the handler function
  */
-#define _SYSCALL_MEMORY(ptr, size, write, ssf) \
-	_SYSCALL_VERIFY(!_arch_buffer_validate((void *)ptr, size, write), ssf)
+#define _SYSCALL_MEMORY_WRITE(ptr, size, ssf) \
+	_SYSCALL_MEMORY(ptr, size, 1, ssf)
+
+#define _SYSCALL_MEMORY_ARRAY(ptr, nmemb, size, write, ssf) \
+	do { \
+		u32_t product; \
+		_SYSCALL_VERIFY_MSG(!__builtin_umul_overflow((u32_t)(nmemb), \
+							     (u32_t)(size), \
+							     &product), ssf, \
+				    "%ux%u array is too large", \
+				    (u32_t)(nmemb), (u32_t)(size)); \
+		_SYSCALL_MEMORY(ptr, product, write, ssf); \
+	} while (0)
+
+/**
+ * @brief Validate user thread has read permission for sized array
+ *
+ * Used when the memory region is expressed in terms of number of elements and
+ * each element size, handles any overflow issues with computing the total
+ * array bounds. Otherwise see _SYSCALL_MEMORY_READ.
+ *
+ * @param ptr Memory area to examine
+ * @param nmemb Number of elements in the array
+ * @param size Size of each array element
+ * @param ssf Syscall stack frame argument passed to the handler function
+ */
+#define _SYSCALL_MEMORY_ARRAY_READ(ptr, nmemb, size, ssf) \
+	_SYSCALL_MEMORY_ARRAY(ptr, nmemb, size, 0, ssf)
+
+/**
+ * @brief Validate user thread has read/write permission for sized array
+ *
+ * Used when the memory region is expressed in terms of number of elements and
+ * each element size, handles any overflow issues with computing the total
+ * array bounds. Otherwise see _SYSCALL_MEMORY_WRITE.
+ *
+ * @param ptr Memory area to examine
+ * @param nmemb Number of elements in the array
+ * @param size Size of each array element
+ * @param ssf Syscall stack frame argument passed to the handler function
+ */
+#define _SYSCALL_MEMORY_ARRAY_WRITE(ptr, nmemb, size, ssf) \
+	_SYSCALL_MEMORY_ARRAY(ptr, nmemb, size, 1, ssf)
+
+#define _SYSCALL_IS_OBJ(ptr, type, init, ssf) \
+	_SYSCALL_VERIFY_MSG(!_k_object_validate((void *)ptr, type, init), ssf, \
+			    "object %p access denied", (void *)(ptr))
 
 /**
- * @brief Runtime check that a pointer is a kernel object of expected type
+ * @brief Runtime check kernel object pointer for non-init functions
  *
- * Passes along arguments to _k_object_validate() and triggers a kernel oops
- * if the object wasn't valid or had incorrect permissions.
+ * Calls _k_object_validate and triggers a kernel oops if the check files.
+ * For use in system call handlers which are not init functions; a check
+ * enforcing that an object is initialized* will not occur.
  *
  * @param ptr Untrusted kernel object pointer
  * @param type Expected kernel object type
- * @param init Whether this is an init function handler
  * @param ssf Syscall stack frame argument passed to the handler function
  */
-#define _SYSCALL_IS_OBJ(ptr, type, init, ssf) \
-	_SYSCALL_VERIFY(!_k_object_validate((void *)ptr, type, init), ssf)
+#define _SYSCALL_OBJ(ptr, type, ssf) \
+	_SYSCALL_IS_OBJ(ptr, type, 0, ssf)
+
+/**
+ * @brief Runtime check kernel object pointer for non-init functions
+ *
+ * See description of _SYSCALL_IS_OBJ. For use in system call handlers which
+ * are not init functions; a check enforcing that an object is initialized
+ * will not occur.
+ *
+ * @param ptr Untrusted kernel object pointer
+ * @param type Expected kernel object type
+ * @param ssf Syscall stack frame argument passed to the handler function
+ */
+
+#define _SYSCALL_OBJ_INIT(ptr, type, ssf) \
+	_SYSCALL_IS_OBJ(ptr, type, 1, ssf)
 
 /* Convenience macros for handler implementations */
 #define _SYSCALL_ARG0	ARG_UNUSED(arg1); ARG_UNUSED(arg2); ARG_UNUSED(arg3); \

kernel/msg_q.c

@@ -69,8 +69,8 @@ u32_t _handler_k_msgq_init(u32_t q, u32_t buffer, u32_t msg_size,
 {
 	_SYSCALL_ARG4;
 
-	_SYSCALL_IS_OBJ(q, K_OBJ_MSGQ, 1, ssf);
-	_SYSCALL_MEMORY(buffer, msg_size * max_msgs, 1, ssf);
+	_SYSCALL_OBJ_INIT(q, K_OBJ_MSGQ, ssf);
+	_SYSCALL_MEMORY_ARRAY_WRITE(buffer, max_msgs, msg_size, ssf);
 
 	_impl_k_msgq_init((struct k_msgq *)q, (char *)buffer, msg_size,
 			  max_msgs);
@@ -133,8 +133,8 @@ u32_t _handler_k_msgq_put(u32_t msgq_p, u32_t data, u32_t timeout,
 	struct k_msgq *q = (struct k_msgq *)msgq_p;
 	_SYSCALL_ARG3;
 
-	_SYSCALL_IS_OBJ(q, K_OBJ_MSGQ, 0, ssf);
-	_SYSCALL_MEMORY(data, q->msg_size, 0, ssf);
+	_SYSCALL_OBJ(q, K_OBJ_MSGQ, ssf);
+	_SYSCALL_MEMORY_READ(data, q->msg_size, ssf);
 
 	return _impl_k_msgq_put(q, (void *)data, timeout);
 }
@@ -201,8 +201,8 @@ u32_t _handler_k_msgq_get(u32_t msgq_p, u32_t data, u32_t timeout,
 	struct k_msgq *q = (struct k_msgq *)msgq_p;
 	_SYSCALL_ARG3;
 
-	_SYSCALL_IS_OBJ(q, K_OBJ_MSGQ, 0, ssf);
-	_SYSCALL_MEMORY(data, q->msg_size, 1, ssf);
+	_SYSCALL_OBJ(q, K_OBJ_MSGQ, ssf);
+	_SYSCALL_MEMORY_WRITE(data, q->msg_size, ssf);
 
 	return _impl_k_msgq_get(q, (void *)data, timeout);
 }
@@ -232,7 +232,7 @@ u32_t _handler_k_msgq_purge(u32_t q, u32_t arg2, u32_t arg3,
 {
 	_SYSCALL_ARG1;
 
-	_SYSCALL_IS_OBJ(q, K_OBJ_MSGQ, 0, ssf);
+	_SYSCALL_OBJ(q, K_OBJ_MSGQ, ssf);
 
 	_impl_k_msgq_purge((struct k_msgq *)q);
 	return 0;
@@ -243,7 +243,7 @@ u32_t _handler_k_msgq_num_free_get(u32_t q, u32_t arg2, u32_t arg3, u32_t arg4,
 {
 	_SYSCALL_ARG1;
 
-	_SYSCALL_IS_OBJ(q, K_OBJ_MSGQ, 0, ssf);
+	_SYSCALL_OBJ(q, K_OBJ_MSGQ, ssf);
 
 	return _impl_k_msgq_num_free_get((struct k_msgq *)q);
 }
@@ -253,7 +253,7 @@ u32_t _handler_k_msgq_num_used_get(u32_t q, u32_t arg2, u32_t arg3, u32_t arg4,
 {
 	_SYSCALL_ARG1;
 
-	_SYSCALL_IS_OBJ(q, K_OBJ_MSGQ, 0, ssf);
+	_SYSCALL_OBJ(q, K_OBJ_MSGQ, ssf);
 
 	return _impl_k_msgq_num_used_get((struct k_msgq *)q);
 }

kernel/mutex.c

@@ -87,7 +87,7 @@ u32_t _handler_k_mutex_init(u32_t mutex, u32_t arg2, u32_t arg3,
 {
 	_SYSCALL_ARG1;
 
-	_SYSCALL_IS_OBJ(mutex, K_OBJ_MUTEX, 1, ssf);
+	_SYSCALL_OBJ_INIT(mutex, K_OBJ_MUTEX, ssf);
 	_impl_k_mutex_init((struct k_mutex *)mutex);
 
 	return 0;
@@ -208,7 +208,7 @@ u32_t _handler_k_mutex_lock(u32_t mutex, u32_t timeout, u32_t arg3,
 {
 	_SYSCALL_ARG2;
 
-	_SYSCALL_IS_OBJ(mutex, K_OBJ_MUTEX, 0, ssf);
+	_SYSCALL_OBJ(mutex, K_OBJ_MUTEX, ssf);
 	return _impl_k_mutex_lock((struct k_mutex *)mutex, (s32_t)timeout);
 }
 #endif
@@ -272,7 +272,7 @@ u32_t _handler_k_mutex_unlock(u32_t mutex, u32_t arg2, u32_t arg3,
 {
 	_SYSCALL_ARG1;
 
-	_SYSCALL_IS_OBJ(mutex, K_OBJ_MUTEX, 0, ssf);
+	_SYSCALL_OBJ(mutex, K_OBJ_MUTEX, ssf);
 	_impl_k_mutex_unlock((struct k_mutex *)mutex);
 	return 0;
 }

kernel/pipes.c

@@ -146,8 +146,8 @@ u32_t _handler_k_pipe_init(u32_t pipe, u32_t buffer, u32_t size,
 {
 	_SYSCALL_ARG3;
 
-	_SYSCALL_IS_OBJ(pipe, K_OBJ_PIPE, 1, ssf);
-	_SYSCALL_MEMORY(buffer, size, 1, ssf);
+	_SYSCALL_OBJ_INIT(pipe, K_OBJ_PIPE, ssf);
+	_SYSCALL_MEMORY_WRITE(buffer, size, ssf);
 
 	_impl_k_pipe_init((struct k_pipe *)pipe, (unsigned char *)buffer,
 			  size);
@@ -694,9 +694,9 @@ u32_t _handler_k_pipe_get(u32_t pipe, u32_t data, u32_t bytes_to_read,
 	size_t *bytes_read = (size_t *)bytes_read_p;
 	size_t min_xfer = (size_t)min_xfer_p;
 
-	_SYSCALL_IS_OBJ(pipe, K_OBJ_PIPE, 0, ssf);
-	_SYSCALL_MEMORY(bytes_read, sizeof(*bytes_read), 1, ssf);
-	_SYSCALL_MEMORY((void *)data, bytes_to_read, 1, ssf);
+	_SYSCALL_OBJ(pipe, K_OBJ_PIPE, ssf);
+	_SYSCALL_MEMORY_WRITE(bytes_read, sizeof(*bytes_read), ssf);
+	_SYSCALL_MEMORY_WRITE((void *)data, bytes_to_read, ssf);
 	_SYSCALL_VERIFY(min_xfer <= bytes_to_read, ssf);
 
 	return _impl_k_pipe_get((struct k_pipe *)pipe, (void *)data,
@@ -724,9 +724,9 @@ u32_t _handler_k_pipe_put(u32_t pipe, u32_t data, u32_t bytes_to_write,
 	size_t *bytes_written = (size_t *)bytes_written_p;
 	size_t min_xfer = (size_t)min_xfer_p;
 
-	_SYSCALL_IS_OBJ(pipe, K_OBJ_PIPE, 0, ssf);
-	_SYSCALL_MEMORY(bytes_written, sizeof(*bytes_written), 1, ssf);
-	_SYSCALL_MEMORY((void *)data, bytes_to_write, 0, ssf);
+	_SYSCALL_OBJ(pipe, K_OBJ_PIPE, ssf);
+	_SYSCALL_MEMORY_WRITE(bytes_written, sizeof(*bytes_written), ssf);
+	_SYSCALL_MEMORY_READ((void *)data, bytes_to_write,  ssf);
 	_SYSCALL_VERIFY(min_xfer <= bytes_to_write, ssf);
 
 	return _impl_k_pipe_put((struct k_pipe *)pipe, (void *)data,