Issue #957
closedNFC on I9300 does not require libpn544_fw.so if patch is reverted
0%
Description
I looked into why NFC on I9300 required libpn544_fw.so in Replicant but not in CyanogenMod. I found this commit:
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
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.
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?
Updated by Paul Kocialkowski over 9 years ago
I didn't look into such details as of now.
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
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.
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!
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.
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.
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
Updated by Paul Kocialkowski over 9 years ago
- Status changed from New to Closed
- Resolution set to fixed
Updated by Simon Josefsson over 9 years ago
Thank you! Reverting the initial patch always seemed like the simplest and easiest solution to me.
/Simon
Updated by Denis 'GNUtoo' Carikli over 8 years ago
- Category changed from 117 to Framework
- Device Galaxy S 3 (I9300) added