Extend the 2541:fa03 storage-error fixture to enroll the same finger
twice and assert FP_DEVICE_ERROR_DATA_DUPLICATE, reusing the device
session the delete test already opened rather than starting a second
one. Compact redundant polling transactions while preserving the
recorded protocol state changes.
When the sensor rejects an enrollment because the finger is already
present in on-chip storage, fp_check_duplicate_cb() raises
FP_DEVICE_ERROR_PROTO. That error class means "protocol error with the
device", which is not what happened; the device answered correctly.
The practical effect is that fprintd cannot classify the failure.
It already maps FP_DEVICE_ERROR_DATA_DUPLICATE to its "enroll-duplicate"
result, so with PROTO the user is shown "enroll-unknown-error" instead of
being told to try a different finger.
Use FP_DEVICE_ERROR_DATA_DUPLICATE, which exists for exactly this case.
Reproduced on a Realtek 2541:fa03 (Minisforum AI X1 Pro) by enrolling the
same finger twice:
[realtek] SSM Enroll failed in state 5 with error:
Current fingerprint is duplicate!
Device reported enroll completion (print: (nil),
error: [FP_DEVICE_ERROR_PROTO] Current fingerprint is duplicate!)
fprintd: enroll_cb: result enroll-unknown-error
bf91b0ed ("tests/build: Allow to define per-test parameters via a dict")
turned drivers_tests into a dict and updated the loop that defines the real
tests, but the fallback that only adds the skipping ones still asks for a
single iteration variable. Configuring with -Dintrospection=false stops right
there:
tests/meson.build:295:25: ERROR: Foreach expects exactly 2 variables for
iterating over objects of type dict
Hit it with meson 1.11 while building a single driver without introspection.
Deleting a print whose template is not in the sensor's storage makes the
driver fail its task SSM, which is the path that handed an already-freed
GError to fpi_device_delete_complete(). Reading the reported error is
therefore what the test is for.
No enrollment is involved: the test deserializes a stored realtek print
whose template is deliberately not on the device, so the recording needs
no finger presses and the capture stays at 90 packets.
Without the previous commit the test dies rather than fails:
umockdev-run ... died with <Signals.SIGSEGV: 11>
1/1 drivers+custom - libfprint:realtek-storage-errors FAIL
The duplicate-enrollment path reaches the same bug through
fp_enroll_ssm_done(), but capturing it needs two full enrollments and
some 1.07M packets of polling traffic, so it is left out here.
fp_verify_ssm_done(), fp_enroll_ssm_done(), fp_init_ssm_done() and
fp_delete_ssm_done() each overwrite the GError they are given:
if (fpi_ssm_get_error (ssm))
error = fpi_ssm_get_error (ssm);
An SSM completion callback is handed an owned copy of the error --
fpi-ssm.c takes a g_error_copy() before invoking the callback -- whereas
fpi_ssm_get_error() is documented as (transfer none), and the machine's
own error is freed by the fpi_ssm_free() that immediately follows the
callback.
Since every fpi_device_*_complete() takes the error as (transfer full),
these handlers leak the copy they own and hand the consumer a pointer
that is freed moments later. The consumer is then left reading a
dangling GError. For fprintd that is fatal: it logs error->message and
dies with a general protection fault inside strlen(), taking the
session's authentication daemon with it.
kernel: traps: fprintd[67901] general protection fault
ip:7fcef3b6429c sp:7ffd5f4128a8 error:0 in libc.so.6
#0 __strlen_evex ()
#3 g_vasprintf () at ../glib/gprintf.c:342
#5 g_strdup_vprintf () at ../glib/gstrfuncs.c:515
#9 delete_enrolled_fingers (user=... "ge-org",
finger=FP_FINGER_RIGHT_INDEX)
at ../src/device.c:2420
local_error = 0x563c2628bcb0
The assignment is redundant even where it is not harmful, because the
error passed to the callback is already a copy of the machine's error;
using it directly is both correct and sufficient. fpi_ssm_dup_error()
is available for callers that do need an owned copy. No other driver in
the tree assigns the borrowed SSM error this way.
Reproduced on a Realtek 2541:fa03 (Minisforum AI X1 Pro 470) by
enrolling a finger while a template was already present in on-chip
storage, which takes the delete path shown above. The same driver bug
reaches fprintd a second way, via fp_enroll_ssm_done() ->
fpi_device_enroll_complete() -> fprintd's enroll_cb().
task_ssm_done cleared task_ssm before calling maybe_cancel, so
identify_cancel_ssm_done couldn't tell whether the identify already
completed or was cancelled. Use fpi_device_action_is_cancelled()
and skip the duplicate identify_complete call in IDENTIFY_COMPLETE
state when cancelled, letting the cancel flow handle completion.
Cancelling an operation does allow the driver to do perform async ops
at the moment, but rather we are supposed to just send the the cancel
commands while another action may running.
So we should handle this as part of the SSM final stage, if cancellation
happened.
Add IDENTIFY_SEND_CANCEL_RESULT state to send cancel command
after identify/verify completes, triggering firmware template update.
Implement cancel() callback for enroll and identify/verify cancellation.
Updated umockdev test data to cover the new flow.
When a USB transfer is cancelled, actual_length is set to -1. This
gets implicitly cast to gsize (unsigned) in log_transfer(), resulting
in a huge length passed to fp_dbg_hex_dump_data() and causing a
segfault.
Only dump data when the length is within valid bounds.
There is no need for them to be writable and with the previous commit
this change will also not result in any compiler warnings. Simply change
all of the static definitions to be static const.
Signed-off-by: Benjamin Berg <benjamin@sipsolutions.net>
The function only uses the passed command as a const buffer and makes a
copy immediately. As such, there is also no need for a destroy callback
as the caller can simply clean up afterwards.
Signed-off-by: Benjamin Berg <benjamin@sipsolutions.net>
While most of commands should run separated, others such as cancellation
can run concurrently so we cannot share command data in the device
structure, but it has to be rather per command.
Move it there
The driver relied on a running finger-detection or capture callback to
observe the deactivating flag and call complete_deactivation(). When the
async loop was broken by a session error from a terminal callback (such
as capture_sm_complete), no further iteration was left to notice the
flag, so the deactivation never completed.
Rather than scatter complete_deactivation() calls after every
fpi_image_device_session_error(), track whether an operation is actually
pending with an "active" flag and let dev_deactivate() complete the
request itself when nothing is in flight.
This keeps the deactivation lifecycle owned by dev_deactivate.
When tapping on the sensor rather than swiping through it, super RSR will
drop slices with 0-3 pixels of Y motion. In such case, self->strips_len is
zero and causing protocol error. Handle such cases by calling for a re-scan.
Closes: https://gitlab.freedesktop.org/libfprint/libfprint/-/work_items/786
Assisted-by: DeepSeek:deepseek-v4-pro-preview
Signed-off-by: Shengyu Qu <wiagn@4d2.org>
In some cases, the driver generates error, but complete_deactivation() is not
called after calling fpi_image_device_session_error(). In this case, fprintd
would be waiting for fpi_image_device_deactivate_complete() forever. Fix by
adding calls for complete_deactivation();
Closes: https://gitlab.freedesktop.org/libfprint/libfprint/-/work_items/786
Assisted-by: DeepSeek:deepseek-v4-pro-preview
Signed-off-by: Shengyu Qu <wiagn@4d2.org>
We cannot assume that two NBIS prints are matching without going through
proper NBIS checks, so we cannot do a check on the scanned print without
an extra thread, which is rather an overkill.
So let's just do the check for prints we can actually compare (raw ones
for now)
