Attention is currently required from: laforge.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmocom-bb/+/36915?usp=email
to look at the new patch set (#3).
Change subject: trxcon/l1sched: refactor prim management in tx_tch[fh]_fn()
......................................................................
trxcon/l1sched: refactor prim management in tx_tch[fh]_fn()
The code path below the switch statement in tx_tch[fh]_fn() is no
longer common since we added CSD specific channel coding. This is
why we had to jump over it in several case statements.
This patch significantly reduces the number of goto statements
in these two functions and makes them easier to read/follow at
the price of code duplication, which is tolerable.
Change-Id: I5292abf6fcd308c9f7f12c7145d004103c9c7675
---
M src/host/trxcon/src/sched_lchan_tchf.c
M src/host/trxcon/src/sched_lchan_tchh.c
2 files changed, 87 insertions(+), 69 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/15/36915/3
--
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/36915?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: I5292abf6fcd308c9f7f12c7145d004103c9c7675
Gerrit-Change-Number: 36915
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: fixeria, laforge, osmith.
Hello Jenkins Builder, laforge, osmith,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmocom-bb/+/36914?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by laforge, Code-Review+1 by osmith
Change subject: trxcon/l1sched: make l1sched_lchan_emit_data_cnf() NULL-safe
......................................................................
trxcon/l1sched: make l1sched_lchan_emit_data_cnf() NULL-safe
Passing NULL to l1sched_lchan_emit_data_cnf() is not normal and
generally not expected, but definitely not fatal enough to abort
the process completely (due to assertion failure).
Change-Id: Ie64c176265f66a6c1515c66eb465d7e60f6768db
---
M src/host/trxcon/src/sched_prim.c
1 file changed, 15 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/14/36914/2
--
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/36914?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: Ie64c176265f66a6c1515c66eb465d7e60f6768db
Gerrit-Change-Number: 36914
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/37066?usp=email )
Change subject: libosmosim: class_tables: Fix GlobalPlatform CLA=8x INS=CA/CB GET DATA
......................................................................
libosmosim: class_tables: Fix GlobalPlatform CLA=8x INS=CA/CB GET DATA
in their infinite wisdom, GlobalPlatform made GET DATA a command that can be either APDU
case 2 or case 4. As the specify Le must be 0x00, we can conclude that P3 == 0x00 must be
Le, while P3 != 0x00 must be Lc and hence case 4 */
Change-Id: Ic8a17921f5a42d227791f1de39f90b4967c2e1b6
Related: SYS#6865
---
M src/sim/class_tables.c
M tests/sim/sim_test.c
M tests/sim/sim_test.ok
3 files changed, 33 insertions(+), 2 deletions(-)
Approvals:
fixeria: Looks good to me, but someone else must approve
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/src/sim/class_tables.c b/src/sim/class_tables.c
index 29ef2d7..3d50521 100644
--- a/src/sim/class_tables.c
+++ b/src/sim/class_tables.c
@@ -178,6 +178,7 @@
{
uint8_t ins = hdr[1];
uint8_t p1 = hdr[2];
+ uint8_t p3 = hdr[4];
switch (ins) {
case 0xE2: /* STORE DATA */
@@ -197,6 +198,16 @@
else
return 2; /* ETSI TS 102 221 V16.2.0 11.1.2 */
break;
+ case 0xCA:
+ case 0xCB:
+ /* in their infinite wisdom, GlobalPlatform made GET DATA a command that can be either APDU
+ * case 2 or case 4. As the specify Le must be 0x00, we can conclude that P3 == 0x00 must be
+ * Le, while P3 != 0x00 must be Lc and hence case 4 */
+ if (p3 == 0x00)
+ return 2;
+ else
+ return 4;
+ break;
}
return 0;
}
@@ -225,8 +236,8 @@
static const uint8_t gp_ins_tbl_8ce[256] = {
[0xE4] = 4, /* DELETE */
[0xE2] = 0x80, /* STORE DATA */
- [0xCA] = 4, /* GET DATA */
- [0xCB] = 4, /* GET DATA */
+ [0xCA] = 0x80, /* GET DATA */
+ [0xCB] = 0x80, /* GET DATA */
[0xF2] = 0x80, /* GET STATUS */
[0xE6] = 4, /* INSTALL */
[0xE8] = 4, /* LOAD */
diff --git a/tests/sim/sim_test.c b/tests/sim/sim_test.c
index 9a52af4..ab5d2be 100644
--- a/tests/sim/sim_test.c
+++ b/tests/sim/sim_test.c
@@ -29,6 +29,8 @@
const uint8_t uicc_upd[] = { 0x00, 0xD6, 0x00, 0x00, 0x02, 0x01, 0x02 };
const uint8_t uicc_get_status[] = { 0x80, 0xf2, 0x00, 0x02, 0x10 };
const uint8_t euicc_m2m_get_status[] = { 0x81, 0xf2, 0x40, 0x02, 0x02, 0x4f, 0x00 };
+const uint8_t gp_get_data2[] = { 0x81, 0xCA, 0x00, 0x5A, 0x00 };
+const uint8_t gp_get_data4[] = { 0x81, 0xCA, 0x00, 0x5A, 0x12 };
#define APDU_CASE_ASSERT(x, y) \
do { \
@@ -49,6 +51,8 @@
APDU_CASE_ASSERT(uicc_upd, 3);
APDU_CASE_ASSERT(uicc_get_status, 2);
APDU_CASE_ASSERT(euicc_m2m_get_status, 4);
+ APDU_CASE_ASSERT(gp_get_data2, 2);
+ APDU_CASE_ASSERT(gp_get_data4, 4);
}
int main(int argc, char **argv)
diff --git a/tests/sim/sim_test.ok b/tests/sim/sim_test.ok
index cac59ba..3abfb71 100644
--- a/tests/sim/sim_test.ok
+++ b/tests/sim/sim_test.ok
@@ -6,3 +6,5 @@
Testing uicc_upd
Testing uicc_get_status
Testing euicc_m2m_get_status
+Testing gp_get_data2
+Testing gp_get_data4
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/37066?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ic8a17921f5a42d227791f1de39f90b4967c2e1b6
Gerrit-Change-Number: 37066
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36501?usp=email )
Change subject: libosmosim: class_tables: Resolve conflicting CLA=8x INS=F2 definitions
......................................................................
libosmosim: class_tables: Resolve conflicting CLA=8x INS=F2 definitions
In their infinite wisdom, GlobalPlatform re-defined the CLA 8x / INS F2 command
alreay specified by ETSI TS 102 221. This wouldn't be as bads if they
had the same "Case". However, ETSI has case 2 while GP has case 4.
Lucikly, the P1 coding of ETSI [so far] states all the four upper bits
must be 0, while GP always has one of those bits set.
Before this patch, it is possible that a Modem/Phone will send an 8xF2
command and intends it as a GlobalPlatform command (with Lc > 0 and
command data portion), while this code assumes it is an ETSI UICC
command with Lc=0 and hence no command data portion. This will make
communication break when using simtrace2 'cardem'.
Change-Id: I8dd317ef8f942542e412b18c834a0467c51291c3
Related: SYS#6865
Related: https://lists.osmocom.org/hyperkitty/list/simtrace@lists.osmocom.org/thread…
---
M src/sim/class_tables.c
M tests/sim/sim_test.c
M tests/sim/sim_test.ok
3 files changed, 53 insertions(+), 6 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
fixeria: Looks good to me, but someone else must approve
diff --git a/src/sim/class_tables.c b/src/sim/class_tables.c
index 29c1e40..29ef2d7 100644
--- a/src/sim/class_tables.c
+++ b/src/sim/class_tables.c
@@ -187,6 +187,16 @@
default:
return 3;
}
+ break;
+ case 0xF2:
+ /* in their infinite wisdom, GlobalPlatform re-defined the CLA 8x / INS F2 command, so one can
+ * take a guess if it's GlobalPlatform or ETSI. Lucikly, the P1 coding of ETSI [so far]
+ * states all the four upper bits must be 0, while GP always has one of those bits set */
+ if (p1 & 0xF0)
+ return 4; /* GlobalPlatform v2.2 11.4.2 */
+ else
+ return 2; /* ETSI TS 102 221 V16.2.0 11.1.2 */
+ break;
}
return 0;
}
@@ -217,7 +227,7 @@
[0xE2] = 0x80, /* STORE DATA */
[0xCA] = 4, /* GET DATA */
[0xCB] = 4, /* GET DATA */
- [0xF2] = 4, /* GET STATUS */
+ [0xF2] = 0x80, /* GET STATUS */
[0xE6] = 4, /* INSTALL */
[0xE8] = 4, /* LOAD */
[0xD8] = 4, /* PUT KEY */
@@ -246,6 +256,12 @@
.helper = uicc046_cla_ins_helper,
.ins_tbl = uicc_ins_tbl_046,
}, {
+ /* must be before uicc_ins_tbl_8ce below with same CLA+mask */
+ .cla = 0x80,
+ .cla_mask = 0xF0,
+ .helper = gp_cla_ins_helper,
+ .ins_tbl = gp_ins_tbl_8ce,
+ }, {
.cla = 0x80,
.cla_mask = 0xF0,
.ins_tbl = uicc_ins_tbl_8ce,
@@ -258,11 +274,6 @@
.cla_mask = 0xF0,
.ins_tbl = uicc_ins_tbl_8ce,
}, {
- .cla = 0x80,
- .cla_mask = 0xF0,
- .helper = gp_cla_ins_helper,
- .ins_tbl = gp_ins_tbl_8ce,
- }, {
.cla = 0xC0,
.cla_mask = 0xF0,
.helper = gp_cla_ins_helper,
@@ -308,6 +319,12 @@
.helper = uicc046_cla_ins_helper,
.ins_tbl = uicc_ins_tbl_046,
}, {
+ /* must be before uicc_ins_tbl_8ce below with same CLA+mask */
+ .cla = 0x80,
+ .cla_mask = 0xF0,
+ .helper = gp_cla_ins_helper,
+ .ins_tbl = gp_ins_tbl_8ce,
+ }, {
.cla = 0x80,
.cla_mask = 0xF0,
.ins_tbl = uicc_ins_tbl_8ce,
diff --git a/tests/sim/sim_test.c b/tests/sim/sim_test.c
index 2e2eec5..9a52af4 100644
--- a/tests/sim/sim_test.c
+++ b/tests/sim/sim_test.c
@@ -27,6 +27,8 @@
const uint8_t uicc_tprof_wrong_class[] = { 0x00, 0x10, 0x00, 0x00, 0x02, 0x01, 0x02 };
const uint8_t uicc_read[] = { 0x00, 0xB0, 0x00, 0x00, 0x10 };
const uint8_t uicc_upd[] = { 0x00, 0xD6, 0x00, 0x00, 0x02, 0x01, 0x02 };
+const uint8_t uicc_get_status[] = { 0x80, 0xf2, 0x00, 0x02, 0x10 };
+const uint8_t euicc_m2m_get_status[] = { 0x81, 0xf2, 0x40, 0x02, 0x02, 0x4f, 0x00 };
#define APDU_CASE_ASSERT(x, y) \
do { \
@@ -45,6 +47,8 @@
APDU_CASE_ASSERT(uicc_tprof_wrong_class, 0);
APDU_CASE_ASSERT(uicc_read, 2);
APDU_CASE_ASSERT(uicc_upd, 3);
+ APDU_CASE_ASSERT(uicc_get_status, 2);
+ APDU_CASE_ASSERT(euicc_m2m_get_status, 4);
}
int main(int argc, char **argv)
diff --git a/tests/sim/sim_test.ok b/tests/sim/sim_test.ok
index 7d3f986..cac59ba 100644
--- a/tests/sim/sim_test.ok
+++ b/tests/sim/sim_test.ok
@@ -4,3 +4,5 @@
Testing uicc_tprof_wrong_class
Testing uicc_read
Testing uicc_upd
+Testing uicc_get_status
+Testing euicc_m2m_get_status
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36501?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I8dd317ef8f942542e412b18c834a0467c51291c3
Gerrit-Change-Number: 36501
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged