Issue #957: NFC on I9300 does not require libpn544_fw.so if patch is reverted - Replicant

Actions

Copy link

Issue #957

closed

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

Added by Simon Josefsson over 9 years ago. Updated almost 8 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Paul Kocialkowski

Category:

Framework

Target version:

Start date:

08/05/2014

Due date:

% Done:

Estimated time:

Resolution:

fixed

Device:

Galaxy S 3 (I9300)

Grant:

Type of work:

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

Download all files

0001-Don-t-require-non-free-libpn544_fw.so-file.patch (1.13 KB) 0001-Don-t-require-non-free-libpn544_fw.so-file.patch		Simon Josefsson, 08/09/2014 11:46 PM
0001-Reject-non-whitelisted-NFC-firmwares.patch (1.53 KB) 0001-Reject-non-whitelisted-NFC-firmwares.patch		Simon Josefsson, 08/09/2014 11:46 PM
0001-Reject-non-whitelisted-NFC-firmwares.patch (1.51 KB) 0001-Reject-non-whitelisted-NFC-firmwares.patch		Simon Josefsson, 08/14/2014 05:39 PM

History
Notes
Property changes

Actions

Copy link

Updated by Paul Kocialkowski over 9 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.

Actions

Copy link

Updated by Simon Josefsson over 9 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?

Actions

Copy link

Updated by Paul Kocialkowski over 9 years ago

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

Actions

Copy link

Updated by Simon Josefsson over 9 years ago

File 0001-Don-t-require-non-free-libpn544_fw.so-file.patch 0001-Don-t-require-non-free-libpn544_fw.so-file.patch added
File 0001-Reject-non-whitelisted-NFC-firmwares.patch 0001-Reject-non-whitelisted-NFC-firmwares.patch added

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

Actions

Copy link

Updated by Simon Josefsson over 9 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.

Actions

Copy link

Updated by Paul Kocialkowski over 9 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!

Actions

Copy link

Updated by Simon Josefsson over 9 years ago

File 0001-Reject-non-whitelisted-NFC-firmwares.patch 0001-Reject-non-whitelisted-NFC-firmwares.patch added

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.

Actions

Copy link

Updated by Paul Kocialkowski over 9 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.

Actions

Copy link

Updated by Paul Kocialkowski over 9 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

Actions

Copy link

#10