Attention is currently required from: laforge, lynxis lazus. dexter has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/simtrace2/+/43185?usp=email )
Change subject: Add osmo_apdu_segment_in2() to ensure correct parsing of GP GET DATA ...................................................................... Patch Set 3: Code-Review+1 (10 comments) Patchset: PS3: I have looked through your patch. Unfortunately I do not have the big picture, but the problem you describe and the solution appears to make sense. As far as I understand the problem is that the information about the original APDU case gets lost and this later causes problems when forwarding it. That is why you transfer the prev_ac->apdu_case to ac->apdu_case. Where I am a bit lost is to understand the difference between ETSI TS 102 221, Table 7.1 and GlobalPlatform A.1. They both apparently perform the re-send in the same manner. Maybe this corner case is more generic than originally assumed. You may also have a look [1], maybe this is helpful. [1] https://gitea.osmocom.org/sim-card/pysim/src/branch/master/pySim/transport/__init__.py Commit Message: https://gerrit.osmocom.org/c/simtrace2/+/43185/comment/6e023419_0d59277c?usp=email : PS3, Line 9: osmo_apdu_segment_in() don't have the context of the previous APDU, don't => does not https://gerrit.osmocom.org/c/simtrace2/+/43185/comment/b8040acd_06f148ee?usp=email : PS3, Line 12: GP GET DATA defines the Le field as 0x00 which is used to detect if this this => it https://gerrit.osmocom.org/c/simtrace2/+/43185/comment/0606454e_310fc93b?usp=email : PS3, Line 17: instead of a case 2 with Le != 0. I am having difficulties to understand this description. After responding we cannot go back and manipulated the Le field, we would have to go back in time. Which we cannot. Do you mean prev_ac with that. Or do you wanted to write ".. the following GP GET DATA ..."? File host/lib/apdu_dispatch.c: https://gerrit.osmocom.org/c/simtrace2/+/43185/comment/177859c7_811fa7c0?usp=email : PS3, Line 86: * \param prev_ac Previous APDU context across successive calls ac and prev_ac are [inout], right? (its probably not necessary to explicitly mark inout parameter in doxygen...) https://gerrit.osmocom.org/c/simtrace2/+/43185/comment/00bf6a02_a1d0661d?usp=email : PS3, Line 120: ac->hdr.cla & 0x80 && /* check for GP */ In GlobalPlatform Card Specification, section 11.3.2 they list more possible CLA bytes. Those are probably not relevant for our usecase. I also have only seen 0x80 so far. File host/tests/apdu_dispatch/apdu_dispatch_test.c: https://gerrit.osmocom.org/c/simtrace2/+/43185/comment/5ebb424a_830ea41f?usp=email : PS3, Line 18: memset(&prev_ac, 0, sizeof(prev_ac)); \ prev_ac only appears in the memset and nowhere else, is this line needed in this macro? https://gerrit.osmocom.org/c/simtrace2/+/43185/comment/c6612c0b_67103fa9?usp=email : PS3, Line 22: printf("%d (actual) != %d (intended)\n", rc, exp_rc);\ I think (expected) is more accurate. The variable is also called "exp_rc". https://gerrit.osmocom.org/c/simtrace2/+/43185/comment/cc45f40d_9d1f2fc4?usp=email : PS3, Line 55: rc = osmo_apdu_segment_in2(&ac, &prev_ac, get_data_c2_ca, ARRAY_SIZE(get_data_c2_ca), 1); The last parameter of osmo_apdu_segment_in2 is a bool. Maybe it is better to pass true instead of 1 then. Its the same, but easier to read. https://gerrit.osmocom.org/c/simtrace2/+/43185/comment/30c364ec_cb77aaaf?usp=email : PS3, Line 70: OSMO_ASSERT(rc == APDU_ACT_TX_CAPDU_TO_CARD); I wonder if it would make sense to inspect the contents of ac and prev_ac after osmo_apdu_segment_in2 was called. -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43185?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: Ie238662e7e6a10dd4283ede0c8d8d73bc375c247 Gerrit-Change-Number: 43185 Gerrit-PatchSet: 3 Gerrit-Owner: lynxis lazus <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Attention: lynxis lazus <[email protected]> Gerrit-Comment-Date: Wed, 12 Aug 2026 11:03:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes
