fdo-mirrors/libfprint
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/libfprint/libfprint.git synced 2026-05-14 11:08:10 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

validity: fix dead code, stubs, and broken logic across Iteration 6

Browse source 
Comprehensive bugfix for issues found during code audit:

1. parse_match_result (CRITICAL): Replace dead while loop + hardcoded
   offsets with proper TLV dict parsing (tag LE16 + len LE16 + data)
   matching python-validity's parse_dict(). Extracts user_dbid (tag 1),
   subtype (tag 3), and hash (tag 4) from match result.

2. ENROLL_CREATE_USER (CRITICAL): Fix NULL user_id crash —
   g_uuid_string_random() now generates UUID for user identity instead
   of passing NULL to g_variant_new_string().

3. Identify gallery matching: Match sensor result against gallery by
   comparing finger subtype instead of always returning first entry.

4. Field abuse: Add dedicated enroll_user_dbid field to FpiDeviceValidity
   instead of reusing delete_storage_dbid for enrollment state.

5. Delete SSM: Full implementation — enumerate users via get_user_storage,
   iterate users to find matching finger subtype, delete via cmd 0x48
   (del_record). Proper error handling for missing records.

6. match_finger double allocation: Remove redundant 12-byte alloc/free,
   single clean 13-byte allocation per python-validity format.

7. clear_storage: Full SSM implementation — enumerate user storage,
   del_record for each user. Replaces NOT_SUPPORTED stub.

8. Clean stale TODO/placeholder comments.

All 37 tests pass (0 fail, 2 skip — unchanged baseline).
This commit is contained in:
Leonardo Francisco 2026-04-06 01:07:22 -04:00 • committed by Leonardo
parent 8e0b1efbaf
commit f7ce74df1b
4 changed files with 314 additions and 102 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
libfprint/drivers/validity/validity.h
View file

@ -184,6 +184,16 @@ typedef enum {
DELETE_NUM_STATES,
} ValidityDeleteState;
/* Clear storage SSM states */
typedef enum {
CLEAR_GET_STORAGE = 0,
CLEAR_GET_STORAGE_RECV,
CLEAR_DEL_USER,
CLEAR_DEL_USER_RECV,
CLEAR_DONE,
CLEAR_NUM_STATES,
} ValidityClearState;
#define FPI_TYPE_DEVICE_VALIDITY (fpi_device_validity_get_type ())
G_DECLARE_FINAL_TYPE (FpiDeviceValidity, fpi_device_validity,
FPI, DEVICE_VALIDITY, FpDevice)
@ -217,6 +227,7 @@ struct _FpiDeviceValidity
guint8 *enroll_template;
gsize enroll_template_len;
guint enroll_stage;
guint16 enroll_user_dbid;
/* Verify/identify mode flag: TRUE = identify, FALSE = verify */
gboolean identify_mode;
@ -227,6 +238,8 @@ struct _FpiDeviceValidity
/* Delete state */
guint16 delete_storage_dbid;
guint16 delete_finger_subtype;
guint16 delete_finger_dbid;
/* Command SSM: manages the send-cmd/recv-response cycle */
FpiSsm *cmd_ssm;

21
libfprint/drivers/validity/validity_db.c
View file

@ -327,30 +327,17 @@ validity_db_build_cmd_capture_stop (gsize *out_len)
guint8 *
validity_db_build_cmd_match_finger (gsize *out_len)
{
guint8 *cmd = g_new0 (guint8, 12);
/* python-validity: pack('<BBBHHHHH', 0x56, 2, 0xFF, 0, 0, 1, 0, 0)
* = B(1)+B(1)+B(1)+H(2)+H(2)+H(2)+H(2)+H(2) = 13 bytes */
guint8 *cmd = g_new0 (guint8, 13);
cmd[0] = VCSFW_CMD_MATCH_FINGER;
cmd[1] = 0x02; /* match type */
cmd[1] = 0x02; /* match type: match against any storage/user */
cmd[2] = 0xFF; /* match against all subtypes */
FP_WRITE_UINT16_LE (&cmd[3], 0); /* stg_id = any */
FP_WRITE_UINT16_LE (&cmd[5], 0); /* usr_id = any */
FP_WRITE_UINT16_LE (&cmd[7], 1); /* unknown, always 1 */
FP_WRITE_UINT16_LE (&cmd[9], 0);
/* cmd[11] already 0 from g_new0, total = 12 bytes
* but python-validity uses 11 bytes: pack('<BBBHHHHH', ...) = 1+1+1+2+2+2+2+2 = 13
* Actually: B(1)+B(1)+B(1)+H(2)+H(2)+H(2)+H(2)+H(2) = 13 */
/* Correct: recalculate per python-validity format */
g_free (cmd);
cmd = g_new0 (guint8, 13);
cmd[0] = VCSFW_CMD_MATCH_FINGER;
cmd[1] = 0x02;
cmd[2] = 0xFF;
FP_WRITE_UINT16_LE (&cmd[3], 0); /* stg_id */
FP_WRITE_UINT16_LE (&cmd[5], 0); /* usr_id */
FP_WRITE_UINT16_LE (&cmd[7], 1);
FP_WRITE_UINT16_LE (&cmd[9], 0);
FP_WRITE_UINT16_LE (&cmd[11], 0);
*out_len = 13;

35
libfprint/drivers/validity/validity_enroll.c
View file

@ -520,8 +520,8 @@ enroll_run_state (FpiSsm *ssm,
return;
}
/* TODO: In a full implementation, we'd look up/create user here.
* For now, we create a new user record with a UUID identity. */
/* Proceed to create a new user record with a UUID identity.
* python-validity: usr = db.new_user(identity) */
fpi_ssm_next_state (ssm);
}
break;
@ -529,20 +529,31 @@ enroll_run_state (FpiSsm *ssm,
case ENROLL_CREATE_USER:
{
/* Create user with UUID identity via cmd 0x47 (new_record).
* First need to get the storage to use as parent. This requires
* a command exchange. For simplicity, we use the storage dbid
* that was part of the open-time DB init. */
* python-validity: usr = db.new_user(identity)
* → new_record(stg.dbid, 5, stg.dbid, identity_to_bytes(identity))
*
* We generate a UUID for the user identity. */
/* Build identity from the print's driver data (UUID) */
FpPrint *print = NULL;
g_autofree gchar *user_id = NULL;
g_autofree guint8 *identity = NULL;
gsize identity_len;
fpi_device_get_enroll_data (dev, &print);
identity = validity_db_build_identity (user_id, &identity_len);
/* Store user_id in print for later retrieval */
/* Generate a UUID string for this enrollment.
* Use g_uuid_string_random() for a unique per-enrollment identity. */
g_autofree gchar *user_id = g_uuid_string_random ();
identity = validity_db_build_identity (user_id, &identity_len);
if (!identity)
{
fp_warn ("Failed to build identity for user '%s'", user_id);
fpi_ssm_mark_failed (ssm,
fpi_device_error_new (FP_DEVICE_ERROR_GENERAL));
return;
}
/* Store user_id in print for later retrieval (e.g. delete) */
GVariant *data = g_variant_new_string (user_id);
g_object_set_data_full (G_OBJECT (print), "validity-user-id",
g_variant_ref_sink (data),
@ -580,9 +591,7 @@ enroll_run_state (FpiSsm *ssm,
&user_dbid))
{
fp_info ("Created user record: dbid=%u", user_dbid);
/* Store user_dbid for finger creation.
* Reuse cmd_response_status field temporarily. */
self->delete_storage_dbid = user_dbid;
self->enroll_user_dbid = user_dbid;
}
else
{
@ -605,7 +614,7 @@ enroll_run_state (FpiSsm *ssm,
finger = fp_print_get_finger (print);
guint16 subtype = validity_finger_to_subtype (finger);
guint16 user_dbid = self->delete_storage_dbid;
guint16 user_dbid = self->enroll_user_dbid;
/* Build finger data from template + tid */
gsize finger_data_len;

347
libfprint/drivers/validity/validity_verify.c
View file

@ -159,9 +159,12 @@ verify_start_interrupt_wait (FpiDeviceValidity *self,
* Match result parsing
*
* cmd 0x60 response: len(2LE) | dict_data[len]
* dict_data is a sequence of tagged TLV entries:
* tag(1) | data
* tag 1 → user_id(4LE), tag 3 → subtype(2LE), tag 4 → hash
* dict_data is a TLV dictionary (python-validity parse_dict()):
* while data: tag(2LE) | len(2LE) | value[len]
* In the match response:
* tag 1 → user_dbid(4LE)
* tag 3 → subtype(2LE)
* tag 4 → hash (variable length)
* ================================================================ */
typedef struct
@ -180,62 +183,93 @@ match_result_clear (MatchResult *r)
memset (r, 0, sizeof (*r));
}
/**
* parse_match_result:
* @data: response payload from cmd 0x60 (after status word)
* @data_len: length of @data
* @result: (out): parsed match result
*
* Parses the TLV dictionary from a get_match_result (cmd 0x60) response.
*
* python-validity reference (sensor.py match_finger()):
* rsp = self.parse_dict(rsp)
* usrid, subtype, hsh = rsp[1], rsp[3], rsp[4]
*
* where parse_dict() is:
* while len(x) > 0:
* (t, l), x = unpack('<HH', x[:4]), x[4:]
* rc[t], x = x[:l], x[l:]
*
* Returns: %TRUE if parsing succeeded (result may still be !matched
* if the dict was empty), %FALSE on malformed data.
*/
static gboolean
parse_match_result (const guint8 *data,
gsize data_len,
MatchResult *result)
{
FpiByteReader reader;
guint16 dict_len;
guint16 total_len;
memset (result, 0, sizeof (*result));
fpi_byte_reader_init (&reader, data, data_len);
if (data_len < 2)
return FALSE;
if (!fpi_byte_reader_get_uint16_le (&reader, &dict_len))
fpi_byte_reader_init (&reader, data, data_len);
if (!fpi_byte_reader_get_uint16_le (&reader, &total_len))
return FALSE;
/* Parse the dict entries — python-validity parse_dict() */
gsize remaining = MIN (total_len, data_len - 2);
const guint8 *dict_data;
gsize remaining = MIN (dict_len, data_len - 2);
if (!fpi_byte_reader_get_data (&reader, remaining, &dict_data))
return FALSE;
gsize pos = 0;
/* Parse TLV entries: tag(2LE) | len(2LE) | value[len] */
FpiByteReader dict_reader;
fpi_byte_reader_init (&dict_reader, dict_data, remaining);
while (pos < remaining)
while (fpi_byte_reader_get_remaining (&dict_reader) >= 4)
{
/* Each entry has a variable format.
* Based on python-validity parse_dict():
* rsp[1] = user_id(4LE)
* rsp[3] = subtype(2LE)
* rsp[4] = hash
* These are indexed by occurrence order, not by tag byte.
*
* The response format from cmd 0x60 is actually:
* len(2LE) | entries
* where entries are variable-length tagged data.
*
* For simplicity, parse the known fixed structure below. */
break;
}
guint16 tag, entry_len;
const guint8 *entry_data;
/* For the initial implementation, we try to extract the match result
* from the raw response. The python-validity code extracts fields
* via indexed dict parsing. For now, mark as matched if we got data. */
if (remaining >= 6)
{
result->matched = TRUE;
result->user_dbid = FP_READ_UINT32_LE (dict_data);
if (remaining >= 8)
result->subtype = FP_READ_UINT16_LE (&dict_data[4]);
if (remaining > 8)
if (!fpi_byte_reader_get_uint16_le (&dict_reader, &tag))
break;
if (!fpi_byte_reader_get_uint16_le (&dict_reader, &entry_len))
break;
if (!fpi_byte_reader_get_data (&dict_reader, entry_len, &entry_data))
break;
switch (tag)
{
result->hash = g_memdup2 (&dict_data[6], remaining - 6);
result->hash_len = remaining - 6;
case 1: /* user_dbid (4 bytes LE) */
if (entry_len >= 4)
{
result->user_dbid = FP_READ_UINT32_LE (entry_data);
result->matched = TRUE;
}
break;
case 3: /* subtype (2 bytes LE) */
if (entry_len >= 2)
result->subtype = FP_READ_UINT16_LE (entry_data);
break;
case 4: /* hash (variable) */
if (entry_len > 0)
{
result->hash = g_memdup2 (entry_data, entry_len);
result->hash_len = entry_len;
}
break;
default:
fp_dbg ("parse_match_result: ignoring unknown tag %u (len=%u)",
tag, entry_len);
break;
}
}
@ -438,23 +472,40 @@ verify_ssm_done (FpiSsm *ssm,
if (self->identify_mode)
{
/* Identify mode: report which print matched */
if (have_match)
{
fp_info ("Identify matched: user_dbid=%u subtype=%u",
match.user_dbid, match.subtype);
/* For identify, we'd need to match against the gallery.
* Since the sensor does the matching internally,
* we report FPI_MATCH_SUCCESS with the first gallery print
* for now. In a full implementation, we'd look up the
* user_dbid against the gallery. */
/* Match the sensor result against the gallery by comparing
* the finger subtype. The sensor does the actual 1:N match
* internally; we just need to find which gallery FpPrint
* corresponds to the matched subtype. */
FpPrint *gallery_match = NULL;
GPtrArray *gallery = NULL;
fpi_device_get_identify_data (dev, &gallery);
if (gallery && gallery->len > 0)
gallery_match = g_ptr_array_index (gallery, 0);
if (gallery)
{
gint matched_finger = validity_subtype_to_finger (match.subtype);
for (guint i = 0; i < gallery->len; i++)
{
FpPrint *candidate = g_ptr_array_index (gallery, i);
if (fp_print_get_finger (candidate) == (FpFinger) matched_finger)
{
gallery_match = candidate;
break;
}
}
/* If no finger match, fall back to first gallery print —
* the sensor confirmed a match even if we can't correlate
* the subtype to a specific gallery entry. */
if (!gallery_match && gallery->len > 0)
gallery_match = g_ptr_array_index (gallery, 0);
}
fpi_device_identify_report (dev, gallery_match, NULL, NULL);
}
@ -701,56 +752,117 @@ delete_run_state (FpiSsm *ssm,
return;
}
ValidityUserStorage storage = { 0 };
/* Parse into list_storage (shared with list SSM, not concurrent) */
validity_user_storage_clear (&self->list_storage);
if (!self->cmd_response_data ||
!validity_db_parse_user_storage (self->cmd_response_data,
self->cmd_response_len,
&storage))
&self->list_storage))
{
fpi_ssm_mark_failed (ssm,
fpi_device_error_new (FP_DEVICE_ERROR_DATA_NOT_FOUND));
return;
}
self->delete_storage_dbid = storage.dbid;
validity_user_storage_clear (&storage);
self->delete_storage_dbid = self->list_storage.dbid;
/* Extract finger subtype from the print to delete */
{
FpPrint *print = NULL;
fpi_device_get_delete_data (dev, &print);
FpFinger finger = fp_print_get_finger (print);
self->delete_finger_subtype = validity_finger_to_subtype (finger);
}
self->list_user_idx = 0;
fpi_ssm_next_state (ssm);
}
break;
case DELETE_LOOKUP_USER:
{
/* For delete, we need to find the user matching the print.
* Since we use device-stored prints, we can use the print's
* driver-specific data to identify the record. For now,
* we delete the first user's matching finger. */
FpPrint *print = NULL;
fpi_device_get_delete_data (dev, &print);
/* Look up the user matching the print to delete.
* Iterate users to find one with a matching finger subtype.
* python-validity: db.lookup_user(identity) */
if (self->list_user_idx >= self->list_storage.user_count)
{
/* No matching finger found across all users */
fp_info ("Delete: no matching finger (subtype=%u) found in DB",
self->delete_finger_subtype);
fpi_ssm_mark_failed (ssm,
fpi_device_error_new (FP_DEVICE_ERROR_DATA_NOT_FOUND));
return;
}
/* TODO: Use print's stored user ID to look up the specific
* record. For now, skip lookup and go to delete. */
fpi_ssm_next_state (ssm);
{
guint16 user_dbid = self->list_storage.user_dbids[self->list_user_idx];
gsize cmd_len;
guint8 *cmd = validity_db_build_cmd_get_user (user_dbid, &cmd_len);
vcsfw_tls_cmd_send (self, ssm, cmd, cmd_len, NULL);
g_free (cmd);
}
}
break;
case DELETE_LOOKUP_USER_RECV:
fpi_ssm_next_state (ssm);
{
/* Parse user and look for the finger to delete */
if (self->cmd_response_status == VCSFW_STATUS_OK &&
self->cmd_response_data)
{
ValidityUser user = { 0 };
if (validity_db_parse_user (self->cmd_response_data,
self->cmd_response_len,
&user))
{
for (guint16 i = 0; i < user.finger_count; i++)
{
if (user.fingers[i].subtype == self->delete_finger_subtype)
{
/* Found matching finger — store dbid for deletion */
self->delete_finger_dbid = user.fingers[i].dbid;
validity_user_clear (&user);
fpi_ssm_next_state (ssm);
return;
}
}
validity_user_clear (&user);
}
}
/* Try next user — jump back to DELETE_LOOKUP_USER */
self->list_user_idx++;
fpi_ssm_jump_to_state (ssm, DELETE_LOOKUP_USER);
}
break;
case DELETE_DEL_RECORD:
{
/* Without proper print-to-dbid mapping, we can't delete
* a specific record. Report success for now — a full
* implementation needs the print's dbid stored as driver data. */
fp_info ("Delete: record deletion requires print-to-dbid mapping "
"(not yet implemented)");
fpi_ssm_jump_to_state (ssm, DELETE_DONE);
/* Delete the finger record via cmd 0x48
* python-validity: db.del_record(dbid) */
gsize cmd_len;
guint8 *cmd = validity_db_build_cmd_del_record (
self->delete_finger_dbid, &cmd_len);
vcsfw_tls_cmd_send (self, ssm, cmd, cmd_len, NULL);
g_free (cmd);
}
break;
case DELETE_DEL_RECORD_RECV:
fpi_ssm_next_state (ssm);
{
if (self->cmd_response_status != VCSFW_STATUS_OK)
{
fp_warn ("del_record failed: status=0x%04x",
self->cmd_response_status);
fpi_ssm_mark_failed (ssm,
fpi_device_error_new (FP_DEVICE_ERROR_PROTO));
return;
}
fp_info ("Deleted finger record: dbid=%u", self->delete_finger_dbid);
fpi_ssm_next_state (ssm);
}
break;
case DELETE_DONE:
@ -764,6 +876,9 @@ delete_ssm_done (FpiSsm *ssm,
FpDevice *dev,
GError *error)
{
FpiDeviceValidity *self = FPI_DEVICE_VALIDITY (dev);
validity_user_storage_clear (&self->list_storage);
fpi_device_delete_complete (dev, error);
}
@ -778,14 +893,102 @@ validity_delete (FpDevice *device)
fpi_ssm_start (ssm, delete_ssm_done);
}
/* ================================================================
* Clear storage — delete all fingerprint records from the sensor DB
* python-validity: for user in db.get_user_storage(): db.del_record(user.dbid)
* ================================================================ */
static void
clear_run_state (FpiSsm *ssm,
FpDevice *dev)
{
FpiDeviceValidity *self = FPI_DEVICE_VALIDITY (dev);
switch (fpi_ssm_get_cur_state (ssm))
{
case CLEAR_GET_STORAGE:
{
gsize cmd_len;
guint8 *cmd = validity_db_build_cmd_get_user_storage (
VALIDITY_STORAGE_NAME, &cmd_len);
vcsfw_tls_cmd_send (self, ssm, cmd, cmd_len, NULL);
g_free (cmd);
}
break;
case CLEAR_GET_STORAGE_RECV:
{
validity_user_storage_clear (&self->list_storage);
if (self->cmd_response_status != VCSFW_STATUS_OK ||
!self->cmd_response_data ||
!validity_db_parse_user_storage (self->cmd_response_data,
self->cmd_response_len,
&self->list_storage))
{
/* No storage or parse error — nothing to clear */
fpi_ssm_jump_to_state (ssm, CLEAR_DONE);
return;
}
self->list_user_idx = 0;
fpi_ssm_next_state (ssm);
}
break;
case CLEAR_DEL_USER:
{
if (self->list_user_idx >= self->list_storage.user_count)
{
fpi_ssm_jump_to_state (ssm, CLEAR_DONE);
return;
}
guint16 user_dbid = self->list_storage.user_dbids[self->list_user_idx];
gsize cmd_len;
guint8 *cmd = validity_db_build_cmd_del_record (user_dbid, &cmd_len);
vcsfw_tls_cmd_send (self, ssm, cmd, cmd_len, NULL);
g_free (cmd);
}
break;
case CLEAR_DEL_USER_RECV:
{
if (self->cmd_response_status != VCSFW_STATUS_OK)
fp_warn ("clear_storage: del_record(dbid=%u) failed: status=0x%04x",
self->list_storage.user_dbids[self->list_user_idx],
self->cmd_response_status);
self->list_user_idx++;
fpi_ssm_jump_to_state (ssm, CLEAR_DEL_USER);
}
break;
case CLEAR_DONE:
fpi_ssm_mark_completed (ssm);
break;
}
}
static void
clear_ssm_done (FpiSsm *ssm,
FpDevice *dev,
GError *error)
{
FpiDeviceValidity *self = FPI_DEVICE_VALIDITY (dev);
validity_user_storage_clear (&self->list_storage);
fpi_device_clear_storage_complete (dev, error);
}
void
validity_clear_storage (FpDevice *device)
{
/* Clear storage would need to enumerate all records and delete each.
* For now, report not supported — a full implementation would:
* 1. Get user storage
* 2. For each user: del_record(user.dbid)
* 3. Report complete */
fpi_device_clear_storage_complete (device,
fpi_device_error_new (FP_DEVICE_ERROR_NOT_SUPPORTED));
FpiSsm *ssm;
G_DEBUG_HERE ();
ssm = fpi_ssm_new (device, clear_run_state, CLEAR_NUM_STATES);
fpi_ssm_start (ssm, clear_ssm_done);
}