Project

General

Profile

Issue #957

NFC on I9300 does not require libpn544_fw.so if patch is reverted

Added by Simon Josefsson about 5 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Category:
Framework
Target version:
-
Start date:
08/05/2014
Due date:
% Done:

0%

Estimated time:
Resolution:
fixed
Device:
Galaxy S 3 (I9300)

Description

I looked into why NFC on I9300 required libpn544_fw.so in Replicant but not in CyanogenMod. I found this commit:

https://www.gitorious.org/replicant/external_libnfc-nxp/commit/85af7251179c3cf53c785e9a055942d3ab6462c1

It changes a soft-fail to a hard-fail when the file is missing. Reverting this makes everything work for me. Why was the patch added?

For background and debug messages see http://blog.josefsson.org/2014/08/05/replicant-4-2-0002-and-nfc-on-i9300/

If it is needed on some other platform, please consider making it soft-fail for I9300 devices.


Files

History

#1

Updated by Paul Kocialkowski about 5 years ago

As explained on IRC, the firmware is mandatory because some units ship with a non-working firmware that leaves the NFC chip in a semi-loaded and unusable state. A better solution would be to check the preinstalled firmware version and decide whether it can be used as-is or requires a firmware load. Anyone is welcome to work on implementing that.

#2

Updated by Simon Josefsson about 5 years ago

Sounds good to me -- do we know which from which versions, or if the relationship isn't as simple as >=, which versions, works?

#3

Updated by Paul Kocialkowski about 5 years ago

I didn't look into such details as of now.

#4

Updated by Simon Josefsson about 5 years ago

See attached patches to implement whitelisting. The patch reverts the earlier patch that unconditionally reject NFC on any device that doesn't have libpn544_fw.so, and then adds another check for firmware version once we know it.

If we don't know how to identify the non-working models/version, and you require that nfc is not initialized on non-working models, the only solution I see is to only let through known-good models (like mine). So that's how it is implemented. The exact model I have is permitted, anything else will be rejected. There may be better ideas, it is getting quite late so I may be missing something...

On a whitelisted model it would print:

D/NFCJNI ( 2504): Start Initialization
D/NFCJNI ( 2504): NFC capabilities: HAL = 8150100, FW = b10122, HW = 620003, Model = 12, HCI = 1, Full_FW = 1, Rev = 34, FW Update Info = 0
D/NFCJNI ( 2504): NFC firmware Replicant approved
D/NFCJNI ( 2504): phLibNfc_SE_GetSecureElementList()

On a non-whitelisted model it would print:

D/NFCJNI ( 2493): Start Initialization
E/NFC-HCI ( 2493): Could not open /system/vendor/firmware/libpn544_fw.so or /system/lib/libpn544_fw.so
W/NFC ( 2493): Firmware image not available: this device might be running old NFC firmware!
D/NFCJNI ( 2493): NFC capabilities: HAL = 8150100, FW = b10122, HW = 620003, Model = 12, HCI = 1, Full_FW = 1, Rev = 34, FW Update Info = 0
E/NFCJNI ( 2493): NFC firmware not Replicant approved, disabling NFC...
E/NFCJNI ( 2493): phLibNfc_Mgt_UnConfigureDriver() returned error 0x0032[NFCSTATUS_ALREADY_INITIALISED] -- this should never happen
D/NFCJNI ( 2493): Terminating client thread...
W/NfcService( 2493): Error enabling NFC

#5

Updated by Simon Josefsson about 5 years ago

Just to clarify: I don't have any phone which is non-whitelisted, so the error messages printed were just made up by changing the whitelisting-test to not match on my phone. So the error message about NFCSTATUS_ALREADY_INITIALISED is probably not happening on any real hardware with old firmware.

#6

Updated by Paul Kocialkowski about 5 years ago

The general idea is what I expected, even though I'll wait to test it on my devices and make sure it properly turns off the chip (as my previous patch did) when the preinstalled firmware doesn't match.

The patch needs a rework though, because it mentions Replicant in a very misleading way (don't mention Replicant at all, please). Saying that a firmware is Replicant-approved would somewhat suggest that we encourage the use of that non-free program, which is not the case!

Thanks for your work!

#7

Updated by Simon Josefsson about 5 years ago

Testing on more devices would be good -- I only have two devices and they behave the same. Agree fully on the messages -- sorry about that, updated patch attached.

#8

Updated by Paul Kocialkowski about 5 years ago

I'll test this on all the NFC-enabled Replicant-supported devices and let you know of the results. I will probably rework your code a bit anyways, but I'll keep you in Signed-off in case I end up making a separate patch.

Note that "NFC chip does not need firmware to work" is also wrong, because the devices does need a proprietary firmware for NFC to work in any case, either preinstalled or loaded by the system.

#9

Updated by Paul Kocialkowski almost 5 years ago

Since some devices, such as yours, work just fine with the preinstalled firmware, I decided to revert my previous patch. My concern was all about power management, but it seems to be handled fine thanks to the changes I've added to https://gitorious.org/replicant/packages_apps_nfc

In the meantime, I switched the expected firmware data location form the library that was dynamically opened to a static file, which is considerably better as it doesn't have to run any code from the library (this was apparently not the case with the library either, but there was still a security risk of code execution). See commit ec2052799f860ea6432482dbc26f54b5819130d3

#10

Updated by Paul Kocialkowski almost 5 years ago

  • Status changed from New to Closed
  • Resolution set to fixed
#11

Updated by Simon Josefsson almost 5 years ago

Thank you! Reverting the initial patch always seemed like the simplest and easiest solution to me.

/Simon

#12

Updated by Denis 'GNUtoo' Carikli over 3 years ago

  • Category changed from 117 to Framework
  • Device set to Galaxy S 3 (I9300)

Also available in: Atom PDF