From d46461bd2d8b27146c109a64895ede69a0b43773 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Mon, 8 Sep 2025 21:47:13 -0400 Subject: [PATCH 1/3] lpac-jni: Prevent potential C-side memory leak --- .../impl/LocalProfileAssistantImpl.kt | 116 ++++++++++-------- 1 file changed, 65 insertions(+), 51 deletions(-) diff --git a/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt b/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt index 3674f4f..90e20aa 100644 --- a/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt +++ b/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt @@ -16,7 +16,7 @@ class LocalProfileAssistantImpl( isdrAid: ByteArray, rawApduInterface: ApduInterface, rawHttpInterface: HttpInterface -): LocalProfileAssistant { +) : LocalProfileAssistant { companion object { private const val TAG = "LocalProfileAssistantImpl" } @@ -113,15 +113,17 @@ class LocalProfileAssistantImpl( while (curr != 0L) { val state = LocalProfileInfo.State.fromString(LpacJni.profileGetStateString(curr)) val clazz = LocalProfileInfo.Clazz.fromString(LpacJni.profileGetClassString(curr)) - ret.add(LocalProfileInfo( - LpacJni.profileGetIccid(curr), - state, - LpacJni.profileGetName(curr), - LpacJni.profileGetNickname(curr), - LpacJni.profileGetServiceProvider(curr), - LpacJni.profileGetIsdpAid(curr), - clazz - )) + ret.add( + LocalProfileInfo( + LpacJni.profileGetIccid(curr), + state, + LpacJni.profileGetName(curr), + LpacJni.profileGetNickname(curr), + LpacJni.profileGetServiceProvider(curr), + LpacJni.profileGetIsdpAid(curr), + clazz + ) + ) curr = LpacJni.profilesNext(curr) } @@ -134,18 +136,28 @@ class LocalProfileAssistantImpl( get() { val head = LpacJni.es10bListNotification(contextHandle) var curr = head - val ret = mutableListOf() - while (curr != 0L) { - ret.add(LocalProfileNotification( - LpacJni.notificationGetSeq(curr), - LocalProfileNotification.Operation.fromString(LpacJni.notificationGetOperationString(curr)), - LpacJni.notificationGetAddress(curr), - LpacJni.notificationGetIccid(curr), - )) - curr = LpacJni.notificationsNext(curr) + + try { + val ret = mutableListOf() + while (curr != 0L) { + ret.add( + LocalProfileNotification( + LpacJni.notificationGetSeq(curr), + LocalProfileNotification.Operation.fromString( + LpacJni.notificationGetOperationString( + curr + ) + ), + LpacJni.notificationGetAddress(curr), + LpacJni.notificationGetIccid(curr), + ) + ) + curr = LpacJni.notificationsNext(curr) + } + return ret.sortedBy { it.seqNumber }.reversed() + } finally { + LpacJni.notificationsFree(head) } - LpacJni.notificationsFree(head) - return ret.sortedBy { it.seqNumber }.reversed() } override val eID: String @@ -158,34 +170,34 @@ class LocalProfileAssistantImpl( val cInfo = LpacJni.es10cexGetEuiccInfo2(contextHandle) if (cInfo == 0L) return null - val ret = EuiccInfo2( - Version(LpacJni.euiccInfo2GetSGP22Version(cInfo)), - Version(LpacJni.euiccInfo2GetProfileVersion(cInfo)), - Version(LpacJni.euiccInfo2GetEuiccFirmwareVersion(cInfo)), - Version(LpacJni.euiccInfo2GetGlobalPlatformVersion(cInfo)), - LpacJni.euiccInfo2GetSasAcreditationNumber(cInfo), - Version(LpacJni.euiccInfo2GetPpVersion(cInfo)), - LpacJni.euiccInfo2GetFreeNonVolatileMemory(cInfo).toInt(), - LpacJni.euiccInfo2GetFreeVolatileMemory(cInfo).toInt(), - buildSet { - var cursor = LpacJni.euiccInfo2GetEuiccCiPKIdListForSigning(cInfo) - while (cursor != 0L) { - add(LpacJni.stringDeref(cursor)) - cursor = LpacJni.stringArrNext(cursor) - } - }, - buildSet { - var cursor = LpacJni.euiccInfo2GetEuiccCiPKIdListForVerification(cInfo) - while (cursor != 0L) { - add(LpacJni.stringDeref(cursor)) - cursor = LpacJni.stringArrNext(cursor) - } - }, - ) - - LpacJni.euiccInfo2Free(cInfo) - - return ret + try { + return EuiccInfo2( + Version(LpacJni.euiccInfo2GetSGP22Version(cInfo)), + Version(LpacJni.euiccInfo2GetProfileVersion(cInfo)), + Version(LpacJni.euiccInfo2GetEuiccFirmwareVersion(cInfo)), + Version(LpacJni.euiccInfo2GetGlobalPlatformVersion(cInfo)), + LpacJni.euiccInfo2GetSasAcreditationNumber(cInfo), + Version(LpacJni.euiccInfo2GetPpVersion(cInfo)), + LpacJni.euiccInfo2GetFreeNonVolatileMemory(cInfo).toInt(), + LpacJni.euiccInfo2GetFreeVolatileMemory(cInfo).toInt(), + buildSet { + var cursor = LpacJni.euiccInfo2GetEuiccCiPKIdListForSigning(cInfo) + while (cursor != 0L) { + add(LpacJni.stringDeref(cursor)) + cursor = LpacJni.stringArrNext(cursor) + } + }, + buildSet { + var cursor = LpacJni.euiccInfo2GetEuiccCiPKIdListForVerification(cInfo) + while (cursor != 0L) { + add(LpacJni.stringDeref(cursor)) + cursor = LpacJni.stringArrNext(cursor) + } + }, + ) + } finally { + LpacJni.euiccInfo2Free(cInfo) + } } @Synchronized @@ -201,8 +213,10 @@ class LocalProfileAssistantImpl( LpacJni.es10cDeleteProfile(contextHandle, iccid) == 0 @Synchronized - override fun downloadProfile(smdp: String, matchingId: String?, imei: String?, - confirmationCode: String?, callback: ProfileDownloadCallback) { + override fun downloadProfile( + smdp: String, matchingId: String?, imei: String?, + confirmationCode: String?, callback: ProfileDownloadCallback + ) { val res = LpacJni.downloadProfile( contextHandle, smdp, From 7db1c1ea9d3f313c15637de4b2c1f0b07b09b4fa Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Mon, 8 Sep 2025 21:48:41 -0400 Subject: [PATCH 2/3] lpac-jni: Add missing synchronized annotation --- .../java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt b/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt index 90e20aa..21f2f4e 100644 --- a/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt +++ b/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt @@ -272,6 +272,7 @@ class LocalProfileAssistantImpl( } } + @Synchronized override fun euiccMemoryReset() { LpacJni.es10cEuiccMemoryReset(contextHandle) } From 2cda633fd095666e0d4862bc457c6cafe78d7024 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Mon, 8 Sep 2025 21:58:53 -0400 Subject: [PATCH 3/3] lpac-jni: Convert all @Synchronized usage to a ReentrantLock --- .../impl/LocalProfileAssistantImpl.kt | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt b/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt index 21f2f4e..ce09717 100644 --- a/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt +++ b/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt @@ -11,6 +11,8 @@ import net.typeblog.lpac_jni.LocalProfileInfo import net.typeblog.lpac_jni.LocalProfileNotification import net.typeblog.lpac_jni.ProfileDownloadCallback import net.typeblog.lpac_jni.Version +import java.util.concurrent.locks.ReentrantLock +import kotlin.concurrent.withLock class LocalProfileAssistantImpl( isdrAid: ByteArray, @@ -74,6 +76,10 @@ class LocalProfileAssistantImpl( } } + // Controls concurrency of every single method in this class, since + // the C-side is explicitly NOT thread-safe + private val lock = ReentrantLock() + private val apduInterface = ApduInterfaceWrapper(rawApduInterface) private val httpInterface = HttpInterfaceWrapper(rawHttpInterface) @@ -105,8 +111,7 @@ class LocalProfileAssistantImpl( } override val profiles: List - @Synchronized - get() { + get() = lock.withLock { val head = LpacJni.es10cGetProfilesInfo(contextHandle) var curr = head val ret = mutableListOf() @@ -132,8 +137,7 @@ class LocalProfileAssistantImpl( } override val notifications: List - @Synchronized - get() { + get() = lock.withLock { val head = LpacJni.es10bListNotification(contextHandle) var curr = head @@ -161,12 +165,10 @@ class LocalProfileAssistantImpl( } override val eID: String - @Synchronized - get() = LpacJni.es10cGetEid(contextHandle)!! + get() = lock.withLock { LpacJni.es10cGetEid(contextHandle)!! } override val euiccInfo2: EuiccInfo2? - @Synchronized - get() { + get() = lock.withLock { val cInfo = LpacJni.es10cexGetEuiccInfo2(contextHandle) if (cInfo == 0L) return null @@ -200,23 +202,22 @@ class LocalProfileAssistantImpl( } } - @Synchronized - override fun enableProfile(iccid: String, refresh: Boolean): Boolean = + override fun enableProfile(iccid: String, refresh: Boolean): Boolean = lock.withLock { LpacJni.es10cEnableProfile(contextHandle, iccid, refresh) == 0 + } - @Synchronized - override fun disableProfile(iccid: String, refresh: Boolean): Boolean = + override fun disableProfile(iccid: String, refresh: Boolean): Boolean = lock.withLock { LpacJni.es10cDisableProfile(contextHandle, iccid, refresh) == 0 + } - @Synchronized - override fun deleteProfile(iccid: String): Boolean = + override fun deleteProfile(iccid: String): Boolean = lock.withLock { LpacJni.es10cDeleteProfile(contextHandle, iccid) == 0 + } - @Synchronized override fun downloadProfile( smdp: String, matchingId: String?, imei: String?, confirmationCode: String?, callback: ProfileDownloadCallback - ) { + ) = lock.withLock { val res = LpacJni.downloadProfile( contextHandle, smdp, @@ -243,18 +244,17 @@ class LocalProfileAssistantImpl( } } - @Synchronized - override fun deleteNotification(seqNumber: Long): Boolean = + override fun deleteNotification(seqNumber: Long): Boolean = lock.withLock { LpacJni.es10bDeleteNotification(contextHandle, seqNumber) == 0 + } - @Synchronized - override fun handleNotification(seqNumber: Long): Boolean = + override fun handleNotification(seqNumber: Long): Boolean = lock.withLock { LpacJni.handleNotification(contextHandle, seqNumber).also { Log.d(TAG, "handleNotification $seqNumber = $it") } == 0 + } - @Synchronized - override fun setNickname(iccid: String, nickname: String) { + override fun setNickname(iccid: String, nickname: String) = lock.withLock { val encoded = try { Charsets.UTF_8.encode(nickname).array() } catch (e: CharacterCodingException) { @@ -272,13 +272,13 @@ class LocalProfileAssistantImpl( } } - @Synchronized override fun euiccMemoryReset() { - LpacJni.es10cEuiccMemoryReset(contextHandle) + lock.withLock { + LpacJni.es10cEuiccMemoryReset(contextHandle) + } } - @Synchronized - override fun close() { + override fun close() = lock.withLock { if (!finalized) { LpacJni.euiccFini(contextHandle) LpacJni.destroyContext(contextHandle)