diff --git a/libfprint/drivers/crfpmoc/crfpmoc.c b/libfprint/drivers/crfpmoc/crfpmoc.c index 8898d34d..92dfc6ef 100644 --- a/libfprint/drivers/crfpmoc/crfpmoc.c +++ b/libfprint/drivers/crfpmoc/crfpmoc.c @@ -35,21 +35,30 @@ struct _FpiDeviceCrfpMoc GCancellable *interrupt_cancellable; int fd; int emul_fd; + int poll_source; }; G_DEFINE_TYPE (FpiDeviceCrfpMoc, fpi_device_crfpmoc, FP_TYPE_DEVICE) -typedef struct crfpmoc_enroll_print +/* Data for the enroll state machine */ +struct crfpmoc_enroll_data { - FpPrint *print; - int stage; -} EnrollPrint; + FpPrint *print; /* print template to be completed and returned */ + int stage; /* stage of enrollment */ +}; static const FpIdEntry crfpmoc_id_table[] = { {.udev_types = FPI_DEVICE_UDEV_SUBTYPE_MISC, .misc_name = "cros_fp"}, {.udev_types = 0} }; +/* Define the variant data format and offsets for FpPrint */ +#define CRFPMOC_PRINT_DATA_VARIANT_FMT "(@ay@ayii)" +#define CRFPMOC_PRINT_DATA_VARIANT_PRINT_ID 0 +#define CRFPMOC_PRINT_DATA_VARIANT_TEMPLATE 1 +#define CRFPMOC_PRINT_DATA_VARIANT_MAX_OUTSIZE 2 +#define CRFPMOC_PRINT_DATA_VARIANT_MAX_TEMPLATES 3 + static const gchar *const crfpmoc_meanings[] = { "SUCCESS", "INVALID_COMMAND", @@ -91,6 +100,7 @@ get_print_data_descriptor (FpPrint *print, gint8 template) driver = fp_print_get_driver (print); dev_id = fp_print_get_device_id (print); + /* Caller is responsible for g_free() */ return g_strdup_printf ("%s/%s/%d", driver, dev_id, template); } @@ -116,6 +126,7 @@ crfpmoc_set_print_data (FpPrint *print, gint8 template_idx, fp_dbg ("template id %d, descr %p (%s)", template_idx, descr, descr ? descr : ""); + // TODO: technically this is a guint8, not gchar */ print_id_var = g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, descr, descr_len, sizeof (guchar)); @@ -129,7 +140,7 @@ crfpmoc_set_print_data (FpPrint *print, gint8 template_idx, fp_dbg ("EC max outsize: %d, max templates: %d, template %p, size %zd", ec_max_outsize, max_templates, template, template_size); - fpi_data = g_variant_new ("(@ay@ayii)", print_id_var, template_var, + fpi_data = g_variant_new (CRFPMOC_PRINT_DATA_VARIANT_FMT, print_id_var, template_var, ec_max_outsize, max_templates); g_object_set (print, "fpi-data", fpi_data, NULL); @@ -165,8 +176,9 @@ crfpmoc_get_print_data (FpPrint *print, guint8 **template, return; } - // print_id is var 0 - print_id_var = g_variant_get_child_value (fpi_data, 0); + print_id_var = + g_variant_get_child_value (fpi_data, + CRFPMOC_PRINT_DATA_VARIANT_PRINT_ID); if (print_id_var) { print_id = g_variant_get_fixed_array (print_id_var, &print_id_size, @@ -174,12 +186,16 @@ crfpmoc_get_print_data (FpPrint *print, guint8 **template, fp_dbg ("print id %.*s", (int) print_id_size, print_id); } - if ((template_var = g_variant_get_child_value (fpi_data, 1)) == NULL) + if ((template_var = + g_variant_get_child_value (fpi_data, + CRFPMOC_PRINT_DATA_VARIANT_TEMPLATE)) == NULL) fp_warn ("no template data in print!!"); if (ec_max_outsize) { - g_autoptr(GVariant) variant_ec_max = g_variant_get_child_value (fpi_data, 2); + g_autoptr(GVariant) variant_ec_max = + g_variant_get_child_value (fpi_data, + CRFPMOC_PRINT_DATA_VARIANT_MAX_OUTSIZE); if (variant_ec_max) { *ec_max_outsize = g_variant_get_int32 (variant_ec_max); @@ -194,7 +210,8 @@ crfpmoc_get_print_data (FpPrint *print, guint8 **template, if (max_templates) { g_autoptr(GVariant) variant_max_templates = - g_variant_get_child_value (fpi_data, 3); + g_variant_get_child_value (fpi_data, + CRFPMOC_PRINT_DATA_VARIANT_MAX_TEMPLATES); if (variant_max_templates) { *max_templates = g_variant_get_int32 (variant_max_templates); @@ -217,6 +234,7 @@ crfpmoc_get_print_data (FpPrint *print, guint8 **template, fp_dbg ("template data %p, size %zd", template_data, template_data_size); if (template_data && template_data_size > 0) { + /* Caller is responsible for g_free() */ if (template) *template = g_memdup2 (template_data, template_data_size); if (template_size) @@ -229,7 +247,7 @@ static void crfpmoc_umockdev_record (FpiDeviceCrfpMoc *self, int res, int cmd, void *arg) { - gchar *buffer; + g_autofree gchar *buffer = NULL; gsize size, bufsiz, len; struct crfpmoc_cros_ec_command_v2 *s_cmd = (struct crfpmoc_cros_ec_command_v2 *) arg; @@ -256,7 +274,6 @@ crfpmoc_umockdev_record (FpiDeviceCrfpMoc *self, int res, buffer[len++] = '\n'; if (write (self->emul_fd, buffer, len) != len) fp_dbg ("emulation trace write failed"); - g_free (buffer); break; default: @@ -331,7 +348,7 @@ crfpmoc_read_bytes (gint fd, GIOCondition condition, struct crfpmoc_ec_response_get_next_event_v1 buffer = {0}; if (fd != self->fd) - return FALSE; + goto one_shot_out; rv = read (fd, &buffer, sizeof (buffer)); @@ -340,19 +357,31 @@ crfpmoc_read_bytes (gint fd, GIOCondition condition, fp_warn ("Timeout waiting for MKBP event"); fpi_ssm_mark_failed (self->task_ssm, fpi_device_error_new (FP_DEVICE_ERROR_GENERAL)); - return FALSE; + goto one_shot_out; } else if (rv < 0) { fp_warn ("Error polling for MKBP event"); fpi_ssm_mark_failed (self->task_ssm, fpi_device_error_new (FP_DEVICE_ERROR_GENERAL)); - return FALSE; + goto one_shot_out; } fp_dbg ("MKBP event %d data", buffer.event_type); fpi_ssm_next_state (self->task_ssm); + +one_shot_out: + /* The call of this function with FALSE return will complete + * the poll source. No need to remove it on close or cancel. + */ + self->poll_source = 0; return FALSE; + +#ifdef LATER +rearm_out: + /* Keep the poll source active */ + return TRUE; +#endif } static void @@ -368,7 +397,7 @@ crfpmoc_ec_pollevent (FpiDeviceCrfpMoc *self, unsigned long mask) return; } - g_unix_fd_add (self->fd, G_IO_IN, crfpmoc_read_bytes, self); + self->poll_source = g_unix_fd_add (self->fd, G_IO_IN, crfpmoc_read_bytes, self); } static gboolean @@ -689,7 +718,8 @@ crfpmoc_fp_download_template (FpiDeviceCrfpMoc *self, { struct crfpmoc_ec_params_fp_template p; gsize stride, size; - guint8 *buffer = NULL, *ptr; + g_autofree guint8 *buffer = NULL; + guint8 *ptr; gboolean rv; gint cmdver = 1; gsize rsize = sizeof (*info); @@ -763,7 +793,7 @@ crfpmoc_fp_download_template (FpiDeviceCrfpMoc *self, fp_dbg ("crfpmoc_fp_download_template: done, %lu bytes read, sum %d", ptr - buffer, sum); - *out_buffer = buffer; + *out_buffer = g_steal_pointer (&buffer); return TRUE; } @@ -782,13 +812,17 @@ crfpmoc_fp_upload_template (FpiDeviceCrfpMoc *self, guint32 max_chunk; guint16 sum = 0; - max_chunk = CRFPMOC_EC_PROTO2_MAX_PARAM_SIZE - offsetof (struct crfpmoc_ec_params_fp_template, data) - 4; + max_chunk = CRFPMOC_EC_PROTO2_MAX_PARAM_SIZE - + offsetof (struct crfpmoc_ec_params_fp_template, data) - 4; while (template_size > 0) { tlen = MIN (max_chunk, template_size); struct_size = offsetof (struct crfpmoc_ec_params_fp_template, data) + tlen; + /* Note g_autofree does not detect overwrite so dont use it here and + * do the g_free manually + */ p = g_malloc0 (struct_size); p->offset = offset; @@ -807,10 +841,7 @@ crfpmoc_fp_upload_template (FpiDeviceCrfpMoc *self, p, struct_size, NULL, 0, error); - g_free (p); - p = NULL; - if (rv != EC_RES_SUCCESS) { g_prefix_error (error, @@ -876,7 +907,6 @@ crfpmoc_open (FpDevice *device) /* For testing we need to create the umockdev trace file ourselves * since there is no record support. There is playback support. */ - self->emul_fd = -1; if (g_strcmp0 (g_getenv ("FP_DEVICE_EMULATION"), "1") == 0) self->emul_fd = creat ("/tmp/crfpmoc.ioctl", 0777); @@ -905,6 +935,10 @@ crfpmoc_cancel (FpDevice *device) fp_dbg ("Cancel"); FpiDeviceCrfpMoc *self = FPI_DEVICE_CRFPMOC (device); + /* Cancel any poll source */ + if (self->poll_source) + g_source_remove (self->poll_source); + /* Cancel the running state machine, if any */ if (self->task_ssm != NULL) fpi_ssm_mark_failed (self->task_ssm, @@ -975,9 +1009,9 @@ handle_enroll_wait_finger (FpDevice *device, } static void -handle_enroll_sensor_check (FpiSsm *ssm, FpDevice *device, - FpiDeviceCrfpMoc *self, - EnrollPrint *enroll_print) +handle_enroll_sensor_check (FpiSsm *ssm, FpDevice *device, + FpiDeviceCrfpMoc *self, + struct crfpmoc_enroll_data *enroll_data) { GError *error = NULL; guint32 mode; @@ -1000,11 +1034,11 @@ handle_enroll_sensor_check (FpiSsm *ssm, FpDevice *device, { fpi_device_report_finger_status (device, FP_FINGER_STATUS_PRESENT); - enroll_print->stage++; - fp_info ("Partial capture successful (%d/%d).", enroll_print->stage, + enroll_data->stage++; + fp_info ("Partial capture successful (%d/%d).", enroll_data->stage, CRFPMOC_NR_ENROLL_STAGES); - fpi_device_enroll_progress (device, enroll_print->stage, - enroll_print->print, NULL); + fpi_device_enroll_progress (device, enroll_data->stage, + enroll_data->print, NULL); fpi_ssm_jump_to_state (ssm, ENROLL_SENSOR_ENROLL); } @@ -1019,7 +1053,7 @@ handle_enroll_sensor_check (FpiSsm *ssm, FpDevice *device, { fpi_device_report_finger_status (device, FP_FINGER_STATUS_PRESENT); - fpi_device_enroll_progress (device, enroll_print->stage, NULL, + fpi_device_enroll_progress (device, enroll_data->stage, NULL, fpi_device_retry_new_msg (FP_DEVICE_RETRY_GENERAL, "FP mode: (0x%x)", mode)); @@ -1030,29 +1064,29 @@ handle_enroll_sensor_check (FpiSsm *ssm, FpDevice *device, } static void -handle_enroll_commit (FpiSsm *ssm, FpDevice *device, - FpiDeviceCrfpMoc *self, - EnrollPrint *enroll_print) +handle_enroll_commit (FpiSsm *ssm, FpDevice *device, + FpiDeviceCrfpMoc *self, + struct crfpmoc_enroll_data *enroll_data) { GError *error = NULL; guint16 enrolled_templates = 0; - guint8 *template = NULL; + g_autofree guint8 *template = NULL; guint16 ec_max_outsize = 10; struct crfpmoc_ec_response_fp_info info; crfpmoc_cmd_fp_info (self, &enrolled_templates, &error); fp_dbg ("Number of enrolled templates is: %d", enrolled_templates); - g_autofree gchar *user_id = fpi_print_generate_user_id (enroll_print->print); + g_autofree gchar *user_id = fpi_print_generate_user_id (enroll_data->print); fp_dbg ("New fingerprint ID: %s", user_id); - g_object_set (enroll_print->print, "description", user_id, NULL); + g_object_set (enroll_data->print, "description", user_id, NULL); gboolean r = crfpmoc_ec_max_outsize (self, &ec_max_outsize, &error); if (!r) { fp_err ("Failed to get max outsize"); - crfpmoc_set_print_data (enroll_print->print, enrolled_templates - 1, + crfpmoc_set_print_data (enroll_data->print, enrolled_templates - 1, NULL, 0, ec_max_outsize, info.template_max); fpi_ssm_mark_failed (ssm, @@ -1068,7 +1102,7 @@ handle_enroll_commit (FpiSsm *ssm, FpDevice *device, if (!r) { fp_err ("Failed to download template"); - crfpmoc_set_print_data (enroll_print->print, enrolled_templates - 1, NULL, 0, + crfpmoc_set_print_data (enroll_data->print, enrolled_templates - 1, NULL, 0, ec_max_outsize, info.template_max); fpi_ssm_mark_failed (ssm, fpi_device_retry_new_msg (FP_DEVICE_RETRY_GENERAL, @@ -1076,15 +1110,13 @@ handle_enroll_commit (FpiSsm *ssm, FpDevice *device, } else { - crfpmoc_set_print_data (enroll_print->print, enrolled_templates - 1, + crfpmoc_set_print_data (enroll_data->print, enrolled_templates - 1, template, info.template_size, ec_max_outsize, info.template_max); } - g_free (template); - fp_info ("Enrollment was successful!"); - fpi_device_enroll_complete (device, g_object_ref (enroll_print->print), NULL); + fpi_device_enroll_complete (device, g_object_ref (enroll_data->print), NULL); fpi_ssm_mark_completed (ssm); } @@ -1093,7 +1125,7 @@ static void crfpmoc_enroll_run_state (FpiSsm *ssm, FpDevice *device) { FpiDeviceCrfpMoc *self = FPI_DEVICE_CRFPMOC (device); - EnrollPrint *enroll_print = fpi_ssm_get_data (ssm); + struct crfpmoc_enroll_data *enroll_data = fpi_ssm_get_data (ssm); switch (fpi_ssm_get_cur_state (ssm)) { @@ -1106,11 +1138,11 @@ crfpmoc_enroll_run_state (FpiSsm *ssm, FpDevice *device) break; case ENROLL_SENSOR_CHECK: - handle_enroll_sensor_check (ssm, device, self, enroll_print); + handle_enroll_sensor_check (ssm, device, self, enroll_data); break; case ENROLL_COMMIT: - handle_enroll_commit (ssm, device, self, enroll_print); + handle_enroll_commit (ssm, device, self, enroll_data); break; } } @@ -1122,7 +1154,8 @@ crfpmoc_enroll (FpDevice *device) GError *error = NULL; gboolean r; FpiDeviceCrfpMoc *self = FPI_DEVICE_CRFPMOC (device); - EnrollPrint *enroll_print = g_new0 (EnrollPrint, 1); + struct crfpmoc_enroll_data *enroll_data = + g_malloc0 (sizeof (struct crfpmoc_enroll_data)); r = crfpmoc_set_keys (self, &error); @@ -1132,12 +1165,13 @@ crfpmoc_enroll (FpDevice *device) return; } - fpi_device_get_enroll_data (device, &enroll_print->print); - enroll_print->stage = 0; + fpi_device_get_enroll_data (device, &enroll_data->print); + enroll_data->stage = 0; g_assert (self->task_ssm == NULL); - self->task_ssm = fpi_ssm_new (device, crfpmoc_enroll_run_state, ENROLL_STATES); - fpi_ssm_set_data (self->task_ssm, g_steal_pointer (&enroll_print), g_free); + self->task_ssm = + fpi_ssm_new (device, crfpmoc_enroll_run_state, ENROLL_STATES); + fpi_ssm_set_data (self->task_ssm, enroll_data, g_free); fpi_ssm_start (self->task_ssm, crfpmoc_task_ssm_done); } @@ -1149,7 +1183,7 @@ handle_verify_upload_template (FpiSsm *ssm, FpDevice *device, gboolean upload_successful = FALSE; guint16 fp_ec_max_outsize = 10; guint16 fp_max_templates = 10; - guint8 *template = NULL; + g_autofree guint8 *template = NULL; size_t template_size = 0; FpPrint *print = NULL; GPtrArray *prints = NULL; @@ -1192,8 +1226,6 @@ handle_verify_upload_template (FpiSsm *ssm, FpDevice *device, fpi_ssm_mark_failed (ssm, error); return; } - - g_free (template); } } } @@ -1228,7 +1260,6 @@ handle_verify_upload_template (FpiSsm *ssm, FpDevice *device, { fp_dbg ("uploaded template, %zd bytes", template_size); } - g_free (template); } else { @@ -1533,6 +1564,8 @@ static void fpi_device_crfpmoc_init (FpiDeviceCrfpMoc *self) { self->fd = -1; + self->emul_fd = -1; + self->poll_source = 0; } static void