We were attaching the sources to the default main context. Instead, we
should be attaching them to the current tasks main context (or, failing
that, the current thread local main context).
g_idle_add attaches to the default main context, but the rest of
libfprint is using the thread local main context. Switch to use the
internal fpi_device_add_timeout API for the workaround in order to
not rely on the default main context.
Unfortunately, the implementation was not thread safe and was not
sticking to the thread local main context.
In addition to this, it is not entirely clear to me how this API should
behave. The current approach is to simply cancel the transition with the
state machine halting in its current state. Instead, it could also make
sense for cancellation to cause the state machine to return a
G_IO_ERROR_CANCELLED.
As such, simply remove the feature for now. If anyone actually has a
good use-case then we can add it again.
libfprint uses the thread local context in almost all cases. Update
FpContext to also use it and make sure that any sources are removed when
the FpContext object is finalized. Otherwise we may run into
use-after-free issues.
In some situations one may want to guarantee that the last steps of an
SSM are run even when the SSM is completed early or failed.
This can easily be done by making fpi_ssm_mark_completed jump to the
next cleanup stage when called (this also includes mark_failed). Due to
the mechanism, it is still possible to explicitly jump cleanup states by
using fpi_ssm_jump_to_state, including a jump to the final state in
order to skip all cleanup states.
These routines assume that any messages is composed of a write and/or
read part. While the API allows sending and receiving as part of one
messages/transfer, it does not permit full duplex operation where data is
both send and received at the same time.
Upstream systemd/udev is pulling our autosuspend hwdb, so if udev is new
enough, then there is no need to install the file. As such, add
auto-detection logic for the scenario.
This also changes the name of the option and the type to "feature".
The device will always use sequence number 0 for certain messages. We
use this knowledge to filter the messages and assume that it is one of
these special messages rather than a response to a command.
However, we could end up sending a command with a sequence counter of 0
which would result in the response being ignored. Fix this by ensuring
we correctly wrap from 255 to 1 instead of 0.
Fixes: #358
The irq handler may already be stopped if stop_irq_handler is called. In
that case, we should immediately call the handler rather than just never
calling it.
This fixes deactivation when the device is unexpectedly unplugged.
Closes: #355
While meson suggests to always use 'library' this leads to some unwanted
behaviors when it comes to generate pkg-config files for it, as they
will include `Libs.Private` / `Required.private` fields that should not
be really part of a shared library as libfprint is meant to be used.
There are two variants one with storage and identify support and the
other without storage.
It implements the following commands:
* INSERT id
* REMOVE id
* SCAN id
* ERROR error-code
* LIST (returns saved print)
The INSERT/REMOVE/LIST commands are only available in the storage
driver. The SCAN command emulates presenting a finger.
These commands can be send ahead of time, and will be queued and
processed when appropriate. i.e. for INSERT/REMOVE that is immediately
when possible, for SCAN/ERROR processing is delayed.
The LIST command is always processed immediately.
Note that only a single command can be send per socket connection and
the command must be send in a single message. The socket will be closed
after the command has been processed.
Co-authored-by: Bastien Nocera <hadess@hadess.net>
Co-authored-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
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>
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.
Add a new test that checks that the unsupported list is not out of date.
As the wiki can be edited at any time, add this as a further optional
check into the CI pipeline.
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
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.
The gallery needs to be copied, as such we must do a deep comparison
instead of comparing the pointers. We also can't do the comparison
afterwards, as the gallery is owned by the operation and that operation
is finished already.
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.
This tests all relevant scenarios of device removal, i.e.:
* device is not open
* device is currently closing
* device is open and idle
* device is currently opening
* device is open and active
The test ensures that in all scenarios the following holds true:
* device "removed" signal is only emitted after the action completes
* context "device-removed" signal is only emitted after the device has
been closed
Note that the "opening" case is special. Here we confirm that a success
from "open" will not be overriden by a FP_DEVICE_ERROR_REMOVED error, in
order to correctly signal that the internal device state is open and it
needs to be closed.
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.
It is not very useful to just delete the data again after a failure, as
it might be useful for debugging. Just store it into an "errors"
subdirectory of the PWD in the hope that this is a sane location.
Note that it'll error out if the directory already exists, but that
should be acceptable in all cases. i.e. it won't exist in the CI and
developers can just wipe the directory.
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.