fpsensor: validate args of EC_CMD_FP* commands

- More thorough validation of the arguments passed to host commands.
- Remove obsolete EC_CMD_FP_SENSOR_CONFIG host command.
- Some console commands can't be used on locked systems.

BRANCH=nocturne
BUG=b:116065496
TEST=enroll finger
TEST=unlock with finger
TEST=Capture
ectool --name=cros_fp fpmode capture vendor;
ectool --name=cros_fp waitevent 5 10000;
ectool --name=cros_fp fpframe raw > /tmp/fp.raw
/usr/local/opt/fpc/fputils.py --png /tmp/fp.raw
TEST=MQT
ectool --name=cros_fp fpmode capture qual;
ectool --name=cros_fp waitevent 5 10000;
ectool --name=cros_fp fpframe raw > /tmp/fp.raw;
/usr/local/opt/fpc/fputils.py --mqt /tmp/fp.raw
TEST=Reset_pixel
ectool --name=cros_fp fpmode capture test_reset;
ectool --name=cros_fp fpframe > /tmp/test_reset.pnm
TEST=Checkerboard
ectool --name=cros_fp fpmode capture pattern0;
ectool --name=cros_fp waitevent 5 500;
ectool --name=cros_fp fpframe > /tmp/pattern0.pnm
ectool --name=cros_fp fpmode capture pattern1;
ectool --name=cros_fp waitevent 5 500;
ectool --name=cros_fp fpframe > /tmp/pattern1.pnm

Change-Id: I3c9aa4749ffd77c73f8ca52cddbcc0e8ca6ae48c
Signed-off-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1239247
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Nicolas Boichat <drinkcat@chromium.org>

2 files changed, 65 insertions, 46 deletions

diff --git a/common/fpsensor.c b/common/fpsensor.c
index 2f20cd1b54..11d9d3257c 100644
--- a/common/fpsensor.c
+++ b/common/fpsensor.c
@@ -421,19 +421,31 @@ static int fp_command_passthru(struct host_cmd_handler_args *args)
 }
 DECLARE_HOST_COMMAND(EC_CMD_FP_PASSTHRU, fp_command_passthru, EC_VER_MASK(0));
 
-static int fp_command_sensor_config(struct host_cmd_handler_args *args)
+static int validate_fp_mode(const uint32_t mode)
 {
-	/* const struct ec_params_fp_sensor_config *p = args->params; */
+	uint32_t capture_type = FP_CAPTURE_TYPE(mode);
+	uint32_t algo_mode = mode & ~FP_MODE_CAPTURE_TYPE_MASK;
 
-	return EC_RES_UNAVAILABLE;
+	if (capture_type >= FP_CAPTURE_TYPE_MAX)
+		return EC_ERROR_INVAL;
+
+	if (algo_mode & ~FP_VALID_MODES)
+		return EC_ERROR_INVAL;
+
+	return EC_SUCCESS;
 }
-DECLARE_HOST_COMMAND(EC_CMD_FP_SENSOR_CONFIG, fp_command_sensor_config,
-		     EC_VER_MASK(0));
 
 static int fp_command_mode(struct host_cmd_handler_args *args)
 {
 	const struct ec_params_fp_mode *p = args->params;
 	struct ec_response_fp_mode *r = args->response;
+	int ret;
+
+	ret = validate_fp_mode(p->mode);
+	if (ret != EC_SUCCESS) {
+		CPRINTS("Invalid FP mode 0x%x", p->mode);
+		return EC_RES_INVALID_PARAM;
+	}
 
 	if (!(p->mode & FP_MODE_DONT_CHANGE)) {
 		sensor_mode = p->mode;
@@ -647,7 +659,7 @@ static int fp_command_stats(struct host_cmd_handler_args *args)
 	r->timestamps_invalid = timestamps_invalid;
 	r->template_matched = template_matched;
 
-	args->response_size = sizeof(struct ec_response_fp_stats);
+	args->response_size = sizeof(*r);
 	return EC_RES_SUCCESS;
 }
 DECLARE_HOST_COMMAND(EC_CMD_FP_STATS, fp_command_stats, EC_VER_MASK(0));
@@ -804,6 +816,9 @@ int command_fpcapture(int argc, char **argv)
 	uint32_t mode;
 	int rc;
 
+	if (system_is_locked())
+		return EC_RES_ACCESS_DENIED;
+
 	if (argc >= 2) {
 		char *e;
 
@@ -811,8 +826,8 @@ int command_fpcapture(int argc, char **argv)
 		if (*e || capture_type < 0)
 			return EC_ERROR_PARAM1;
 	}
-	mode = FP_MODE_CAPTURE | ((capture_type & FP_MODE_CAPTURE_TYPE_MASK)
-				  << FP_MODE_CAPTURE_TYPE_SHIFT);
+	mode = FP_MODE_CAPTURE | ((capture_type << FP_MODE_CAPTURE_TYPE_SHIFT)
+				  & FP_MODE_CAPTURE_TYPE_MASK);
 
 	rc = fp_console_action(mode);
 	if (rc == EC_SUCCESS)
@@ -830,6 +845,9 @@ int command_fpenroll(int argc, char **argv)
 	static const char * const enroll_str[] = {"OK", "Low Quality",
 						  "Immobile", "Low Coverage"};
 
+	if (system_is_locked())
+		return EC_RES_ACCESS_DENIED;
+
 	do {
 		int tries = 1000;
 
diff --git a/include/ec_commands.h b/include/ec_commands.h
index e4d0357eab..e6d02c8ee9 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -4922,51 +4922,17 @@ struct __ec_align2 ec_params_fp_passthru {
 	uint8_t data[];		/* Data to send */
 };
 
-/* Fingerprint sensor configuration command: prototyping ONLY */
-#define EC_CMD_FP_SENSOR_CONFIG 0x0401
-
-#define EC_FP_SENSOR_CONFIG_MAX_REGS 16
-
-struct __ec_align2 ec_params_fp_sensor_config {
-	uint8_t count;		/* Number of setup registers */
-	/*
-	 * the value to send to each of the 'count' setup registers
-	 * is stored in the 'data' array for 'len' bytes just after
-	 * the previous one.
-	 */
-	uint8_t len[EC_FP_SENSOR_CONFIG_MAX_REGS];
-	uint8_t data[];
-};
-
 /* Configure the Fingerprint MCU behavior */
 #define EC_CMD_FP_MODE 0x0402
 
 /* Put the sensor in its lowest power mode */
-#define FP_MODE_DEEPSLEEP     (1<<0)
+#define FP_MODE_DEEPSLEEP      (1<<0)
 /* Wait to see a finger on the sensor */
-#define FP_MODE_FINGER_DOWN   (1<<1)
+#define FP_MODE_FINGER_DOWN    (1<<1)
 /* Poll until the finger has left the sensor */
-#define FP_MODE_FINGER_UP     (1<<2)
+#define FP_MODE_FINGER_UP      (1<<2)
 /* Capture the current finger image */
-#define FP_MODE_CAPTURE       (1<<3)
-/* Capture types defined in bits [30..28] */
-#define FP_MODE_CAPTURE_TYPE_SHIFT 28
-#define FP_MODE_CAPTURE_TYPE_MASK  0x7
-/* Full blown vendor-defined capture (produces 'frame_size' bytes) */
-#define FP_CAPTURE_VENDOR_FORMAT 0
-/* Simple raw image capture (produces width x height x bpp bits) */
-#define FP_CAPTURE_SIMPLE_IMAGE  1
-/* Self test pattern (e.g. checkerboard) */
-#define FP_CAPTURE_PATTERN0      2
-/* Self test pattern (e.g. inverted checkerboard) */
-#define FP_CAPTURE_PATTERN1      3
-/* Capture for Quality test with fixed contrast */
-#define FP_CAPTURE_QUALITY_TEST  4
-/* Capture for pixel reset value test */
-#define FP_CAPTURE_RESET_TEST    5
-/* Extracts the capture type from the sensor 'mode' word */
-#define FP_CAPTURE_TYPE(mode) (((mode) >> FP_MODE_CAPTURE_TYPE_SHIFT) \
-					& FP_MODE_CAPTURE_TYPE_MASK)
+#define FP_MODE_CAPTURE        (1<<3)
 /* Finger enrollment session on-going */
 #define FP_MODE_ENROLL_SESSION (1<<4)
 /* Enroll the current finger image */
@@ -4976,6 +4942,41 @@ struct __ec_align2 ec_params_fp_sensor_config {
 /* special value: don't change anything just read back current mode */
 #define FP_MODE_DONT_CHANGE    (1<<31)
 
+#define FP_VALID_MODES (FP_MODE_DEEPSLEEP      | \
+			FP_MODE_FINGER_DOWN    | \
+			FP_MODE_FINGER_UP      | \
+			FP_MODE_CAPTURE        | \
+			FP_MODE_ENROLL_SESSION | \
+			FP_MODE_ENROLL_IMAGE   | \
+			FP_MODE_MATCH          | \
+			FP_MODE_DONT_CHANGE)
+
+/* Capture types defined in bits [30..28] */
+#define FP_MODE_CAPTURE_TYPE_SHIFT 28
+#define FP_MODE_CAPTURE_TYPE_MASK  (0x7 << FP_MODE_CAPTURE_TYPE_SHIFT)
+/*
+ * This enum must remain ordered, if you add new values you must ensure that
+ * FP_CAPTURE_TYPE_MAX is still the last one.
+ */
+enum fp_capture_type {
+	/* Full blown vendor-defined capture (produces 'frame_size' bytes) */
+	FP_CAPTURE_VENDOR_FORMAT = 0,
+	/* Simple raw image capture (produces width x height x bpp bits) */
+	FP_CAPTURE_SIMPLE_IMAGE = 1,
+	/* Self test pattern (e.g. checkerboard) */
+	FP_CAPTURE_PATTERN0 = 2,
+	/* Self test pattern (e.g. inverted checkerboard) */
+	FP_CAPTURE_PATTERN1 = 3,
+	/* Capture for Quality test with fixed contrast */
+	FP_CAPTURE_QUALITY_TEST = 4,
+	/* Capture for pixel reset value test */
+	FP_CAPTURE_RESET_TEST = 5,
+	FP_CAPTURE_TYPE_MAX,
+};
+/* Extracts the capture type from the sensor 'mode' word */
+#define FP_CAPTURE_TYPE(mode) (((mode) & FP_MODE_CAPTURE_TYPE_MASK) \
+				       >> FP_MODE_CAPTURE_TYPE_SHIFT)
+
 struct __ec_align4 ec_params_fp_mode {
 	uint32_t mode; /* as defined by FP_MODE_ constants */
 };