FeitianSmartcardReader / seek-for-android

Automatically exported from code.google.com/p/seek-for-android
2 stars 1 forks source link

Channel resource NOT release if SmartCardService is killed #36

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
On Android, a service could potentially be killed for varvious reason.  like a 
service it is depending on has died.  Anyway, the issue is that while there's 
cleanup logic on SEService Client API level on Service unBind and also 
SmartCardService onDestroy, it would not cover the situation if the process is 
killed.

To reproduce it
1. Have an application that open all the possible logical channel to UICC with 
SEEK (e.g. using a multiselectable Applet)
2. on debug DDMS, kill remote SmartCardService API (SEEK) process
3. Client close the session and shutdown the SEService on SEEK
4. Attemp to open a logical channel with SEEK again

- on 4), client should be able to open new channel again with a new SEService
- in reality, when Client close the session and shutdown the SEService, since 
the service it is associated with is already killed, that unbind the service 
and result in the cleanup logic from client level no longer working correctly.
- neither would the SmartCardService onDestroy would run because the process is 
killed

As a brainstorm solution idea:
- SmartCardService is keeping track on channel that is opened so it can do 
cleanup probably if onDestroy is triggered, but that is not presistent, so if 
onDestroy is not called, no cleanup would run.
- If opened channel is kept presistent, then when SmartCardService is 
started/restarted, it checks with any opened channels and close them properly.

Original issue reported on code.google.com by tommypo...@gmail.com on 7 Sep 2012 at 3:44

GoogleCodeExporter commented 9 years ago
When SmartCard API evolved in the beginning we discussed the lowmemorykiller.c 
problem in Android where a process (=SmartCardService) could get killed and no 
resources (=open channels) could get freed as onDestroy() where the cleanup is 
implemented never gets called.
Your proposal seems to be the way to go - release all logical channels on all 
connected secure elements in onCreate() once the service starts. However, this 
implies that SmartCard API is the only entity in Android to control secure 
elements...which is not the case.

I don't see 'kill <pid>' in adb shell or ddms in a real world usecase so I 
relied on the low-memory strategy in Android:  
http://developer.android.com/guide/components/processes-and-threads.html under 
'Process lifecycle' 
SCAPI service is only killed from the kernel after all other background 
activities are removed and one single foreground activity using the service is 
left over... then the device has other problems.
Still critical: 'malicious' activities that do not release their service 
bindings in onPause/Stop/Destroy()
(any lowmemorykiller.c expert: please correct me here if the interpretation is 
wrong!)

Current implementation is a trade-off between SCAPI acquiring control over all 
secure elements and Android process lifecycle implementation.

What's your opinion?

Original comment by Daniel.A...@gi-de.com on 7 Sep 2012 at 10:45

GoogleCodeExporter commented 9 years ago
Yes, the step to reproduce the issue is just a sure fire way of showing there's 
a problem if the smartcardapi process got killed unsuccessfully.

The real problem is that i have observed the SmartCardAPI process got killed 
because of some unexpected/uncaught runTime situation during the execution of 
SEEK implementation (e.g. Dead Object, some service SEEK is depending got 
killed and restarted).  And it happens more/less depending on specific device 
timing and lower level (Android platform) implementation.  Hence, this is 
definitely a legit error need to be address.

Another way, might be, to deal with that is to ensure all the implementation 
catching all possible exception and handle it gracefully, so not uncaught 
situation occurs.  Though that would be a huge change in implementation and 
probably a bigger impact on performance to try to catch everything.

There's a way to implement UncaughtException Handler for your process, but it 
would be difficult as when the exception is thrown, you don't really know what 
it is going on there.  So in my humble opinion, that might not be the 
feasible/correct direction to approach.

Original comment by tommypo...@gmail.com on 14 Sep 2012 at 7:55

GoogleCodeExporter commented 9 years ago
Agreed, SCAPI relies on the low level components to work properly. If a problem 
arises there, SCAPI is not able to deal with such. We have been discussing an 
architectural change where the terminals are separated in different APKs than 
the service which might help to solve the problem.

Original comment by Daniel.A...@gi-de.com on 18 Sep 2012 at 10:15