Instead of repeating the same code in both the virtual-image and the
virtual-device drivers, implement a class to handle the socket listening
an data reading.
Co-authored-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
This solves various problems:
1. It stays the same also if some drivers have been disabled
2. It uses a stable path for being imported by systemd
3. It is still checked for its validity by tests
4. It can be auto-generated using a simple command
The change to print a warning (for testing purposes) from commit
944e0d0383 (udev-rules: Print warning if an ID is supported) was
incorrect because it prevented duplicated to be suppressed if a device
is listed by two independent drivers.
As we are shipping a hwdb file now, we cannot have a collision with the
old libfprint version. Also, we are going to pull these rules into
systemd and they will not be installed via libfprint in the future. As
such, collisions will not happen again and it makes more sense like this
for systemd.
We want systemd to pull our hwdb. In order to ease this, always build
the hwdb file, even if it is disabled.
Once systemd has merged the rules, downstream should turn off the rules
in libfprint. The default in libfprint will also be changed to not build
the hwdb (udev_rules option) eventually.
We only use the rules/hwdb to enable auto-suspend. So, instead of
shipping our own rules, we can just use the existing autosuspend rules
and ship a hwdb that sets the appropriate flag.
Closes: #336
there is no specific API for report finger status,
finger needed status is set when captrue sample cmd send, once cmd receive correct,
finger is pressing on sensor.
Found by coverity. While quite bad in theory, proper safeguards are in
place, so it will only result in a g_return_val_if_fail to be hit rather
than causing more severe side effects.
We used to return early in the case where the print matched in order to
report the result more quickly. However, with the early reporting
mechanism and the fprintd side implementation of it, this is not
necessary anymore.
As such, only stop the "verify" and "identify" operations when the
finger is removed (or the operation is cancelled, which is actually what
will happen currently).
It is easier (and more correct) to create a new print from the reported
data and match that against the prints in the gallery.
We continue to return NULL during verify as we cannot provide any
additional information in that case.
NBIS just does weird things and while the array-parameter warning is
easy to fix, the other is not trivial. So disable these warnings so that
we can still build using newer GCC versions.
This function was always documented to return a sunk reference, but it
did not do so. This change is technically backward incompatible.
However, it only has an effect if anything is doing a g_object_ref_sink.
Which may happen inside libfprint itself. With the change, most API
users (including fprintd) are fixed to do refcounting correctly. Any API
user which worked around this will have a memory leak now.
That is not ideal, but it is not really that bad overall. And returning
a floating reference for FpPrint creation was a bad idea in the first
place. And it really only makes sense for fp_print_new as the only
(public) use case is to create the template for enrollment.
When serializing an image print in big endian machine we ended up
swapping the arrays contents two times, first when adding the values and
eventually when calling g_variant_byteswap which already handles this
properly.
With this, we get the test passing into s390x.
Fixes: #236
Identify function is supposed to propagate a boolean value, but we make
it return an integer instead on idle, this can be normally the same in
most of architectures, but not in BE ones.
So, make it return the proper type.
Fixes test failures in s390x.
Related to #236
In general, we rely on the underlying transport layer to throw errors
which will abort the current operation. This does not work for the
virtual image device though, but we need it there for testing purposes.
Add a notify::removed handler that makes things work as expected. Let it
throw a protocol error which should not be visible to the outside.
We require the close call, but as the underlying transport layer is
gone, it will generally just return an error.
In principle, it makes sense to think of close as a function that always
succeeds (i.e. it makes no sense to try again). Should the device be in
a bad state, then a subsequent open() will simply fail.
This enhances the device removal to create a well defined behaviour.
Primarily, it means that:
* "device-removed" will only be called for closed devices
* "removed" will be called only when no operation is active
Note that all actions will fail with FP_DEVICE_ERROR_REMOVED, *except*
for open which will only return this error if it failed.
Resolves: #330
When a new connection came in we would close the old connection. This in
turn would trigger a receive error causing the *new* connection to be
closed from the error handler.
Fix this by simply cancelling any pending transfers when a new
connection comes in. Also change the error handling code to catch issues
like partial writes correctly.
This fixes an issue for the fprintd test where some tests were flaky.
While the image device has its own finger status tracking, we use a simpler
version as public data information, so let's just report the finger-on/off
and when a finger is expected to the parent class.
Verify that this happens as expected using the virtual-image class
When porting the driver to the new libfprint 1.90.0 a mistake was made
where the device was not passed through user_data anymore but it was
still read from there. Stop using user_data in the callback to fix this.
See: #320
The elan driver always "deactivates" the device after a cpature run. We
can simplify the while internal state into a single "active" state and
rely on the image device driving this state machine correctly.
i.e.:
* We start a callibrate/capture/deactivate when we go into the
AWAIT_FINGER_ON state
* We only store the fact that we are active and want to deactivate
* We rely on the image device never going into the AWAIT_FINGER_ON
state without first waiting for the finger to be off (which implies
deactivation).
This adds a number of new internal states to better capture what is
going on. Also added are checks that all transitions we make are in the
set of expected and valid transitions.
Only three drivers use the state_change notification. These drivers are
updated accordingly.
The boolean is just used to emit a warning for unexpected state
transitions. It is sufficient to pass it to the deactivate function
directly for that purpose.
We might redo image transfers, but we only ever had one reference that
was implicitly removed after the transfer completed. Add a new reference
each time it is submitted and only free the last reference in the stop
handler.
For some reason we re-submitted the interrupt transfer with a timeout of
1s while the original submission was without a timeout. This looks like
typo, remove the timeout.
Annotate the USB transfer creation functions with the correct buffer
access attribute. Note that we only annotate them as read_only as the
functions may be used for sending and receiving.
Hopefully this will catch buffer overflows in drivers in the future.
Doing this avoids race conditions in testing code where the code might
try to submit errors/images before the device has been activated. This
would then (sometimes) trigger assertions in the image driver code.
The earlier image device code tried to hide deactivation from the
surrounding library. However, this does not make any sense anymore with
the early reporting feature present.
This changes the operation to complete only once the device is
deactivated. Also changed that in most cases (except for cancellation)
we wait for the finger to be removed before deactivating the device.
The driver seems to have relied on the device to be fully
deactivated and activated again for each capture. This is not the case
during enroll, and as such in can cause issues.
Try fixing this by splitting out the last few commands send to the
device and assuming that this starts the capture.
Fixes: #306
We have plenty of code paths where a transfer may be cancelled before it
is submitted. Unfortunately, libgusb up to and including version 0.3.6
are not handling that case correctly (due to libusb ignoring
cancellation on transfers that are not yet submitted).
Work around this, but do so in a somewhat lazy fashion that is not
entirely race free.
Closes: #306
Delay the open/close callbacks by 100ms so that we have a window of
opportunity for race conditions. This is needed to test certain
conditiosn in fprintd.
Adding a trailing \n to g_message, g_debug, g_warning and g_error is not
neccessary, as a newline will be added automatically by the logging
infrastructure.
The cancellable needs to be free'ed at deactivation. Also free it if we
run into a fatal error, which then in turn indicates that the device is
deactivated already.
The change_state function is called synchronously from the
image_captured callback. This means that deactivation of the device
happens during the img_cb function, causing the USB transfer to be
re-registered even though the device is already deactivating.
There are various ways to fix this, but it makes sense to directly bind
the cancellation to the deactivation. So create a cancellable that we
cancel at deactivation time, and make sure we always deactivate by going
through cancellation.
closes: #306
Image devices are simply deactivated suddenly. As such, the cancellation
logic of FpDevice is not really useful there, but the documentation was
apparently accidentally copied unmodified.
This is unlikely to happen in a real world scenario and currently breaks
running the CI test (not the umockdev based ones) while building the
flatpak. Lower the severity to avoid aborting because there is a
warning.
This is based on the patch and observation from Bastien that some
URU4000B devices do not use encryption by default (it is a configuration
stored within the firmware). As such, it makes sense to always detect
whether encryption is in use by inspecting the image.
The encryption option would disable flipping of the image for the
URU400B device. Retain this behaviour for backward compatibility.
The code would just read 4096 bytes from the packet, without checking
the size and neither setting short_is_error. It is not clear whether
packets from the device are always 4096 bytes or not. But the code
assume we always get a full line, so enforce that and use the actual
packet size otherwise.
This was not used. The old driver used this if creating a USB transfer
failed, however, we delay any such failures (which cannot really happen)
into the callback today, where the error is handled differently.
The GPtrArray needs to be created at some point. Also, reference
counting was wrong as submitting the transfer sinks the ref, but we rely
on it surviving.
Note that we really should change this to only have one in-flight
transfer and starting a new one after it finishes.
Co-authored-by: Vasily Khoruzhick <anarsoul@gmail.com>
When sending static data, it would not be copied. The function that
sends it assumed that it should be free'ed though.
Fix this by simply always making a copy.
The driver would warn about the fact that a state change is queued, but
still queue it a second time. This would result in deactivation to run
twice.
See: #216
The image driver may still be deactivating when a new activation request
comes in. This is because of a hack to do early reporting, which is
technically not needed anymore.
Fix the immediate issue by properly reporting the retry case. The proper
fix is to only finish the previous operation after the device has been
deactivated.
The driver might forget to set the type of the print. Catch that error a
bit earlier rather than failing when the API user tries to load it from
disk again.
The type of the print (RAW or NBIS) needs to be filled in by the driver.
For most drivers the image devices does this (NBIS), but the
corresponding call was missing in the upekts driver, rendering the
enrolled print unusable.
__handle_incoming_msg would copy the payload of the message into a newly
created buffer just to destroy it again immediately after calling the
callback. Just reference the correct address inside the original package
instead.
Also, in one case the extra buffer was leaked afterwards.
The driver would correctly calculate the amount of extra space needed to
receive the whole packet. It would also request the correct number of
bytes for this transfer.
However, the reallocated buffer to hold this data was directly derived
from the expected payload size and did not include the overhead.
Make the code more explicit and get rid of the confusing
MAX_DATA_IN_READ_BUF define that hides details on buffer allocation
calculation from the code.
The device is already beeing de-initialised from the verify/enroll
commands. Trying it again will result in a timeout error as it is not
responding properly at that point.
And activate perimeter points removal if this flag is set
This flag should be set for aes1610, aesx660, aes2501, aes2550
and upektc_img since these sensors may produce incomplete image.
Fixes: #142
This step helps to drop false minutiae for short sensors and
it was accidentally dropped during NBIS update. Re-add this step
and add a patch to update script to ensure that it's not dropped
during next update.
Fixes: 9fb789dc78 ("nbis: Update to NBIS 5.0.0")
If a transfer errors out then actual_length is negative. The only error
that is not caught is a timeout error, which should also result in the
SSM to move to the next state.
For some reason static checkers report that the auto-free is not run if
the goto exists the loop. It seems to me like that should work fine, but
we can simply make the analysers happy by moving it.
We need more than 1 line for assembling, but in general, we should
have a reasonable amount of lines. So use the width in the hope we'll
get an image that is about square at least.
Closes: #135
When no driver is found for an USB device a debug message is
printed. However, it has PID and VID in wrong order - usually it
is Vendor ID which goes first. This is how 'lsusb' prints it.
Matching the order helps debugging.
Signed-off-by: Michal Privoznik <michal@privoznik.com>
In the fpi_print_fill_from_user_id() GDate is defined using
g_autoptr(). However, this requires new enough GLib. For older
versions there's a definition provided locally in fpi-compact.h.
Include the file to fix build with older version of GLib.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If we fail when setting the scanned image to a print, we'll have a fatal
error, in such case we can terminate the current action and deactivate the
device.
In some cases nbim's get_minutiae returns no minutiae without providing an
error code, and if this happens we would crash in the task callback function
as we would try to deference the data->minutiae pointer.
Related to: #251
If the image height is less than the sensor horizontal resolution, then
return a retry error rather than trying to submit the image for further
processing.
Related to: #251
The minutiae detection might fail and we must not copy any data at that
point. So check g_task_had_error to ensure that we only do so when the
task was successful.
Fixes: #251
libfprint/fp-print.c: In function ‘fp_print_equal’:
libfprint/fp-print.c:596:21: warning: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’} [-Wsign-compare]
596 | for (i = 0; i < self->prints->len; i++)
| ^
libfprint/fp-print.c: In function ‘fp_print_serialize’:
libfprint/fp-print.c:667:21: warning: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’} [-Wsign-compare]
667 | for (i = 0; i < print->prints->len; i++)
| ^
libfprint/fp-print.c: In function ‘fp_print_deserialize’:
libfprint/fp-print.c:823:21: warning: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘gsize’ {aka ‘long unsigned int’} [-Wsign-compare]
823 | for (i = 0; i < g_variant_n_children (prints); i++)
| ^
The public API uses gio and gobject header, ensure that these are in the
list of Required pkg-config modules, otherwise they are added to
Required.private which is not OK.
It appears the order of linking is relevant in this case, change it to
fix some linking issues.
It may be that there are better solutions to this problem.
Devices with no storage don't allow listing prints, and if we try to do
that, we'd end up in trying to call a NULL function pointer, causing a crash
So always check if the device has storage before calling the list vfunc, and
if we fail, return an error.
Include an unit-test to verify this situation
Without this we get warnings like the following:
cc1: warning: .../_build/libfprint/nbis/libfprint-include: No such file or directory [-Wmissing-include-dirs]
We are already using a number of defines and autoptrs from newer GLib
releases. Add the appropriate compatibility defines rather than removing
the corresponding code.
Closes: #222
To avoid conflicts with previous libfprint version and make sure that the
target version is the correct one (plus to allow parallel install in some
distros), let's use a versioned naming for the library keeping the abi
version in sync.
Fixes#223
The callback function would continue processing even after having failed
the SSM already. This causes further invalid operations on the SSM.
This error was found using a coverity scan.
When quickly scanning a finger with the synaptic driver, it may wait forever
for finger removal even if this has already happened.
In fact we don't take care of the finger status when reporting the
verification.
To avoid this, add a function that delays the completion of the verification
until the finger removal if the finger is on sensor, otherwise it just
performs it.
Fixes#228
Remove the never-used cmd_complete_data value and the repetition of
cmd_complete_error.
In fact now as per the fpi_device_verify_report() usage, we already pass
the match information to the device, such as the error and it will be
they will be used on completion if needed.
This allows to simplify cmd_ssm_done() as well.
In some cases we want to complete the verification after that the finger has
been removed, but we still need to promptly report the match state otherwise
fpi-device will complain about, and will eventually cause a match error
instead that reporting a non-match:
synaptics: Finger is now on the sensor
synaptics: Received message with 0 sequence number 0x91, ignoring!
synaptics: interrupt transfer done
synaptics: delaying match failure until after finger removal!
synaptics: interrupt transfer done
device: Driver reported successful verify complete but did not report the
result earlier. Reporting error instead
libfprint: Failed to verify print: An unspecified error occured!
Fixes#227
This was added as alias to the error check, but given we're passing to the
callback both the error and the match itself, we can just avoid adding an
extra parameter that could be confusing (as may imply that the matching
happened).
Also clarify the documentation and ensure that the match value is properly
set in tests.
It is a good idea to report match results early, to e.g. log in a user
immediately even if more device interaction is needed. Add new _full
variants for the verify/identify functions, with a corresponding
callback. Also move driver result reporting into new
fpi_device_{identify,verify}_report functions and remove the reporting
from the fpi_device_{identify,verify}_complete calls.
Basic updates to code is done in places. Only the upekts driver is
actually modified from a behaviour point of view. The image driver code
should be restructured quite a bit to split the reporting and only
report completion after device deactivation. This should simplifiy the
code quite a bit again.
Some things were odd with regard to the ownership of passed objects. Try
to make things sane overall, in particular with the possible floating
FpPrint reference.
The progress report user data free func was not assigned and therefore
never called. Add the missing assign, potentially fixing memory leaks
(mostly relevant for bindings).
Unfortunately, the timeout handling cannot be simulated properly. This
also adds a workaround in the driver to not consider it a protocol error
if this happens.
Continuing an enroll was broken in case of a retry error. Explicitly add
code to wait for the finger to go OFF after a retry error, and ensure
that the enroll will continue once that has happened.
We cannot copy information from the class in the _init routine, instead,
this needs to be done in _constructed. Move the relevant code into a new
_constructed function to fix importing the bz3_threshold override from
drivers.
Fixes: #206
The offset stored for a frame was not always relative to the previous
frame. This was the case for reverse movement, but for forwrad movement
the offset was the one to the next frame.
Make the offset handling consistent and alwasy store the offset to the
previous frame. Also update the frame assembling code to add the offset
before blitting the frame (i.e. making it relative to the previous frame
there too).
Note that fpi_assemble_lines already made the assumption that this was
the case as it forced the offset for the first frame to be zero. As
such, the code was inconsistent.
This could affect the AES drivers slightly as they use hardware reported
values which might not adhere to these assumptions.
We might want to iterate through the supported fingers values, to do that we
were hardcoding FP_FINGER_LEFT_THUMB and FP_FINGER_RIGHT_LITTLE as first and
last fingers.
Not that we'd ever get more fingers (unless some weird radiation would do
the job), but it's logically nicer to read than random hardcoded values.
We prefixed them with fp- which is not as obvious as fpi-. Also,
explicitly mark them as private and to be skipped in the GObject
Introspection annotatinos.
Warning: FPrint: (Signal)fp-image-device-state-changed: argument object: Unresolved type: 'FpiImageDeviceState'
Don't make headers inclusions dependent on each others, given that FpiSsm
depends on FpiUsbTransfer and vice-versa, so fix the dependency cycle by
using forwarded declarations.
The private library needs to indirectly include fp-enum.h. This
dependency was not listed anyway, resulting in a race condition during
the build process.
