Incorrect verify implementation, unable to verify on one finger

[Loic Yhuel]

The closed-source library blocks on synchronous USB transfers in its "verify" method (at least, there might be others with the same issue).
The "identify" one is correct (asynchronous).

As a result :
$ fprintd-verify
Using device /net/reactivated/Fprint/Device/0
Listing enrolled fingers:
 - #0: right-index-finger
(finger pressed)
Verify started!
Verifying: right-index-finger
(blocked)

"Verify started!" should happen immediately, but the code libfprint code waits for fp_device_verify to return, which only happens when the finger is pressed with this driver.
The verify success message is sent during the call, so it is ignored.

"fprintd-verify -f any" and pam-fprintd use fp_device_identify, so they work correctly.
But with https://gitlab.freedesktop.org/libfprint/fprintd/-/commit/fc65055279 (so I suppose on jammy), the "verify" method will be used instead if only one finger is enrolled.

TL,DR: the fingerprint reader appears to mostly work on focal, but starting with jammy it won't unless at least two fingers are enrolled.

Note that this is tested on a Dell Latitude 5521, but the issue should be the same with all Broadcom fingerprint readers using this library.

Duplicate bug #1981974

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[prafulla chandra kota (prafullakota)]

Andy, FYI, I didn't observed this issue of fprintd-verify on previous ubuntu versions 20.04.

[Kai-Chuan Hsieh (kchsieh)]

@prafullakota

it's true the lib works for Ubuntu 20.04, however, OEM new platform will enable 22.04, and current fprintd works for other vendor goodix. The library is a prebuilt binary, could you install 22.04 and take a look. We need to know if Broadcom have to provide new library for 22.04.

Thanks,

[Andy Chi (andch)]

@prafullakota,
We do encounter fprintd-verify is not working on 20.04 and a bug is here. Not sure if you can access it. Bug #1940619

Below is the debug info by KC.

On the passed device XPS 13 (Modena TGL). The fprintd will emit VerifyFingerSelected at once when the test started. Then the application fprintd-verify will ready to receive VerifyStatus(verify-no-match) event.

On Broadcom's fingerprint, it will not get the VerifyFingerSelected immediately, then the fprintd-verify application keeps waiting. Once the user touch the fingerprint sensor it will receive two signal 1. VerifyStatus(verify-no-match) 2. VerifyFingerSelected. Then the fprintd-verify put itself info ready to verification phase, however, the VerifyStatus(verify-no-match) signal has emitted already, so it will wait eternally.

[prafulla chandra kota (prafullakota)]

Thanks for the details Andy, i don't have access to check Bug #1940619, I don't recollect the exact ubuntu version, we have released it on ver 18 and 20.04, but I do remember it was verified on one of the version, may be 18.04 then, anyways FP identify working is more important than this application.

[Kai-Chuan Hsieh (kchsieh)]

@parafullakota

hello,

I think it is not true, the fprintd-verify is the excutable provided by fprintd package from upstream, and Ubuntu hasn't modified anything for the package, which means current tod library can't work with upstream fprintd correctly.

https://gitlab.freedesktop.org/libfprint/fprintd

Can you fix it first? then we can check if it still has problem on Ubuntu 22.04.

Thanks,

[prafulla chandra kota (prafullakota)]

Hi Kai-Chuan Hsieh

As per my understanding: Tod library talks to libfprintd module and
libfprintd talks to fprintd.
Could you let me know what is changed and what is to be fixed in the tod
driver in compatible with upstream fprintd module?

As per the statement below, I believe ABI is preserved and no changes
required here, if you could let me know any changes to be done from TOD
driver, i will work on it.
------------------

There are currently no plans to backport it to 20.04.

No changes are strictly required on the drivers side, nor they need to be
recompiled (as the TOD v1.90.1 ABI was preserved), however it's strongly
advised to do so, as new internal functions and feature are now available
to use, and the old driver may not take advantage of them.

Thanks,
Prafulla

On Tue, Aug 16, 2022 at 11:15 AM Kai-Chuan Hsieh <email address hidden>
wrote:

> @parafullakota
>
> hello,
>
> I think it is not true, the fprintd-verify is the excutable provided by
> fprintd package from upstream, and Ubuntu hasn't modified anything for
> the package, which means current tod library can't work with upstream
> fprintd correctly.
>
> https://gitlab.freedesktop.org/libfprint/fprintd
>
> Can you fix it first? then we can check if it still has problem on
> Ubuntu 22.04.
>
> Thanks,
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1956885
>
> Title:
> Incorrect verify implementation, unable to verify on one finger
>
> Status in libfprint-2-tod1-broadcom:
> Confirmed
> Status in OEM Priority Project:
> Confirmed
> Status in libfprint package in Ubuntu:
> Confirmed
>
> Bug description:
> The closed-source library blocks on synchronous USB transfers in its
> "verify" method (at least, there might be others with the same issue).
> The "identify" one is correct (asynchronous).
>
> As a result :
> $ fprintd-verify
> Using device /net/reactivated/Fprint/Device/0
> Listing enrolled fingers:
> - #0: right-index-finger
> (finger pressed)
> Verify started!
> Verifying: right-index-finger
> (blocked)
>
> "Verify started!" should happen immediately, but the code libfprint code
> waits for fp_device_verify to return, which only happens when the finger is
> pressed with this driver.
> The verify success message is sent during the call, so it is ignored.
>
> "fprintd-verify -f any" and pam-fprintd use fp_device_identify, so they
> work correctly.
> But with
> https://gitlab.freedesktop.org/libfprint/fprintd/-/commit/fc65055279 (so
> I suppose on jammy), the "verify" method will be used instead if only one
> finger is enrolled.
>
> TL,DR: the fingerprint reader appears to mostly work on focal, but
> starting with jammy it won't unless at least two fingers are enrolled.
>
> Note that this is tested on a Dell Latitude 5521, but the issue should
> be the same with all Broadcom fingerprint readers using this library.
>
> Duplicate bug #1981974
>
> To manage notifications about this bug go to:
>
> https://bugs.launchpad.net/libfprint-2-tod1-broadcom/+bug/1956885/+subscriptions
>
>

--
Thanks,
Prafulla Kota
9866544810

[Loic Yhuel (lyhuel)]

Everything is in the description : the "verify" method isn't correct in the Broadcom library (it is synchronous, it must be asynchronous).
On 20.04, the login would always use "identify", so the problem was hidden.
But due to the libprintfd change, on 22.04 the login uses "verify" when only one finger has been enrolled (and still "identify" if two or more fingers have been enrolled).

[prafulla chandra kota (prafullakota)]

Hi,

Thanks for inputs regarding change in behavior of 20.04 vs 22.04, I will
look at the verify function implementation.

Thanks,
Prafulla

On Tue, Aug 16, 2022 at 1:11 PM Loic Yhuel <email address hidden>
wrote:

> Everything is in the description : the "verify" method isn't correct in
> the Broadcom library (it is synchronous, it must be asynchronous).
> On 20.04, the login would always use "identify", so the problem was hidden.
> But due to the libprintfd change, on 22.04 the login uses "verify" when
> only one finger has been enrolled (and still "identify" if two or more
> fingers have been enrolled).
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1956885
>
> Title:
> Incorrect verify implementation, unable to verify on one finger
>
> Status in libfprint-2-tod1-broadcom:
> Confirmed
> Status in OEM Priority Project:
> Confirmed
> Status in libfprint package in Ubuntu:
> Confirmed
>
> Bug description:
> The closed-source library blocks on synchronous USB transfers in its
> "verify" method (at least, there might be others with the same issue).
> The "identify" one is correct (asynchronous).
>
> As a result :
> $ fprintd-verify
> Using device /net/reactivated/Fprint/Device/0
> Listing enrolled fingers:
> - #0: right-index-finger
> (finger pressed)
> Verify started!
> Verifying: right-index-finger
> (blocked)
>
> "Verify started!" should happen immediately, but the code libfprint code
> waits for fp_device_verify to return, which only happens when the finger is
> pressed with this driver.
> The verify success message is sent during the call, so it is ignored.
>
> "fprintd-verify -f any" and pam-fprintd use fp_device_identify, so they
> work correctly.
> But with
> https://gitlab.freedesktop.org/libfprint/fprintd/-/commit/fc65055279 (so
> I suppose on jammy), the "verify" method will be used instead if only one
> finger is enrolled.
>
> TL,DR: the fingerprint reader appears to mostly work on focal, but
> starting with jammy it won't unless at least two fingers are enrolled.
>
> Note that this is tested on a Dell Latitude 5521, but the issue should
> be the same with all Broadcom fingerprint readers using this library.
>
> Duplicate bug #1981974
>
> To manage notifications about this bug go to:
>
> https://bugs.launchpad.net/libfprint-2-tod1-broadcom/+bug/1956885/+subscriptions
>
>

--
Thanks,
Prafulla Kota
9866544810

[prafulla chandra kota (prafullakota)]

Hi Loic Yhuel,

Attached the diff log of single finger(verify) vs two finger (identify), i looked at verify implementation of 4 modules, 2 modules are using same function for identify & verify operation, I compared the diff of verify implementation with broadcom module, didn't find any issues.

I also tried using same identify function for both verify and identify, didn't observed any difference in functionality, can you review the attached log and let me know if you see any findings.

Thanks,
Prafulla

[prafulla chandra kota (prafullakota)]

Let us know if you need any information beyond the log and i didn't followed the how the synchronous/asynchronous references to TOD driver verify function, there is no full time blocking call available on verify function.
Attached verify function source for your reference.

[Loic Yhuel (lyhuel)]

The verify fonction is synchronous here, the wait is probably the "while(!cvif_process_interrupt(MAX_WAIT_RX_IDENTIFY_TIME))".

As I wrote, when testing with fprind-verify :
"Verify started!" should happen immediately, but the libfprint code waits for fp_device_verify to return, which only happens when the finger is pressed with this driver.
The verify success message is sent during the call, so it is ignored.

https://fprint.freedesktop.org/libfprint-dev/FpDevice.html#fp-device-verify => fp_device_verify is specified to be asynchronous.
It calls the verify callback of the driver : https://gitlab.freedesktop.org/3v1n0/libfprint/-/blob/tod/libfprint/fp-device.c#L1381.
So this implies the verify callback must not block.
https://fprint.freedesktop.org/libfprint-dev/libfprint-2-Internal-FpDevice.html documents the verify method as "Start a verify operation", the "Start" is probably meant to suggest the asynchronous implementation.

"fprintd-verify -f any" (when at least 2 fingers are enrolled) prints the "Verify started!" message quickly, without waiting for the finger to be pressed.
So it seems the identify function in the Broadcom driver is different, maybe it runs in a thread.

Looking at other drivers, it seems they either do their own asynchronous code, or they use fpi_ssm_start (https://fprint.freedesktop.org/libfprint-stable/libfprint-Sequential-state-machine.html).
In the callbacks like verify, they usually send the command to the device, but then the response would be handled asynchronously.

The issue for the Broadcom driver is that you probably need to call cvif_process_interrupt somewhere, maybe in a thread. From the symbols, it seems it calls libusb synchronous APIs directly, instead of for example using https://fprint.freedesktop.org/libfprint-dev/libfprint-2-USB-transfer-helpers.html (which wrap gusb, which rely on libusb asynchronous APIs).

[prafulla chandra kota (prafullakota)]

Hi Loic,

Thanks for the detailed information, i will look about verify function implementation as per above links.
mean time, similar to other sensors, i tried to configure both identify and verify functions to same function of current identify and reading the current status whether it is identify/verify and returning accordingly.

But i am encountering below error of claim device, how to claim the device during identify function?

Aug 24 15:25:08 adminuser-Latitude-5431 systemd[1]: Started Fingerprint Authentication Daemon.
Aug 24 15:25:08 adminuser-Latitude-5431 fprintd[3236]: Authorization denied to :1.126 to call method 'Release' for device 'Broadcom Sensors': Device was not claimed before use
Aug 24 15:25:38 adminuser-Latitude-5431 systemd[1]: fprintd.service: Deactivated successfully.

Thanks,
Prafulla

[prafulla chandra kota (prafullakota)]

Hi Loic,

I have modified verify function to be asynchronous call and able to verify single Finger enrolled FP identify operation, i am curious to understand below error when i configured verify/identify to same function similar to goodix tod driver.

But i am encountering below error of claim device, how to claim the device during identify function?

Aug 24 15:25:08 adminuser-Latitude-5431 systemd[1]: Started Fingerprint Authentication Daemon.
Aug 24 15:25:08 adminuser-Latitude-5431 fprintd[3236]: Authorization denied to :1.126 to call method 'Release' for device 'Broadcom Sensors': Device was not claimed before use
Aug 24 15:25:38 adminuser-Latitude-5431 systemd[1]: fprintd.service: Deactivated successfully.

Thanks,
prafulla

[Andy Chi (andch)]

Hi prafullakota,
You encountered claim issue when you try to login with your finger?

Can you help to add "Environment=G_MESSAGES_DEBUG=all" in /lib/systemd/system/fprintd.service and try again then upload the log?

[prafulla chandra kota (prafullakota)]

As suggested verify function modified asynchronous call as this function is called at 22.04 unlike Ubuntu 20.04.
It is working fine now for single FP identify and multiple fingers identify is already working good on 22.04 OS.

[Kai-Chuan Hsieh (kchsieh)]

@prafullakota

Thanks for your help, is it possible to deliver new binary for us to create new broadcom-tod debian package to let more user can try?

Thanks,

[prafulla chandra kota (prafullakota)]

Hi Andy & Kai-Chuan Hsieh

Thanks for checking, will discuss with internal team and get back to you
whether it is before our QA or after broadcom QA.

Thanks
Prafulla

On Mon, 12 Sep 2022 at 8:00 AM, Kai-Chuan Hsieh <email address hidden>
wrote:

> @prafullakota
>
> Thanks for your help, is it possible to deliver new binary for us to
> create new broadcom-tod debian package to let more user can try?
>
> Thanks,
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1956885
>
> Title:
> Incorrect verify implementation, unable to verify on one finger
>
> Status in libfprint-2-tod1-broadcom:
> Confirmed
> Status in OEM Priority Project:
> Confirmed
> Status in libfprint package in Ubuntu:
> Confirmed
>
> Bug description:
> The closed-source library blocks on synchronous USB transfers in its
> "verify" method (at least, there might be others with the same issue).
> The "identify" one is correct (asynchronous).
>
> As a result :
> $ fprintd-verify
> Using device /net/reactivated/Fprint/Device/0
> Listing enrolled fingers:
> - #0: right-index-finger
> (finger pressed)
> Verify started!
> Verifying: right-index-finger
> (blocked)
>
> "Verify started!" should happen immediately, but the code libfprint code
> waits for fp_device_verify to return, which only happens when the finger is
> pressed with this driver.
> The verify success message is sent during the call, so it is ignored.
>
> "fprintd-verify -f any" and pam-fprintd use fp_device_identify, so they
> work correctly.
> But with
> https://gitlab.freedesktop.org/libfprint/fprintd/-/commit/fc65055279 (so
> I suppose on jammy), the "verify" method will be used instead if only one
> finger is enrolled.
>
> TL,DR: the fingerprint reader appears to mostly work on focal, but
> starting with jammy it won't unless at least two fingers are enrolled.
>
> Note that this is tested on a Dell Latitude 5521, but the issue should
> be the same with all Broadcom fingerprint readers using this library.
>
> Duplicate bug #1981974
>
> To manage notifications about this bug go to:
>
> https://bugs.launchpad.net/libfprint-2-tod1-broadcom/+bug/1956885/+subscriptions
>
> --
Thanks,
Prafulla Kota
9866544810

[Andy Chi (andch)]

Hi Loic,
I built a new deb here, please also check on your side. I can verify with one finger.
fpritd-verify also works from my testing.

https://launchpad.net/~andch/+archive/ubuntu/staging-fprint

[Loic Yhuel (lyhuel)]

Hi Andy,

The package date, including the files inside, seems wrong : "Fri, 12 Aug 2022", it should be September.

This update contains both a new library, and new firmware files (which might help for other issues, in my case updating to newer firmware from the Windows driver fixed most of the cases where the sensor would disappear after suspend/resume and require a power cycle to appear again).

But seems not all firmware files have the correct version :
 - bcmCitadel_1.otp, bcmsbiCitadelA0_1.otp : new, perhaps the correct versions, but not used on my machine
 - bcmsbiCitadelA0_7.otp : SBI 137 (same as Windows driver Dell-ControlVault3-Driver-and-Firmware_88J36_WIN_5.9.8.17_A20.EXE)
 - bcmCitadel_7.otp : AAI 5.9.13.0 (same as Windows driver Dell-ControlVault3-Driver-and-Firmware_88J36_WIN_5.9.8.17_A20.EXE)

On my Dell Latitude 5521, it uses the "_7" firmwares, maybe there are others which use the "_1" ones, and which wouldn't have any issue with the update.

The new bcm_cv_current_version.txt contains :
01020000_512004_0
version: 5.12.004.0
SBI_VERSION: 172

With this, the library tries to update the firmware over and over, because :
Current AAI Version = 5.9.13.0
Current SBI Version = 137
AAI version available for upgrade = 5.12.4.0
SBI version available for upgrade = 172

It's the bcm_cv_current_version.txt which tells it the available versions.
After update it reads 5.9.13.0 and 137 from the device (since it's what is in the firmware files), so it restarts...

As a workaround, I modified bcm_cv_current_version.txt to :
01020000_509013_0
version: 5.9.013.0
SBI_VERSION: 137
(note that the Windows driver had 01020000_59013_0, but the new library parses it as version 0.59.13.0, which doesn't work).

With this change, the new library works (even if the firmwares might be older than what the library was designed to use), and seems to fix the issue.

If there is hardware using the "_1" firmwares instead of the "_7" like me, I suppose the included bcm_cv_current_version.txt would be correct.
So either the bcmsbiCitadelA0_7.otp and bcmCitadel_7.otp need to be upgraded to the correct versions, or the bcm_cv_current_version.txt needs to be modified and the bcmCitadel_1.otp and bcmsbiCitadelA0_1.otp probably need to be downgraded.

[prafulla chandra kota (prafullakota)]

Hi Loic Yhuel (lyhuel),

That was very good analysis, thanks for detailed information, we really appreciate.

Our release will have two binaries CID1(pre-production dell systems), CID7(production systems).

"This initial release that our team uploaded was marked “for pre-production systems only”. This is a release for testing. It’s only good for CID1 (pre-production) CV3 parts. Production laptops have CID7 parts so they cannot run the CID1 firmware in this release."

Work around you made for CID7 is absolutely a right way to progress testing temporarily until CID7 release happens except that CID7 FW is slightly older than CID-1 FW, but that doesn't stop any basic testing functionality verification, it will still sail through all basic verifications.

If you have CID1 system, we prefer you to use CID1 system for testing, as it will have latest CV3 firmware and latest FP Sensor libraries.

Thanks,
Prafulla

[Loic Yhuel (lyhuel)]

I didn't see the new 5.12.018.0 on git.

With those files :
 - the firmware update works (as the bcm_cv_current_version.txt has the correct versions which match CID7 firmwares, unlike in the test 5.12.303-0ubuntu1~22.04.01~oem3 package)
 - the issue is fixed (as with the test library)

So the bug could be closed if the updated deb package was pushed to the repositories.

[kalpesh.fulpagare (kalpesh-fulpagare)]

Hi Andy and Loic Yhue,

My device (Dell Latitude-5420) also has Broadcom fingerprint sensor (Broadcom Corp. 58200, ID 0a5c:5843)

I wanted to see if there had been any progress in fixing the bug.

[prafulla chandra kota (prafullakota)]

Hi Kalpesh,

This issue is fixed and you can verify if the updated package 5.12.x is installed on your system.

-Prafulla Kota

[Andy Chi (andch)]

Hi Kalpesh,
Would you mind to install this package on your laptop and give it a try?
https://launchpad.net/~andch/+archive/ubuntu/verify-ppa
libfprint-2-tod1-broadcom 5.12.018-0ubuntu1~22.04.01

[kalpesh.fulpagare (kalpesh-fulpagare)]

Hi Andy and Loic Yhue,

Thanks for the reply.

I can confirm the issue is fixed.
Fingerprint is working as expected for me in Ubuntu 22.04.3

Thanks,
Kalpesh