Change in simtrace2[master]: make peer ERASE more robust
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/14144 ) Change subject: make peer ERASE more robust .. Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/14144 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I06bfeaef09a397bd554bec84321e0dd64ccc3aac Gerrit-Change-Number: 14144 Gerrit-PatchSet: 1 Gerrit-Owner: Kévin Redon Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Comment-Date: Thu, 23 May 2019 16:34:24 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in simtrace2[master]: make peer ERASE more robust
Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/14144 ) Change subject: make peer ERASE more robust .. make peer ERASE more robust adds command 'a' to allow setting/asserting the peer SAM3S ERASE line on the next command. this prevents against accidental erase since only the command 'y' was required, without confirmation. this could happen not only through accidental user input, but noise on the serial line (noise would still cause other issues, but at least now it will not "brick" the device). now the sequence 'ay' is required, as any other command following 'a' would clear the permission again. note: since ERASE is only setting a GPIO within this command parsing function, not accidental function pointer problem calling 'board_exec_dbg_cmd' should cause accidental ERASE since it would need to be called two times with the exact sequence Change-Id: I06bfeaef09a397bd554bec84321e0dd64ccc3aac --- M firmware/libboard/qmod/source/board_qmod.c 1 file changed, 22 insertions(+), 3 deletions(-) Approvals: Harald Welte: Looks good to me, approved Jenkins Builder: Verified diff --git a/firmware/libboard/qmod/source/board_qmod.c b/firmware/libboard/qmod/source/board_qmod.c index ad1f6b7..cab5271 100644 --- a/firmware/libboard/qmod/source/board_qmod.c +++ b/firmware/libboard/qmod/source/board_qmod.c @@ -1,7 +1,7 @@ /* sysmocom quad-modem sysmoQMOD application code * * (C) 2016-2017 by Harald Welte - * (C) 2018, sysmocom -s.f.m.c. GmbH, Author: Kevin Redon + * (C) 2018-2019, sysmocom -s.f.m.c. GmbH, Author: Kevin Redon * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -184,6 +184,11 @@ /* returns '1' in case we should break any endless loop */ void board_exec_dbg_cmd(int ch) { + /* this variable controls if it is allowed to assert/release the ERASE line. + this is done to prevent accidental ERASE on noisy serial input since only one character can trigger the ERASE. +*/ + static bool allow_erase = false; + switch (ch) { case '?': printf("\t?\thelp\n\r"); @@ -205,6 +210,7 @@ printf("\tX\tRelease peer SAM3 from reset\n\r"); printf("\tx\tAssert peer SAM3 reset\n\r"); printf("\tY\tRelease peer SAM3 ERASE signal\n\r"); + printf("\ta\tAllow asserting peer SAM3 ERASE signal\n\r"); printf("\ty\tAssert peer SAM3 ERASE signal\n\r"); printf("\tU\tProceed to USB Initialization\n\r"); printf("\t1\tGenerate 1ms reset pulse on WWAN1\n\r"); @@ -243,9 +249,17 @@ printf("Clearing SIMTRACExx_ERASE (inactive)\n\r"); PIO_Clear(&pin_peer_erase); break; + case 'a': + printf("Asserting SIMTRACExx_ERASE allowed on next command\n\r"); + allow_erase = true; + break; case 'y': - printf("Seetting SIMTRACExx_ERASE (active)\n\r"); - PIO_Set(&pin_peer_erase); + if (allow_erase) { + printf("Setting SIMTRACExx_ERASE (active)\n\r"); + PIO_Set(&pin_peer_erase); + } else { + printf("Please first allow setting SIMTRACExx_ERASE\n\r"); + } break; case '1': printf("Resetting Modem 1 (of this SAM3)\n\r"); @@ -268,6 +282,11 @@ board_exec_dbg_cmd_st12only(ch); break; } + + // set protection back so it can only run for one command + if ('a' != ch) { + allow_erase = false; + } } void board_main_top(void) -- To view, visit https://gerrit.osmocom.org/14144 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I06bfeaef09a397bd554bec84321e0dd64ccc3aac Gerrit-Change-Number: 14144 Gerrit-PatchSet: 2 Gerrit-Owner: Kévin Redon Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102)
Change in simtrace2[master]: make peer ERASE more robust
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/14144 ) Change subject: make peer ERASE more robust .. Patch Set 1: I would prefer to completely remove the code from normal production buidls. We can always have a diffent build / makefile target for those rare casese where we actually want that functionality. -- To view, visit https://gerrit.osmocom.org/14144 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I06bfeaef09a397bd554bec84321e0dd64ccc3aac Gerrit-Change-Number: 14144 Gerrit-PatchSet: 1 Gerrit-Owner: Kévin Redon Gerrit-Reviewer: Jenkins Builder (102) Gerrit-CC: Harald Welte Gerrit-Comment-Date: Thu, 23 May 2019 14:46:04 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in simtrace2[master]: make peer ERASE more robust
Kévin Redon has uploaded this change for review. ( https://gerrit.osmocom.org/14144 Change subject: make peer ERASE more robust .. make peer ERASE more robust adds command 'a' to allow setting/asserting the peer SAM3S ERASE line on the next command. this prevents against accidental erase since only the command 'y' was required, without confirmation. this could happen not only through accidental user input, but noise on the serial line (noise would still cause other issues, but at least now it will not "brick" the device). now the sequence 'ay' is required, as any other command following 'a' would clear the permission again. note: since ERASE is only setting a GPIO within this command parsing function, not accidental function pointer problem calling 'board_exec_dbg_cmd' should cause accidental ERASE since it would need to be called two times with the exact sequence Change-Id: I06bfeaef09a397bd554bec84321e0dd64ccc3aac --- M firmware/libboard/qmod/source/board_qmod.c 1 file changed, 22 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/simtrace2 refs/changes/44/14144/1 diff --git a/firmware/libboard/qmod/source/board_qmod.c b/firmware/libboard/qmod/source/board_qmod.c index ad1f6b7..cab5271 100644 --- a/firmware/libboard/qmod/source/board_qmod.c +++ b/firmware/libboard/qmod/source/board_qmod.c @@ -1,7 +1,7 @@ /* sysmocom quad-modem sysmoQMOD application code * * (C) 2016-2017 by Harald Welte - * (C) 2018, sysmocom -s.f.m.c. GmbH, Author: Kevin Redon + * (C) 2018-2019, sysmocom -s.f.m.c. GmbH, Author: Kevin Redon * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -184,6 +184,11 @@ /* returns '1' in case we should break any endless loop */ void board_exec_dbg_cmd(int ch) { + /* this variable controls if it is allowed to assert/release the ERASE line. + this is done to prevent accidental ERASE on noisy serial input since only one character can trigger the ERASE. +*/ + static bool allow_erase = false; + switch (ch) { case '?': printf("\t?\thelp\n\r"); @@ -205,6 +210,7 @@ printf("\tX\tRelease peer SAM3 from reset\n\r"); printf("\tx\tAssert peer SAM3 reset\n\r"); printf("\tY\tRelease peer SAM3 ERASE signal\n\r"); + printf("\ta\tAllow asserting peer SAM3 ERASE signal\n\r"); printf("\ty\tAssert peer SAM3 ERASE signal\n\r"); printf("\tU\tProceed to USB Initialization\n\r"); printf("\t1\tGenerate 1ms reset pulse on WWAN1\n\r"); @@ -243,9 +249,17 @@ printf("Clearing SIMTRACExx_ERASE (inactive)\n\r"); PIO_Clear(&pin_peer_erase); break; + case 'a': + printf("Asserting SIMTRACExx_ERASE allowed on next command\n\r"); + allow_erase = true; + break; case 'y': - printf("Seetting SIMTRACExx_ERASE (active)\n\r"); - PIO_Set(&pin_peer_erase); + if (allow_erase) { + printf("Setting SIMTRACExx_ERASE (active)\n\r"); + PIO_Set(&pin_peer_erase); + } else { + printf("Please first allow setting SIMTRACExx_ERASE\n\r"); + } break; case '1': printf("Resetting Modem 1 (of this SAM3)\n\r"); @@ -268,6 +282,11 @@ board_exec_dbg_cmd_st12only(ch); break; } + + // set protection back so it can only run for one command + if ('a' != ch) { + allow_erase = false; + } } void board_main_top(void) -- To view, visit https://gerrit.osmocom.org/14144 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I06bfeaef09a397bd554bec84321e0dd64ccc3aac Gerrit-Change-Number: 14144 Gerrit-PatchSet: 1 Gerrit-Owner: Kévin Redon