Merge pull request #5 from qc-azarrabi/bugfix

Bugfix

libqcomtee/CMakeLists.txt

@@ -28,7 +28,7 @@ set(SRC
         src/objects/credentials_obj.c
 )
 
-add_library(qcomtee SHARED ${SRC})
+add_library(qcomtee ${SRC})
 
 # ''Library pkg-config file and version''.
 

libqcomtee/include/qcomtee_object.h

@@ -141,7 +141,9 @@ typedef enum {
 
 /**
  * @def QCOMTEE_OBJECT_PARAMS_MAX
- * @brief Size of parameter array pass to the dispatcher.
+ * @brief Size of parameter array minus one passed to the dispatcher.
+ *
+ * This is allocated on the stack; use your best judgment.
  */
 #define QCOMTEE_OBJECT_PARAMS_MAX 10
 
@@ -163,18 +165,14 @@ struct qcomtee_object_ops {
 	 * @param[in] object Object being invoked by QTEE.
 	 * @param[in] op Operation requested by QTEE.
 	 * @param[in,out] params Parameter array for the invocation.
-	 * @param[in,out] num Number of parameters in the params array.
-	 *
-	 * The size of the parameter array is always %ref QCOMTEE_OBJECT_PARAMS_MAX.
-	 * However, num specifies the number of valid entries upon entry and
-	 * return from the dispatcher.
+	 * @param[in] num Number of parameters in the params array.
 	 *
 	 * @return On success returns @ref QCOMTEE_OK;
 	 *         Otherwise a number; see @ref qcomtee_errno.h. 
 	 */
 	qcomtee_result_t (*dispatch)(struct qcomtee_object *object,
 				     qcomtee_op_t op,
-				     struct qcomtee_param *params, int *num);
+				     struct qcomtee_param *params, int num);
 
 	/**
 	 * @brief Notify the object of the transport state.

libqcomtee/src/objects/credentials_obj.c

@@ -89,29 +89,40 @@ static void qcomtee_object_credentials_release(struct qcomtee_object *object)
 	free(qcomtee_cred);
 }
 
+#define MIN_SIZE_T(a, b) ((a) < (b) ? (a) : (b))
+
 static qcomtee_result_t
 qcomtee_object_credentials_dispatch(struct qcomtee_object *object,
 				    qcomtee_op_t op,
-				    struct qcomtee_param *params, int *num)
+				    struct qcomtee_param *params, int num)
 {
 	struct qcomtee_credentials *qcomtee_cred = CREDENTIALS(object);
 
 	switch (op) {
 	case IIO_OP_GET_LENGTH:
-		params[0].attr = QCOMTEE_UBUF_OUTPUT;
+		/* Expect one argument: ouput buffer. */
+		if (num != 1 || params[0].attr != QCOMTEE_UBUF_OUTPUT)
+			return QCOMTEE_ERROR_INVALID;
+
 		params[0].ubuf.addr = &qcomtee_cred->ubuf.size;
 		params[0].ubuf.size = sizeof(qcomtee_cred->ubuf.size);
 
 		break;
 	case IIO_OP_READ_AT_OFFSET: {
-		uint64_t offset = *((uint64_t *)(params[0].ubuf.addr));
+		uint64_t offset;
 
+		/* Expect two arguments: input and outout buffer. */
+		if (num != 2 || params[0].attr != QCOMTEE_UBUF_INPUT ||
+		    params[1].attr != QCOMTEE_UBUF_OUTPUT)
+			return QCOMTEE_ERROR_INVALID;
+
+		offset = *((uint64_t *)(params[0].ubuf.addr));
 		if (offset >= qcomtee_cred->ubuf.size)
 			return QCOMTEE_ERROR_INVALID;
 
-		params[0].attr = QCOMTEE_UBUF_OUTPUT;
-		params[0].ubuf.addr = qcomtee_cred->ubuf.addr + offset;
-		params[0].ubuf.size = qcomtee_cred->ubuf.size - offset;
+		params[1].ubuf.addr = qcomtee_cred->ubuf.addr + offset;
+		params[1].ubuf.size = MIN_SIZE_T(
+			qcomtee_cred->ubuf.size - offset, params[1].ubuf.size);
 
 		break;
 	}
@@ -120,8 +131,6 @@ qcomtee_object_credentials_dispatch(struct qcomtee_object *object,
 		return QCOMTEE_ERROR_INVALID;
 	}
 
-	*num = 1; /* Return one parameter. */
-
 	return QCOMTEE_OK;
 }
 

libqcomtee/src/qcomtee_object.c

@@ -25,15 +25,6 @@
  */
 #define DISP_BUFFER 1024
 
-/**
- * @def DISP_PARAMS_MAX
- * @brief Maximum number of parameters that can fit into the buffer of
- *        size @ref DISP_BUFFER.
- *
- * Plus one for the meta parameters.
- */
-#define DISP_PARAMS_MAX (QCOMTEE_OBJECT_PARAMS_MAX + 1)
-
 /**
  * @brief Initialize an object.
  * @param object Object to initialize.
@@ -391,7 +382,7 @@ static int qcomtee_object_param_to_tee_param(struct tee_ioctl_param *tee_param,
 	struct qcomtee_object *object = param->object;
 
 	/* It expects caller to set tee_param attribute. */
-	if (tee_param->attr != TEE_IOCTL_PARAM_ATTR_TYPE_OBJREF_INPUT ||
+	if (tee_param->attr != TEE_IOCTL_PARAM_ATTR_TYPE_OBJREF_INPUT &&
 	    tee_param->attr != TEE_IOCTL_PARAM_ATTR_TYPE_OBJREF_OUTPUT)
 		return -1;
 
@@ -606,9 +597,14 @@ static int qcomtee_object_cb_marshal_in(struct qcomtee_param *params,
 	for (i = 0; i < num_params; i++) {
 		switch (tee_params[i].attr) {
 		case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT:
-			params[i].attr = QCOMTEE_UBUF_INPUT;
+		case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
 			params[i].ubuf.addr = (void *)tee_params[i].a;
 			params[i].ubuf.size = (size_t)tee_params[i].b;
+			params[i].attr =
+				(tee_params[i].attr ==
+				 TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT) ?
+					QCOMTEE_UBUF_INPUT :
+					QCOMTEE_UBUF_OUTPUT;
 
 			break;
 		case TEE_IOCTL_PARAM_ATTR_TYPE_OBJREF_INPUT:
@@ -619,10 +615,13 @@ static int qcomtee_object_cb_marshal_in(struct qcomtee_param *params,
 				failed = 1;
 
 			break;
-		case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT:
+
 		case TEE_IOCTL_PARAM_ATTR_TYPE_OBJREF_OUTPUT:
-		default:
-			failed = 1;
+			params[i].attr = QCOMTEE_OBJREF_INPUT;
+
+			break;
+		default: /* NEVER GET HERE! */
+			break;
 		}
 	}
 
@@ -656,14 +655,18 @@ static int qcomtee_object_cb_marshal_out(struct tee_ioctl_param *tee_params,
 	for (i = 0; i < num_params; i++) {
 		switch (params[i].attr) {
 		case QCOMTEE_UBUF_OUTPUT:
-			tee_params[i].attr =
-				TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT;
+			if (tee_params[i].attr !=
+			    TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT)
+				return -1;
+
 			tee_params[i].a = (uintptr_t)params[i].ubuf.addr;
 			tee_params[i].b = params[i].ubuf.size;
 			break;
 		case QCOMTEE_OBJREF_OUTPUT:
-			tee_params[i].attr =
-				TEE_IOCTL_PARAM_ATTR_TYPE_OBJREF_OUTPUT;
+			if (tee_params[i].attr !=
+			    TEE_IOCTL_PARAM_ATTR_TYPE_OBJREF_OUTPUT)
+				return -1;
+
 			if (qcomtee_object_param_to_tee_param(&tee_params[i],
 							      &params[i], root))
 				return -1;
@@ -672,7 +675,7 @@ static int qcomtee_object_cb_marshal_out(struct tee_ioctl_param *tee_params,
 		case QCOMTEE_UBUF_INPUT:
 		case QCOMTEE_OBJREF_INPUT:
 		default:
-			return -1;
+			break;
 		}
 	}
 
@@ -690,15 +693,22 @@ union tee_ioctl_arg {
 	struct tee_iocl_supp_recv_arg recv; /**< TEE_IOC_SUPPL_RECV. */
 };
 
-#define TEE_IOCTL_ARG_SEND_INIT(arg, r, n)    \
-	do {                                  \
-		(arg)->send.num_params = (n); \
-		(arg)->send.ret = (r);        \
+#define TEE_IOCTL_ARG_SEND_INIT(arg, r, n)        \
+	do {                                      \
+		/* +1 for the output meta. */     \
+		(arg)->send.num_params = (n) + 1; \
+		(arg)->send.ret = (r);            \
 	} while (0)
 
-#define qcomtee_arg_alloca(np)               \
-	alloca(sizeof(union tee_ioctl_arg) + \
-	       (np) * sizeof(struct tee_ioctl_param))
+#define qcomtee_arg_alloca(np)                                     \
+	({                                                         \
+		size_t sz = sizeof(union tee_ioctl_arg) +          \
+			    (np) * sizeof(struct tee_ioctl_param); \
+		memset(alloca(sz), 0, sz);                         \
+	})
+
+/* Plus one for the meta parameters. */
+#define DISP_PARAMS_MAX (QCOMTEE_OBJECT_PARAMS_MAX + 1)
 
 /* Direct object invocation. */
 int qcomtee_object_invoke(struct qcomtee_object *object, qcomtee_op_t op,
@@ -711,7 +721,7 @@ int qcomtee_object_invoke(struct qcomtee_object *object, qcomtee_op_t op,
 	union tee_ioctl_arg *arg;
 
 	/* Use can only invoke QTEE object ot root object. */
-	if (object->object_type != QCOMTEE_OBJECT_TYPE_ROOT ||
+	if (object->object_type != QCOMTEE_OBJECT_TYPE_ROOT &&
 	    object->object_type != QCOMTEE_OBJECT_TYPE_TEE)
 		return -1;
 	/* QTEE does not support more than 64 parameter. */
@@ -736,7 +746,7 @@ int qcomtee_object_invoke(struct qcomtee_object *object, qcomtee_op_t op,
 	if (qcomtee_object_marshal_in(tee_params, params, num_params, root))
 		return -1;
 
-	if (ioctl(ROOT_OBJECT(root)->fd, TEE_IOC_OBJECT_INVOKE, buf_data))
+	if (ioctl(ROOT_OBJECT(root)->fd, TEE_IOC_OBJECT_INVOKE, &buf_data))
 		return -1;
 
 	*result = arg->invoke.ret;
@@ -812,7 +822,7 @@ static int qcomtee_object_dispatch_request(struct qcomtee_object *object,
 		return WITHOUT_RESPONSE;
 
 	default:
-		res = object->ops->dispatch(object, op, params, &np);
+		res = object->ops->dispatch(object, op, params, np);
 		if (res != QCOMTEE_OK) {
 			TEE_IOCTL_ARG_SEND_INIT(arg, res, 0);
 			return WITH_RESPONSE_NO_NOTIFY;

tests/common.c

@@ -14,7 +14,7 @@ static int tee_call(int fd, int op, struct tee_ioctl_buf_data *buf_data)
 	ret = ioctl(fd, op, buf_data);
 	pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, NULL);
 	if (ret)
-		PRINT("ioctl: %s\n", strerror(errno));
+		PRINT("ioctl: %s, %d\n", strerror(errno), op);
 
 	return ret;
 }
@@ -24,11 +24,13 @@ static void *test_supplicant_worker(void *arg)
 {
 	while (1) {
 		pthread_testcancel();
-		qcomtee_object_process_one((struct qcomtee_object *)arg,
-					   tee_call);
+		if (qcomtee_object_process_one((struct qcomtee_object *)arg,
+					       tee_call)) {
+			PRINT("qcomtee_object_process_one.\n");
+			break;
+		}
 	}
 
-	PRINT("WE SHOULE NOT GET HERE!\n");
 	return NULL;
 }